Skip to content

Bugfix: Make sure all collectors have loggers #878

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Sticksman
Copy link
Contributor

I think this will make sure all loggers will also tell us when they got queried.

Signed-off-by: Felix Yuan <felix.yuan@reddit.com>
@Sticksman
Copy link
Contributor Author

@sysadmind @SuperQ

Copy link
Contributor

@sysadmind sysadmind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What code suggests that this will log when a collector gets called? I think I mist be missing something.

@SuperQ
Copy link
Contributor

SuperQ commented Sep 13, 2023

I agree, I'm not sure I see where the log collector struct field is used anywhere. I wonder if this is an old feature that has been refactored away.

@SuperQ
Copy link
Contributor

SuperQ commented Sep 13, 2023

Ahh, no, a few collectors use c.log for debug messages.

@sysadmind
Copy link
Contributor

Unless this PR is updated to include log messages when relevant, I don't think we need this PR. @Sticksman I will leave it up to you to decide if you want to add log messages or close the PR.

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

Successfully merging this pull request may close these issues.

3 participants