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.Ctx() should return global logger instead of disabledLogger #344

Closed
irl-segfault opened this issue Aug 11, 2021 · 3 comments
Closed

Comments

@irl-segfault
Copy link

log.Ctx(ctx) returns the associated logger if it's been registered in the ctx and returns disabledLogger otherwise. I think it should instead return the global logger.

It doesn't totally make intuitive sense to me that if a logger has not been registered in a context, that we by default assume logging should be disabled. My use case it to pass a sub-logger. If I miss a call to sublogger.WithContext() somewhere, I don't want logging to be erroneously disabled as a result.

@rs
Copy link
Owner

rs commented Aug 12, 2021

Good point. I would consider a PR.

@irl-segfault
Copy link
Author

Sure, I'll put one up tomorrow. Thanks for considering this -- helps a lot with getting zerolog to work nicely with OpenTelemetry tracing.

@rs
Copy link
Owner

rs commented Aug 12, 2021

Already fixed by #343

@rs rs closed this as completed Aug 12, 2021
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

2 participants