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

feat: Allow event name to be provided to IsEnabled check in Logger #2756

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions opentelemetry-appender-log/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,11 @@ where
{
fn enabled(&self, _metadata: &Metadata) -> bool {
#[cfg(feature = "spec_unstable_logs_enabled")]
return self
.logger
.event_enabled(severity_of_level(_metadata.level()), _metadata.target());
return self.logger.event_enabled(
severity_of_level(_metadata.level()),
_metadata.target(),
None,
);
#[cfg(not(feature = "spec_unstable_logs_enabled"))]
true
}
Expand Down
4 changes: 4 additions & 0 deletions opentelemetry-appender-tracing/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ Receivers (processors, exporters) are expected to use `LogRecord.target()` as
scope name. This is already done in OTLP Exporters, so this change should be
transparent to most users.

- Passes event name to the `event_enabled` method on the `Logger`. This allows
implementations (SDK, processor, exporters) to leverage this additional
information to determine if an event is enabled.

## 0.28.1

Released 2025-Feb-12
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-appender-tracing/benches/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl LogProcessor for NoopProcessor {
&self,
_level: opentelemetry::logs::Severity,
_target: &str,
_name: &str,
_name: Option<&str>,
) -> bool {
self.enabled
}
Expand Down
97 changes: 80 additions & 17 deletions opentelemetry-appender-tracing/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,12 @@
event: &tracing::Event<'_>,
_ctx: tracing_subscriber::layer::Context<'_, S>,
) {
let severity = severity_of_level(event.metadata().level());
let target = event.metadata().target();
let metadata = event.metadata();
let severity = severity_of_level(metadata.level());
let target = metadata.target();
let name = metadata.name();
#[cfg(feature = "spec_unstable_logs_enabled")]
if !self.logger.event_enabled(severity, target) {
if !self.logger.event_enabled(severity, target, Some(name)) {
// TODO: See if we need internal logs or track the count.
return;
}
Expand All @@ -169,16 +171,13 @@
#[cfg(feature = "experimental_metadata_attributes")]
let meta = normalized_meta.as_ref().unwrap_or_else(|| event.metadata());

#[cfg(not(feature = "experimental_metadata_attributes"))]
let meta = event.metadata();

let mut log_record = self.logger.create_log_record();

// TODO: Fix heap allocation
log_record.set_target(target.to_string());
log_record.set_event_name(meta.name());
log_record.set_event_name(name);
log_record.set_severity_number(severity);
log_record.set_severity_text(meta.level().as_str());
log_record.set_severity_text(metadata.level().as_str());
let mut visitor = EventVisitor::new(&mut log_record);
#[cfg(feature = "experimental_metadata_attributes")]
visitor.visit_experimental_metadata(meta);
Expand Down Expand Up @@ -226,9 +225,10 @@
use opentelemetry::logs::Severity;
use opentelemetry::trace::TracerProvider;
use opentelemetry::trace::{TraceContextExt, TraceFlags, Tracer};
use opentelemetry::InstrumentationScope;
use opentelemetry::{logs::AnyValue, Key};
use opentelemetry_sdk::error::OTelSdkResult;
use opentelemetry_sdk::logs::InMemoryLogExporter;
use opentelemetry_sdk::logs::{InMemoryLogExporter, LogProcessor};
use opentelemetry_sdk::logs::{LogBatch, LogExporter};
use opentelemetry_sdk::logs::{SdkLogRecord, SdkLoggerProvider};
use opentelemetry_sdk::trace::{Sampler, SdkTracerProvider};
Expand All @@ -244,10 +244,7 @@
}

#[allow(impl_trait_overcaptures)] // can only be fixed with Rust 1.82+
fn create_tracing_subscriber(
_exporter: InMemoryLogExporter,
logger_provider: &SdkLoggerProvider,
) -> impl tracing::Subscriber {
fn create_tracing_subscriber(logger_provider: &SdkLoggerProvider) -> impl tracing::Subscriber {
let level_filter = tracing_subscriber::filter::LevelFilter::WARN; // Capture WARN and ERROR levels
let layer =
layer::OpenTelemetryTracingBridge::new(logger_provider).with_filter(level_filter); // No filter based on target, only based on log level
Expand Down Expand Up @@ -327,7 +324,7 @@
.with_simple_exporter(exporter.clone())
.build();

let subscriber = create_tracing_subscriber(exporter.clone(), &logger_provider);
let subscriber = create_tracing_subscriber(&logger_provider);

// avoiding setting tracing subscriber as global as that does not
// play well with unit tests.
Expand Down Expand Up @@ -450,7 +447,7 @@
.with_simple_exporter(exporter.clone())
.build();

let subscriber = create_tracing_subscriber(exporter.clone(), &logger_provider);
let subscriber = create_tracing_subscriber(&logger_provider);

// avoiding setting tracing subscriber as global as that does not
// play well with unit tests.
Expand Down Expand Up @@ -627,7 +624,7 @@
.with_simple_exporter(exporter.clone())
.build();

let subscriber = create_tracing_subscriber(exporter.clone(), &logger_provider);
let subscriber = create_tracing_subscriber(&logger_provider);

// avoiding setting tracing subscriber as global as that does not
// play well with unit tests.
Expand Down Expand Up @@ -703,7 +700,7 @@
.with_simple_exporter(exporter.clone())
.build();

let subscriber = create_tracing_subscriber(exporter.clone(), &logger_provider);
let subscriber = create_tracing_subscriber(&logger_provider);

// avoiding setting tracing subscriber as global as that does not
// play well with unit tests.
Expand Down Expand Up @@ -785,4 +782,70 @@
assert!(!attributes_key.contains(&Key::new("log.target")));
}
}

#[derive(Debug)]
struct LogProcessorWithIsEnabled {
severity_level: Severity,
name: String,
target: String,
}

impl LogProcessorWithIsEnabled {
fn new(severity_level: Severity, name: String, target: String) -> Self {
LogProcessorWithIsEnabled {
severity_level,
name,
target,
}
}
}

impl LogProcessor for LogProcessorWithIsEnabled {
fn emit(&self, _record: &mut SdkLogRecord, _scope: &InstrumentationScope) {
// no-op
}

#[cfg(feature = "spec_unstable_logs_enabled")]
fn event_enabled(&self, level: Severity, target: &str, name: Option<&str>) -> bool {
// assert that passed in arguments are same as the ones set in the test.
assert_eq!(self.severity_level, level);
assert_eq!(self.target, target);
assert_eq!(
self.name,
name.expect("name is expected from tracing appender")
);
true
}

fn force_flush(&self) -> OTelSdkResult {
Ok(())
}

Check warning on line 822 in opentelemetry-appender-tracing/src/layer.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-appender-tracing/src/layer.rs#L820-L822

Added lines #L820 - L822 were not covered by tests

fn shutdown(&self) -> OTelSdkResult {
Ok(())
}
}

#[cfg(feature = "spec_unstable_logs_enabled")]
#[test]
fn is_enabled() {
// Arrange
let logger_provider = SdkLoggerProvider::builder()
.with_log_processor(LogProcessorWithIsEnabled::new(
Severity::Error,
"my-event-name".to_string(),
"my-system".to_string(),
))
.build();

let subscriber = create_tracing_subscriber(&logger_provider);

// avoiding setting tracing subscriber as global as that does not
// play well with unit tests.
let _guard = tracing::subscriber::set_default(subscriber);

// Name, Target and Severity are expected to be passed to the IsEnabled check
// The validation is done in the LogProcessorWithIsEnabled struct.
error!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "otel@opentelemetry.io");
}
}
9 changes: 9 additions & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,13 @@

- **Breaking** The SpanExporter::export() method no longer requires a mutable reference to self.
Before:

```rust
async fn export(&mut self, batch: Vec<SpanData>) -> OTelSdkResult
```

After:

```rust
async fn export(&self, batch: Vec<SpanData>) -> OTelSdkResult
```
Expand All @@ -52,6 +55,12 @@
when its `shutdown` is invoked.

- Reduced some info level logs to debug
- **Breaking** for custom LogProcessor/Exporter authors:
Added an additional `name: Option<&str>` parameter to the `event_enabled`
method on the `LogProcessor` and `LogExporter` traits. This allows
implementations to leverage this additional information to determine if an
event is enabled. `SdkLogger` no longer passes its `scope` name but instead
passes the incoming `name` when invoking `event_enabled` on processors.

## 0.28.0

Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/logs/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
}
#[cfg(feature = "spec_unstable_logs_enabled")]
/// Check if logs are enabled.
fn event_enabled(&self, _level: Severity, _target: &str, _name: &str) -> bool {
fn event_enabled(&self, _level: Severity, _target: &str, _name: Option<&str>) -> bool {

Check warning on line 144 in opentelemetry-sdk/src/logs/export.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/logs/export.rs#L144

Added line #L144 was not covered by tests
// By default, all logs are enabled
true
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/logs/log_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub trait LogProcessor: Send + Sync + Debug {
fn shutdown(&self) -> OTelSdkResult;
#[cfg(feature = "spec_unstable_logs_enabled")]
/// Check if logging is enabled
fn event_enabled(&self, _level: Severity, _target: &str, _name: &str) -> bool {
fn event_enabled(&self, _level: Severity, _target: &str, _name: Option<&str>) -> bool {
// By default, all logs are enabled
true
}
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-sdk/src/logs/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ impl opentelemetry::logs::Logger for SdkLogger {
}

#[cfg(feature = "spec_unstable_logs_enabled")]
fn event_enabled(&self, level: Severity, target: &str) -> bool {
fn event_enabled(&self, level: Severity, target: &str, name: Option<&str>) -> bool {
self.provider
.log_processors()
.iter()
.any(|processor| processor.event_enabled(level, target, self.scope.name().as_ref()))
.any(|processor| processor.event_enabled(level, target, name))
}
}
4 changes: 4 additions & 0 deletions opentelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
- Updated `Baggage` constants to reflect latest standard (`MAX_KEY_VALUE_PAIRS` - 180 -> 64, `MAX_BYTES_FOR_ONE_PAIR` - removed) and increased insert performance see #[2284](https://github.com/open-telemetry/opentelemetry-rust/pull/2284).
- *Breaking* Align `Baggage.remove()` signature with `.get()` to take the key as a reference

- Added additional `name: Option<&str>` parameter to the `event_enabled` method
on the `Logger` trait. This allows implementations (SDK, processor, exporters)
to leverage this additional information to determine if an event is enabled.

## 0.28.0

Released 2025-Feb-10
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry/src/logs/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub trait Logger {

#[cfg(feature = "spec_unstable_logs_enabled")]
/// Check if the given log level is enabled.
fn event_enabled(&self, level: Severity, target: &str) -> bool;
fn event_enabled(&self, level: Severity, target: &str, name: Option<&str>) -> bool;
}

/// Interfaces that can create [`Logger`] instances.
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry/src/logs/noop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
}
fn emit(&self, _record: Self::LogRecord) {}
#[cfg(feature = "spec_unstable_logs_enabled")]
fn event_enabled(&self, _level: super::Severity, _target: &str) -> bool {
fn event_enabled(&self, _level: super::Severity, _target: &str, _name: Option<&str>) -> bool {

Check warning on line 82 in opentelemetry/src/logs/noop.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry/src/logs/noop.rs#L82

Added line #L82 was not covered by tests
false
}
}