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

promlog: check the log level first #322

Merged
merged 1 commit into from Sep 26, 2021
Merged

Conversation

royeo
Copy link
Contributor

@royeo royeo commented Aug 12, 2021

The logger returned by level.NewFilter will check the log level first, this avoids meaningless caller and ts calls.

Signed-off-by: royeo <ljn6176@gmail.com>
@roidelapluie
Copy link
Member

Can you please explain why we would need this change? Thanks!

@royeo
Copy link
Contributor Author

royeo commented Aug 13, 2021

The logger returned by log.With will gets the value of the bound field such as ts and caller, runtime.Caller is expensive. So if it's the previous logic, logger will obtains the binding value before checking the log level, this has some impact on performance.

@roidelapluie roidelapluie merged commit eb9347f into prometheus:main Sep 26, 2021
@roidelapluie
Copy link
Member

Thank you, this is correct.

LeviHarrison pushed a commit to LeviHarrison/common that referenced this pull request Oct 9, 2021
Signed-off-by: royeo <ljn6176@gmail.com>
@bboreham
Copy link
Member

Looks like this breaks printing the caller. DefaultCaller is hard-coded to select the 3rd stack frame up, and the filter adds another one.

https://play.golang.org/p/FPPgJPuk2L3

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.

None yet

3 participants