Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for new log formats #1286

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

mpeter50
Copy link

This MR adds definitions for the following log formats:

  • Gitea
  • Syncthing
  • I2P router
  • VirtioFS daemon
  • Android logcat

@mpeter50
Copy link
Author

The MR is WIP for now because I couldnt yet test Gitea and I2P log formats.
They were working some time ago, but currently they crash lnav. I'll update them and if they still crash it, I'll open an issue for that.

I would not be comfortable with publishing the long log files that I used for testing the formats, but if you would like to, I can send them to you in email or some other channel.

@mpeter50
Copy link
Author

There are 2 things I wanted to ask you about the included log files.

The first is, is it ok if the url value does not point to a documentation of the format?
For software like these, this is usually not documented as I have found.

The second is that you may notice that I have used 2 kinds of nonstandard JSON values, "comment(s)" and "subformats".
Can they stay? They are basically documentation oriented.
I use "comments" where something weird happens that I felt it needs to be explained to be maintainable, and for lack of comment support in the main JSON format I have found this to be a semi-good way to have comments.
"subformats" is for the very convoluted log format of the I2P router. I have found it easier to edit the big regex in smaller parts like this.
If they are fine, I would like to ask you to make palce for them in the format verifier, so that it does not warn for the unexpected values. Maybe it could allow a key name prefix, like any key that starts with doc-.

@mpeter50 mpeter50 marked this pull request as draft July 22, 2024 01:05
@mpeter50 mpeter50 changed the title [WIP] Add support for new log formats Add support for new log formats Jul 22, 2024
@tstack
Copy link
Owner

tstack commented Jul 22, 2024

They were working some time ago, but currently they crash lnav. I'll update them and if they still crash it, I'll open an issue for that.

Ah, sorry, what version of lnav is crashing?

I would not be comfortable with publishing the long log files that I used for testing the formats, but if you would like to, I can send them to you in email or some other channel.

You can send them to [email protected]

The first is, is it ok if the url value does not point to a documentation of the format?

That's fine.

The second is that you may notice that I have used 2 kinds of nonstandard JSON values, "comment(s)" and "subformats".
Can they stay? They are basically documentation oriented.

Doesn't lnav spit out some warnings about these?

The JSON parser that lnav uses allows C-style comments (// ... or /* ... */). Can you just convert them to that?

@mpeter50
Copy link
Author

Ah, sorry, what version of lnav is crashing?

No problem. It is 0.11.2. The reason I was surprised is because I didn't remember updating since writing the affected formats.
Just built 0.12.2 and it works fine, doesnt crash anymore. It could have been just my fault by not building on a tag, but on master; I remember that it did happen, but maybe I did not yet rebuild it on all machines.

Doesn't lnav spit out some warnings about these?

Yes, it does. Its like this: "warning: unexpected value for property “/gitea_log/sample#/comment”"

The JSON parser that lnav uses allows C-style comments (// ... or /* ... */). Can you just convert them to that?

Sure, I'll convert it soon. But it seems some text editors dont like it. They highlight this as an error, e.g. in vim and jetbrains idea.

image

image

@mpeter50
Copy link
Author

mpeter50 commented Jul 22, 2024

This version of the MR currently crashes because of the I2P format's regex. Regex101 says its ok, and removing any one of the 3 varaints in the outmost group fixes the crash. I'll open an issue for it.

Edit: opened #1288

tstack added a commit that referenced this pull request Aug 3, 2024
@tstack
Copy link
Owner

tstack commented Aug 3, 2024

The I2P logs you sent seem to already match the java_log format for the most part. Can we modify that instead of adding a separate format?

@tstack
Copy link
Owner

tstack commented Aug 3, 2024

Also, the virtiofsd log seems to be recognized by the Rust env_logger_log that was added to the top-of-tree.

@mpeter50
Copy link
Author

The I2P logs you sent seem to already match the java_log format for the most part. Can we modify that instead of adding a separate format?

For the most part thats a good idea, duplication is not good, but I worry if the java log format would get too bloated in the future if it will implement support for other java based software too :/
What do you think?

But, maybe I just misunderstood its purpose. Java (the runtime) in itself does not really produce log messages, so I guess its logical to use it for java software using a common format.

Though, how should I go about adapting the java_log format for I2P?
Should I change the current subformats (e.g. jvm) to also match the I2P messages?
Or add a new subformat?
Something else?

Also, the virtiofsd log seems to be recognized by the Rust env_logger_log that was added to the top-of-tree.

Thats good to know! I'll remove this format from the MR soon.

tstack added a commit that referenced this pull request Aug 14, 2024
@mpeter50
Copy link
Author

I have removed the virtiofsd format.

I have liked that the key-value pairs were highlighted, with a different color each.

Do you think it would be worth to add highlighting for the \w+=\w+ pattern or something similar to the env_logger_format for a similar effect? That would not make them distinct, though.

Former distinct KV highlighting Proposed KV highlighting in env_logger_log
image image

Is there maybe a setting I could toggle for myself, to highlight KV-pairs with a color unique by the key?


For now I havent touched the java_log format.
Should I just copy the regex pattern to a new object in the java_log.json, at java_log.regex.i2p_log?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants