refactor: re-export tracing for internal-logs #2867
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2867 +/- ##
=====================================
Coverage 81.1% 81.1%
=====================================
Files 124 124
Lines 23927 23927
=====================================
Hits 19410 19410
Misses 4517 4517 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
opentelemetry-proto/Cargo.toml
Outdated
| @@ -44,7 +44,7 @@ zpages = ["trace"] | |||
| testing = ["opentelemetry/testing"] | |||
|
|
|||
| # add ons | |||
| internal-logs = ["tracing"] | |||
| internal-logs = [] | |||
There was a problem hiding this comment.
So there doesn't seem to be any mention of internal-logs anywhere in the crate. Why is there such a feature here?
There was a problem hiding this comment.
The "internal-logs" feature is used by internal logging macros defined in the "opentelemetry" crate. These macros expand at compile time, injecting the corresponding logging code into crates that use them and so this feature needs to be defined in those crates too. Ideally, it would be good if this can be avoided, while still allowing individual crates to independently enable or disable logging.
There was a problem hiding this comment.
@lalitb I know exactly what the feature does in the opentelemetry crate, but the fact that it has no dependencies here and is not referenced in code makes me think that it’s not needed in this crate.
There was a problem hiding this comment.
I think the configuration is/was actually incorrect, it should include the reference to the identically named feature in the opentelemetry crate.
There was a problem hiding this comment.
@bantonsson i believe it is being referenced indirectly here
# Conflicts: # opentelemetry-sdk/Cargo.toml
opentelemetry/src/lib.rs
Outdated
| #[doc(hidden)] | ||
| pub mod _private { | ||
| #[cfg(feature = "internal-logs")] | ||
| pub use tracing; // re-export |
There was a problem hiding this comment.
should this expose entire tracing module or selected components we care about?
There was a problem hiding this comment.
I exported the macros we actually use now. at the end we can always change it. it is not meant to be used directly by someone else than us.
Changes
tracingfrommacheteMerge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes