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

Reintroduce Filtered Logging #8752

Closed
4 tasks
Tracked by #54 ...
alexanderbez opened this issue Jun 14, 2022 · 5 comments
Closed
4 tasks
Tracked by #54 ...

Reintroduce Filtered Logging #8752

alexanderbez opened this issue Jun 14, 2022 · 5 comments
Assignees

Comments

@alexanderbez
Copy link
Contributor

alexanderbez commented Jun 14, 2022

Summary

The logger was refactored to use zerolog along with removing filtered logging in PR #6534

Problem Definition

Operators have expressed enough strong interest over time since the aforementioned PR to re-introduce filtered logging as it's removal introduces a massive burden to them when debugging or investing their logs.

Proposal

Add support for filtered logging by keeping the logging logic as-is but adding the filtering methods that were removed in #6534 back in.

cc @ValarDragon I know this will make you very happy :P


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@tac0turtle
Copy link
Contributor

I have a pr that is mostly completed, just need to fix some tests. Will open tomorrow when I can complete it

@cmwaters
Copy link
Contributor

Yeah I tend to agree. I have found it helpful to narrow down to just the section of logs I'm actually interested in

@tychoish
Copy link
Contributor

tychoish commented Aug 1, 2022

The main branch includes the legacy logger (non-zerolog) and the filtering code, which means that I think this is covered by the revert, and that no further work needs to be done, though if this doesn't work in 0.34.x according to your desires, then we can revisit it.

There's a bug in the implementation that I spotted, where if you create a filter on something that isn't later specified as a field to a With call, (or is) and then craft a message with that key/value pair passed to the message itself, it will show up in the logs even if the filter isn't specified. This means a user who doesn't know this, might see a message, attempt to filter out the k/v pair, and then still see it in the logs. This is maybe not ideal, but is definitely in released versions of the code, and is difficult to fix without some substantial reworking.

I think the way to implement this ontop of zerolog is to have the filtering use zerolog's message/level hook system which can be used to noop the message, rather than forward port the filtering shim as in #9067.

@thanethomson
Copy link
Contributor

@alexanderbez and @ValarDragon are there any substantial differences between what you want in v0.37 and what's already available in v0.34 in terms of filtered logging specifically?

For more general logging-related improvements that aren't related to filtered logging, please comment in #9076.

@tac0turtle
Copy link
Contributor

This was for 0.35 so I think it can be closed

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

No branches or pull requests

5 participants