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.

Sign up for GitHub

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

Jump to bottom

fix(logging): label value should be a logging level #848

Closed
wants to merge 2 commits into from
Closed

fix(logging): label value should be a logging level #848

wants to merge 2 commits into from

Conversation

gnuletik
Copy link

This PR fixes the usage of the Label(..) function in gologger.

When Verbose mode is activated, and gologger is configured to use the JSON formatter, here are some logs printed by subfinder:

{"level":"dnsdumpster","msg":"rate-limited-proxy-108-177-77-100.google.com","timestamp":"2023-05-25T14:07:57+0000"}
{"level":"dnsdumpster","msg":"code.l.google.com","timestamp":"2023-05-25T14:07:57+0000"}

As you can see, the level value is incorrectly defined.
The level value should be one of the one defined by gologger:

https://github.com/projectdiscovery/gologger/blob/1fc5289842cc77b4c81dfef7b775cf3b25f97cdf/gologger.go#L15-L22

So, dnsdumpster is not the correct value.

This PR fixes the behavior by using logger fields to define the current fields.

@olearycrew
Copy link
Contributor

Thanks for this contribution @gnuletik !

It looks like this PR would fix #847

@gnuletik
Copy link
Author

My pleasure! Yes it will fix this issue :)

@ehsandeep ehsandeep changed the base branch from main to dev May 26, 2023 20:22
@ehsandeep ehsandeep linked an issue May 26, 2023 that may be closed by this pull request
[Issue] Logging using incorrect level when verbose is set up #847
Open
@tarunKoyalwar tarunKoyalwar removed their request for review May 26, 2023 20:40
@tarunKoyalwar
Copy link
Member

hii @gnuletik , thanks for the PR but the original issue/feature seems to be gologger and it is being tracked here projectdiscovery/gologger#38

ref: #847 (comment)

@ehsandeep
Copy link
Member

Thank you @gnuletik for opening this PR, closing this one as it will be addressed in the golagger project.

@ehsandeep ehsandeep closed this May 27, 2023
@gnuletik
Copy link
Author

Hi @ehsandeep @tarunKoyalwar

I don't think that this issue should be addressed in gologger's projectdiscovery/gologger#38

As I described in the initial description of this PR, gologger is just allowing its users to override the level value in the log fields (which may not be a good practice, especially without enum, but that's another subject).

However, subfinder is explicitly NOT using the label field properly, as it is not respecting gologger's defined labels value:
https://github.com/projectdiscovery/gologger/blob/1fc5289842cc77b4c81dfef7b775cf3b25f97cdf/gologger.go#L15-L22

This PR fixes the subfinder usage of subfinder.

@gnuletik gnuletik mentioned this pull request May 28, 2023
Open
@gnuletik
Copy link
Author

@ehsandeep can you reconsider this PR please?

I really think that the underlying issue is about the usage of gologger in subfinder.

Thanks!

@ehsandeep
Copy link
Member

@gnuletik see #847 (comment)

@gnuletik gnuletik mentioned this pull request May 31, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Issue] Logging using incorrect level when verbose is set up
4 participants
@gnuletik @olearycrew @tarunKoyalwar @ehsandeep