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

log.SetFlags(0) doesn't remove timestamp from log lines, preventing InferLevels to work properly #100

Closed
lgfa29 opened this issue Oct 20, 2021 · 4 comments · Fixed by #101

Comments

@lgfa29
Copy link
Contributor

lgfa29 commented Oct 20, 2021

While working on hashicorp/nomad#11291 I wasn't able to get InferLevels to work properly because the log lines generated by external dependencies still had the timestamp.

Maybe we can extend the search scope from HasPrefix to Contains?

@evanphx
Copy link
Contributor

evanphx commented Oct 20, 2021

I don't really want to expand the scope to Contains because it's too easy to get a false positive. Can nomad turn timestamps off on it's logger instead?

@lgfa29
Copy link
Contributor Author

lgfa29 commented Oct 20, 2021

Yeah, makes sense. I tried to disable the timestamps from Nomad here, but these log lines are coming from external dependencies (like mdns for example). Even calling log.SetFlags in an init function in the main package didn't help.

What do you think about making the search scope configurable? So callers would be able to do something like:

&hclog.StandardLoggerOptions{
    InferLevels: true,
    InferLevelFullLine: true,
})

But with a better name 😅

@evanphx
Copy link
Contributor

evanphx commented Oct 20, 2021

Sure, we could introduce additional options. Either TryReallyHardToInfer to use Contains or maybe InferAroundTimestamps that could detect and skip the timestamp?

@lgfa29
Copy link
Contributor Author

lgfa29 commented Oct 21, 2021

Nice, trying to detect the timestamp seems like a good approach. I will raise a PR soon. Thanks!

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 a pull request may close this issue.

2 participants