Internal logging code path: Raw logger support #1735
Conversation
…d/tracing_to_otlp_bytes
|
From #1672 (comment) Have discussed with @andborja and I realize that the |
…d/tracing_to_otlp_bytes
|
I removed the changes in crates/config and opentelemetry_client.rs, leaving only the encoding logic, formatting logic used and benchmarks for now. |
…ot affect formatting)
| use tracing::callsite::Identifier; | ||
| use tracing::{Event, Level, Metadata}; |
There was a problem hiding this comment.
Why not our own objects to break this dependency?
There was a problem hiding this comment.
I am not trying to avoid use of Tokio tracing, and I don't think we should. Even when our custom macros use the effect handler directly, they are likely to be Tokio tracing macros.
There was a problem hiding this comment.
This layer of the architecture should not be coupled to external libraries, especially if they are just simple data structures. This is independent of the decision or not to couple another layer to tokio tracing.
| @@ -0,0 +1,114 @@ | |||
| // Copyright The OpenTelemetry Authors | |||
There was a problem hiding this comment.
self_tracing or internal_tracing? The concept used as of now is "internal", not "self"
There was a problem hiding this comment.
I think of self-tracing as including this "raw" logging support for low-level logging about the process itself.
I think of "internal" logging as logging that passes through an internal pipeline starting at ITR.
I'm not against moving combining these two directories, if we decide it makes sense.
|
|
||
| /// The operation to perform on each event within the layer. | ||
| #[derive(Clone, Copy)] | ||
| enum BenchOp { |
There was a problem hiding this comment.
https://github.com/open-telemetry/opentelemetry-rust/pull/3307/changes
I was adding similar benches in OTel Rust to measure the cost of conversions!
| fn on_event(&self, event: &Event<'_>, _ctx: tracing_subscriber::layer::Context<'_, S>) { | ||
| match self.op { | ||
| BenchOp::Encode => { | ||
| for _ in 0..self.iterations { |
There was a problem hiding this comment.
any advantage doing the iteration ourselves, than doing just once, and let criterion do the repeated execution itself?
There was a problem hiding this comment.
I did this I suppose to be certain that overhead from the subscriber isn't being picked up. I'm pleased when I see 100 executions at 8.6xxx and 1000 executions at 85.xxx.
| prints colorized or uncolorized messages on the console. The | ||
| formatting code path in this module is safe for logging in critical | ||
| regions and to be used as a fallback inside other logging handlers. | ||
| It uses no synchronization and depends only on the console. |
There was a problem hiding this comment.
console writing internally uses locks.. not sure if that is worth mentioning here.
There was a problem hiding this comment.
I think the meaning is clear for the users of this low-level interface. If there's user-level configuration that allows configuring "raw" logging of this nature, then it will be important to emphasize this. Can't the OS buffer stdout any way it likes? I assume this is console and OS dependent.
Co-authored-by: Laurent Quérel <[email protected]>
Co-authored-by: Laurent Quérel <[email protected]>
…rrow into jmacd/tracing_to_otlp_bytes
002f436
4 participants
Implements new internal logging configuration option.
Changes the default logging configuration to use internal logging at level INFO. Previously, default logging was disabled.Implements a lightweight Tokio tracing layer to construct partially-encoded OTLP bytes from the Event, forming a struct that can be passed through a channel to a global subscriber.
As the first step, implements "raw logging" directly to the console using simple write! macros and the view object for LogRecord to interpret the partial encoding and print it. The raw logging limits console message size to 4KiB.
Adds a newconfigs/internal-telemetry.yamlto demonstrate this configuration.Adds benchmarks showing good performance, in the 50-200ns range to encode or encode/format: