opentelemetry-rust
opentelemetry-rust copied to clipboard
[WIP] Suppression of nested logs from dependencies
Raising this PR as alternate approach to fix #1171, by extending the Context structure to store the suppression flag. The PR is to discuss this approach. The earlier approach #1315 was based on tokio runtime, and generalizing it would have been difficult.
Change summary
- Move
FutureExt
andWithContext
constructs fromtrace/context.rs
tocontext.rs
. - Add suppress_logs flag in
Context
struct
pub struct Context {
#[cfg(feature = "trace")]
pub(super) span: Option<Arc<SynchronizedSpan>>,
pub suppress_logs: bool,
entries: HashMap<TypeId, Arc<dyn Any + Sync + Send>, BuildHasherDefault<IdHasher>>,
}
- Add
with_suppression
method in Context to enable suppression:
impl Context {
...
pub(super) fn with_suppression(&self) -> Self {
Context {
suppress_logs: true,
entries: self.entries.clone(),
..self.clone()
}
}
}
- Add
with_current_context_supp
method in FutureExt:
pub trait FutureExt: Sized {
...
fn with_current_context_supp(self) -> WithContext<Self> {
let otel_cx = Context::current().with_suppression();
self.with_context(otel_cx)
}
}
- Suppress in Exporter::export method:
impl LogExporter for OtlpHttpClient {
async fn export(&mut self, batch: Vec<LogData>) -> LogResult<()> {
async {
// export logic
}
.with_current_context_supp()
.await
}
- Check if suppression is enabled in LogProcessor:
impl<R: RuntimeChannel<BatchMessage>> LogProcessor for BatchLogProcessor<R> {
...
#[cfg(feature = "logs_level_enabled")]
fn event_enabled(&self, _level: Severity, _target: &str, _name: &str) -> bool {
!Context::current().suppress_logs
}
}
- How to test:
run opentelemetry-otlp/examples/basic-otlp-http/ example, while collector is running (using docker-compose up).
As this PR came into discussion in today's community meeting, summarising the challenge with the current solution -
The PR adds the suppress_logs flag in the Context, and the Context object is propagated inside the async task. However, wrapping the future with FutureExt::WithExt
will not cascade the wrapping behavior to all internal futures. So if these internal futures (which could be from external (say) Hyper crate) get invoked in a separate thread, they won't see the suppression flag from context. And we can't explicitly apply the wrapper to each async function of third-party crates. This means, that if the logging macro is invoked from inside one of these async tasks, the suppression_flag won't be set and thus the logs are not suppressed.
closing this PR. The changes would be done based on the options discussed here - https://github.com/open-telemetry/opentelemetry-rust/issues/761#issuecomment-2121548678.