-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(otlp-tracing): enable to export traces with grpc export with tracing-otlp and tracing-otlp-protocol arg
#18985
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
Changes from all commits
d029359
b44284e
045e6aa
2300a24
632e481
2d0bdbc
dd5dc7c
af092eb
9c4f13f
319e993
4342317
6333e50
3f5efb2
71db3f0
ea38253
32a938f
2c28c20
98a9b58
d832e0d
679c119
311d7d4
39406c2
be76ab5
8a28678
81097fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,10 @@ use reth_optimism_consensus::OpBeaconConsensus; | |
| use reth_optimism_node::{OpExecutorProvider, OpNode}; | ||
| use reth_rpc_server_types::RpcModuleValidator; | ||
| use reth_tracing::{FileWorkerGuard, Layers}; | ||
| use reth_tracing_otlp::OtlpProtocol; | ||
| use std::{fmt, sync::Arc}; | ||
| use tracing::info; | ||
| use url::Url; | ||
|
|
||
| /// A wrapper around a parsed CLI that handles command execution. | ||
| #[derive(Debug)] | ||
|
|
@@ -63,7 +65,8 @@ where | |
| self.cli.logs.log_file_directory.join(chain_spec.chain.to_string()); | ||
| } | ||
|
|
||
| self.init_tracing()?; | ||
| self.init_tracing(&runner)?; | ||
|
|
||
| // Install the prometheus recorder to be sure to record all metrics | ||
| let _ = install_prometheus_recorder(); | ||
|
|
||
|
|
@@ -114,23 +117,52 @@ where | |
| /// Initializes tracing with the configured options. | ||
| /// | ||
| /// If file logging is enabled, this function stores guard to the struct. | ||
| pub fn init_tracing(&mut self) -> Result<()> { | ||
| /// For gRPC OTLP, it requires tokio runtime context. | ||
| pub fn init_tracing(&mut self, runner: &CliRunner) -> Result<()> { | ||
| if self.guard.is_none() { | ||
| let mut layers = self.layers.take().unwrap_or_default(); | ||
|
|
||
| #[cfg(feature = "otlp")] | ||
| if let Some(output_type) = &self.cli.traces.otlp { | ||
| info!(target: "reth::cli", "Starting OTLP tracing export to {:?}", output_type); | ||
| layers.with_span_layer( | ||
| "reth".to_string(), | ||
| output_type.clone(), | ||
| self.cli.traces.otlp_filter.clone(), | ||
| )?; | ||
| { | ||
| self.cli.traces.validate()?; | ||
| if let Some(endpoint) = &self.cli.traces.otlp { | ||
| info!(target: "reth::cli", "Starting OTLP tracing export to {:?}", endpoint); | ||
| self.init_otlp_export(&mut layers, endpoint, runner)?; | ||
| } | ||
| } | ||
|
|
||
| self.guard = self.cli.logs.init_tracing_with_layers(layers)?; | ||
| info!(target: "reth::cli", "Initialized tracing, debug log directory: {}", self.cli.logs.log_file_directory); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Initialize OTLP tracing export based on protocol type. | ||
| /// | ||
| /// For gRPC, `block_on` is required because tonic's channel initialization needs | ||
| /// a tokio runtime context, even though `with_span_layer` itself is not async. | ||
| #[cfg(feature = "otlp")] | ||
| fn init_otlp_export( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure where to put it to avoid the duplicate with op-cli app code
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO we can figure out how to properly dedup tracing + otlp init for OP / eth in a followup |
||
| &self, | ||
| layers: &mut Layers, | ||
| endpoint: &Url, | ||
| runner: &CliRunner, | ||
| ) -> Result<()> { | ||
| let endpoint = endpoint.clone(); | ||
| let protocol = self.cli.traces.protocol; | ||
| let level_filter = self.cli.traces.otlp_filter.clone(); | ||
|
|
||
| match protocol { | ||
| OtlpProtocol::Grpc => { | ||
| runner.block_on(async { | ||
| layers.with_span_layer("reth".to_string(), endpoint, level_filter, protocol) | ||
| })?; | ||
|
Comment on lines
+157
to
+159
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if this isn't an async fn (ie, no await), then why do we need to use the runner and call with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a bit tricky, it's linked to the tonic exporter (there is one per observability event type), and its builder. For example for the span one, when we activate and then in this function, it will call I would say this part There is also probably the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yet I mainly discovered it was needed because of this error
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense, then yeah we can keep this, mind just adding a doc comment explaining this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment in top of |
||
| } | ||
| OtlpProtocol::Http => { | ||
| layers.with_span_layer("reth".to_string(), endpoint, level_filter, protocol)?; | ||
| } | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.