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

Improve internal opentelemetry logging #2128

Merged
merged 38 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
7da284e
initial commit
lalitb Sep 18, 2024
573994d
instrument internal code
lalitb Sep 19, 2024
1bfd464
instrument..
lalitb Sep 19, 2024
90470c3
instrument..
lalitb Sep 19, 2024
5f1b990
update example
lalitb Sep 19, 2024
47abb6d
Merge branch 'main' into log-handler-using-tracing
lalitb Sep 19, 2024
46ce8af
update example
lalitb Sep 19, 2024
bbfc03c
Merge branch 'log-handler-using-tracing' of github.com:lalitb/opentel…
lalitb Sep 19, 2024
e5260d9
changes for filtering
lalitb Sep 19, 2024
a79b636
Merge branch 'main' into log-handler-using-tracing
TommyCpp Sep 21, 2024
197b12a
use structured logging
lalitb Sep 23, 2024
91e56ea
review comments
lalitb Sep 27, 2024
fa7148e
update self diagnostic
lalitb Sep 27, 2024
a78357c
allow internal events in pipeline
lalitb Sep 27, 2024
a3fba36
Merge branch 'main' into log-handler-using-tracing
lalitb Sep 27, 2024
54e0755
fix macros
lalitb Oct 1, 2024
4c8cb42
revert appender-tracing example
lalitb Oct 1, 2024
5c4bab9
Merge branch 'main' into log-handler-using-tracing
lalitb Oct 1, 2024
5e95e00
update instrument
lalitb Oct 2, 2024
da3716d
cont..
lalitb Oct 2, 2024
62b9f83
lint fix
lalitb Oct 2, 2024
591c45a
fix macro to dump operation name as name keyt
lalitb Oct 2, 2024
b6e2327
Merge branch 'main' into log-handler-using-tracing
lalitb Oct 2, 2024
2714508
fix unit test for appender-tracing
lalitb Oct 3, 2024
b0c129c
Merge branch 'log-handler-using-tracing' of github.com:lalitb/opentel…
lalitb Oct 3, 2024
83763b1
fix unit tests, and doc test
lalitb Oct 3, 2024
e325316
add macros under feature flag
lalitb Oct 3, 2024
19822b1
make feature default
lalitb Oct 3, 2024
6b98c16
fix build
lalitb Oct 3, 2024
d248750
fix lint
lalitb Oct 3, 2024
f9e3d2d
lint
lalitb Oct 3, 2024
d4c1eb7
use eprintln for warn and error if internal-logs flag is disabled
lalitb Oct 3, 2024
d5120a2
review comments
lalitb Oct 3, 2024
c103467
remove tracing from opentelemetry crate, and use name as metadata
lalitb Oct 3, 2024
b7470b1
fix
lalitb Oct 3, 2024
30f7a4c
remove tracing from otel dev dep
lalitb Oct 3, 2024
adc3fd2
remove debug
lalitb Oct 3, 2024
3c4d20c
Update opentelemetry-sdk/src/metrics/internal/mod.rs
lalitb Oct 3, 2024
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
6 changes: 3 additions & 3 deletions opentelemetry-appender-tracing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ rust-version = "1.65"

[dependencies]
log = { workspace = true, optional = true }
opentelemetry = { version = "0.25", path = "../opentelemetry", features = ["logs"] }
opentelemetry = { version = "0.25", path = "../opentelemetry", features = ["logs","experimental-internal-debugging"] }
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
tracing = { workspace = true, features = ["std"]}
tracing-core = { workspace = true }
tracing-log = { version = "0.2", optional = true }
tracing-subscriber = { workspace = true, features = ["registry", "std"] }
tracing-subscriber = { workspace = true, features = ["registry", "std", "fmt", "ansi", "env-filter"] }

[dev-dependencies]
log = { workspace = true }
opentelemetry-stdout = { path = "../opentelemetry-stdout", features = ["logs"] }
opentelemetry_sdk = { path = "../opentelemetry-sdk", features = ["logs", "testing"] }
opentelemetry_sdk = { path = "../opentelemetry-sdk", features = ["logs", "testing", "experimental-internal-debugging"] }
tracing-log = "0.2"
async-trait = { workspace = true }
criterion = { workspace = true }
Expand Down
21 changes: 17 additions & 4 deletions opentelemetry-appender-tracing/examples/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ use opentelemetry::KeyValue;
use opentelemetry_appender_tracing::layer;
use opentelemetry_sdk::{logs::LoggerProvider, Resource};
use tracing::error;
use tracing_subscriber::filter::{filter_fn, EnvFilter};
use tracing_subscriber::fmt;
use tracing_subscriber::layer::Layer;
use tracing_subscriber::prelude::*;

fn main() {
Expand All @@ -15,9 +18,19 @@ fn main() {
)]))
.with_simple_exporter(exporter)
.build();
let layer = layer::OpenTelemetryTracingBridge::new(&provider);
tracing_subscriber::registry().with(layer).init();

error!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "otel@opentelemetry.io", message = "This is an example message");
let env_filter = EnvFilter::from_default_env();
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
let otel_layer = layer::OpenTelemetryTracingBridge::new(&provider);
let internal_log_layer = fmt::Layer::default()
.with_writer(std::io::stdout) // Writes to stdout
.pretty()
.with_filter(filter_fn(|meta| meta.target().starts_with("opentelemetry"))) // Custom filter function
.with_filter(env_filter);
tracing_subscriber::registry()
.with(internal_log_layer)
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
.with(otel_layer)
.init();
for i in 0..3 {
error!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "otel@opentelemetry.io", message = "This is an example message", iteration = i);
}
let _ = provider.shutdown();
}
3 changes: 3 additions & 0 deletions opentelemetry-appender-tracing/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ where
event: &tracing::Event<'_>,
_ctx: tracing_subscriber::layer::Context<'_, S>,
) {
if event.metadata().target().starts_with("opentelemetry") {
return;
}
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(feature = "experimental_metadata_attributes")]
let normalized_meta = event.normalized_metadata();

Expand Down
2 changes: 2 additions & 0 deletions opentelemetry-sdk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ url = { workspace = true, optional = true }
tokio = { workspace = true, features = ["rt", "time"], optional = true }
tokio-stream = { workspace = true, optional = true }
http = { workspace = true, optional = true }
tracing = {workspace = true, optional = true}

[package.metadata.docs.rs]
all-features = true
Expand All @@ -51,6 +52,7 @@ testing = ["opentelemetry/testing", "trace", "metrics", "logs", "rt-async-std",
rt-tokio = ["tokio", "tokio-stream"]
rt-tokio-current-thread = ["tokio", "tokio-stream"]
rt-async-std = ["async-std"]
experimental-internal-debugging = ["opentelemetry/experimental-internal-debugging", "tracing"]

[[bench]]
name = "context"
Expand Down
67 changes: 67 additions & 0 deletions opentelemetry-sdk/src/logs/log_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use opentelemetry::{
trace::TraceContextExt,
Context, InstrumentationLibrary,
};
use opentelemetry::{otel_debug, otel_info, otel_target, otel_warn};

#[cfg(feature = "logs_level_enabled")]
use opentelemetry::logs::Severity;
Expand Down Expand Up @@ -49,6 +50,14 @@ impl opentelemetry::logs::LoggerProvider for LoggerProvider {
attributes: Option<Vec<opentelemetry::KeyValue>>,
) -> Logger {
let name = name.into();
otel_info!(
target: otel_target!("logs"),
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
name: "logger_versioned_creation",
instr_lib_name= name,
version = version.as_deref(),
schema_url = schema_url.as_deref(),
attributes = attributes
);

let component_name = if name.is_empty() {
Cow::Borrowed(DEFAULT_COMPONENT_NAME)
Expand All @@ -72,6 +81,11 @@ impl opentelemetry::logs::LoggerProvider for LoggerProvider {
}

fn library_logger(&self, library: Arc<InstrumentationLibrary>) -> Self::Logger {
otel_info!(
target: otel_target!("logs"),
name: "logger_library_logger_creation",
library= library
);
// If the provider is shutdown, new logger will refer a no-op logger provider.
if self.is_shutdown.load(Ordering::Relaxed) {
return Logger::new(library, NOOP_LOGGER_PROVIDER.clone());
Expand All @@ -83,19 +97,37 @@ impl opentelemetry::logs::LoggerProvider for LoggerProvider {
impl LoggerProvider {
/// Create a new `LoggerProvider` builder.
pub fn builder() -> Builder {
otel_info!(
target: otel_target!("logs"),
name: "logger_provider_builder"
);
Builder::default()
}

pub(crate) fn log_processors(&self) -> &[Box<dyn LogProcessor>] {
otel_debug!(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be good enough to get the processor count information just once during LoggerProvider build. Is there any value in repeatedly logging it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to unblock and merge this PR quickly, I think its best to remove all internal logging that are not error/warning, and we can selective bring them one by one in future PRs.

target: otel_target!("logs"),
name: "logger_provider_log_processors",
processor_count= self.inner.processors.len()
);
&self.inner.processors
}

pub(crate) fn resource(&self) -> &Resource {
otel_debug!(
target: otel_target!("logs"),
name: "logger_provider_resource",
resource_count= self.inner.resource.len()
);
&self.inner.resource
}

/// Force flush all remaining logs in log processors and return results.
pub fn force_flush(&self) -> Vec<LogResult<()>> {
otel_info!(
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
target: otel_target!("logs"),
name: "logger_provider_force_flush"
);
self.log_processors()
.iter()
.map(|processor| processor.force_flush())
Expand All @@ -104,6 +136,10 @@ impl LoggerProvider {

/// Shuts down this `LoggerProvider`
pub fn shutdown(&self) -> LogResult<()> {
otel_info!(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be logged only in case of a successful shutdown? Only in this case:

if errs.is_empty() {
    Ok(())
}

target: otel_target!("logs"),
name: "logger_provider_shutdown"
);
if self
.is_shutdown
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
Expand All @@ -114,6 +150,11 @@ impl LoggerProvider {
let mut errs = vec![];
for processor in &self.inner.processors {
if let Err(err) = processor.shutdown() {
otel_warn!(
target: otel_target!("logs"),
name: "logger_provider_shutdown_error",
error= err
);
errs.push(err);
}
}
Expand All @@ -124,6 +165,10 @@ impl LoggerProvider {
Err(LogError::Other(format!("{errs:?}").into()))
}
} else {
otel_warn!(
target: otel_target!("logs"),
name: "logger_provider_already_shutdown"
);
Err(LogError::Other("logger provider already shut down".into()))
}
}
Expand Down Expand Up @@ -203,6 +248,12 @@ impl Builder {
for processor in logger_provider.log_processors() {
processor.set_resource(logger_provider.resource());
}
otel_debug!(
target: otel_target!("logs"),
name: "logger_provider_build",
resource_count = logger_provider.resource().len(),
processor_count = logger_provider.log_processors().len()
);
logger_provider
}
}
Expand All @@ -221,6 +272,10 @@ impl Logger {
instrumentation_lib: Arc<InstrumentationLibrary>,
provider: LoggerProvider,
) -> Self {
otel_info!(
target: otel_target!("logs"),
name: "logger_new"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add some identification information for the logger such as name, version etc.

);
Logger {
instrumentation_lib,
provider,
Expand All @@ -247,6 +302,11 @@ impl opentelemetry::logs::Logger for Logger {

/// Emit a `LogRecord`.
fn emit(&self, mut record: Self::LogRecord) {
otel_debug!(
target: otel_target!("logs"),
name: "log_record_emit_start",
record = record
);
let provider = self.provider();
let processors = provider.log_processors();
let trace_context = Context::map_current(|cx| {
Expand Down Expand Up @@ -280,6 +340,13 @@ impl opentelemetry::logs::Logger for Logger {
self.instrumentation_library().name.as_ref(),
);
}
otel_debug!(
target: otel_target!("logs"),
name: "log_record_event_enabled_check",
enabled_status = enabled,
level = level,
log_target = target
);
enabled
}
}
Expand Down
Loading
Loading