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

Unexpected behavior with max_level_* in dependencies. #309

Closed
olivia-fl opened this issue Dec 4, 2018 · 4 comments
Closed

Unexpected behavior with max_level_* in dependencies. #309

olivia-fl opened this issue Dec 4, 2018 · 4 comments

Comments

@olivia-fl
Copy link

I ran into an issue where one of my transitive dependencies set release_max_level_error. This meant that when I tried to specify release_max_level_info, it didn't actually do anything in either my crate or the dependency. This is both rather confusing to debug, and means that there's currently no way I can get my crate to output info! calls on release builds.

In my case, it would be better to revert the ordering of which max_level_* features override others, os that max_level_info would take priority over max_level_error, but I could also easily imagine a situation where this would be reversed.

Because of this, I'm not sure if there's a reasonable solution.

@KodrAus
Copy link
Contributor

KodrAus commented Dec 4, 2018

@Benjamin-L Hmm this is a problem. Libraries shouldn't be using the max_level features, and leave it to the end binary to specify for this reason.

I think we should call that out in the docs more. For this specific issue I think we should try get that transitive dependency to avoid using those features.

@olivia-fl
Copy link
Author

I feel like it does sometimes make sense to have a dependency default to a particular max_level, since logging could be a really obscure performance issue that users of the library might not think of. Ideally, there would be a way to have different max levels between dependencies, although I can't think of a way to do that with cargo's features.

@KodrAus
Copy link
Contributor

KodrAus commented Dec 4, 2018

Yeh, the global nature of these features is a bit of a footgun. For the system we've got now I think we should strongly recommend libraries against using the max-level features. If they're logging in paths that are hot enough for an atomic load and check to make a noticeable impact then they could provide their own wrappers for the log macros that are compiled out in release mode.

@KodrAus
Copy link
Contributor

KodrAus commented Jan 3, 2019

It looks like the dependency in question will rework their use of the max_level features, and some docs have been added recommending against using these features in libraries.

I'll go ahead and close this one now, but please feel free to re-open it if anything new comes up!

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