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

don't pull attributes feature of tracing #32

Merged
merged 3 commits into from
Sep 3, 2021

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Aug 28, 2021

@matklad
Copy link
Contributor Author

matklad commented Aug 28, 2021

would appreciate a release with this one

@hawkw
Copy link

hawkw commented Aug 28, 2021

AFAICT, this crate really doesn't need to depend on tracing at all; as a Layer implementation, it should only need to depend on tracing_core and tracing_subscriber. The difference between depending on tracing and tracing_core is that tracing includes the instrumentation-side API (e.g., the macros, the Span type, and other APIs for emitting trace data), which this crate doesn't use at all (except in examples).

I think the best thing is probably to just replace the tracing dependency with tracing_core here.

@matklad
Copy link
Contributor Author

matklad commented Aug 29, 2021

(CI failure is unrelated)

@hawkw good suggestion! One interesting observation here is that tracing_subscriber itself depends on tracing when filter features is enabled, which again means that all actual subscribers are compield after syn (if the attributes feature is enaled). Naively, I'd expect STATIC_MAX_LEVEL to live in tracing_core, and for tracing_subscriber to not depend on tracing.

@hawkw
Copy link

hawkw commented Aug 29, 2021

Hmm, the filter feature should probably not be enabled by crates that provide subscriber implementations.

In the future, we should probably split out the implementations (e.g. env_filter) from the core traits in tracing_subscriber, and put the core tracing-subscriber traits in a separate downstream crate...

@matklad
Copy link
Contributor Author

matklad commented Aug 29, 2021

Hmm, the filter feature should probably not be enabled by crates that provide subscriber implementations.

In my case, it’s the leaf crate that enables the filter feature, but that still means that the subscriber gets this feature, and needs to be compiled after proc macros.

@davidbarsky
Copy link
Owner

Whoops, sorry, I didn't see this PR. I'll fixup the CI issues, but I'm happy to merge this after.

@davidbarsky
Copy link
Owner

@matklad Took me a second, sorry! Now that #34 is merged, do you think you can rebase against the main branch? I'll be happy to merge this in after and do a release.

@davidbarsky davidbarsky self-requested a review September 3, 2021 15:02
@davidbarsky davidbarsky merged commit c66014b into davidbarsky:main Sep 3, 2021
bors bot added a commit to rust-lang/rust-analyzer that referenced this pull request Sep 6, 2021
10165: update to tracing-tree 0.1.10, which does not pull in syn r=matklad a=davidbarsky

I've updated tracing-tree to 0.1.10, which does not pull in syn and proc-macro2 (thanks for [the PR](davidbarsky/tracing-tree#32), `@matklad!).`

It took a little bit more work than I expected to land davidbarsky/tracing-tree#33, but I should get that done this week. However, I didn't want to keep y'all waiting, so here's _some_ of the changes that should hopefully improve your compile times.

Co-authored-by: David Barsky <me@davidbarsky.com>
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