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

opentelemetry: add more comments to example #2140

Merged
merged 3 commits into from
Jun 6, 2022

Conversation

bryangarza
Copy link
Member

This patch adds a bit more context around why we are creating a smaller
scope for the spans, and also what happens when we call
global::shutdown_tracer_provider() (that comment was copied from
therust-opentelemetry repo).

This patch adds a bit more context around why we are creating a smaller
scope for the spans, and also what happens when we call
`global::shutdown_tracer_provider()` (that comment was copied from
the`rust-opentelemetry` repo).
@bryangarza bryangarza requested review from hawkw, davidbarsky and a team as code owners May 25, 2022 15:54
@bryangarza bryangarza removed the request for review from hawkw May 25, 2022 15:54
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

This looks good to me. I'm not totally sure why the RustDoc CI build is failing --- it builds fine on my machine.

Also, should we consider adding similar code/comments to the examples in the README and RustDoc for tracing-opentelemetry? Here:

fn main() {
// Install a new OpenTelemetry trace pipeline
let (tracer, _uninstall) = stdout::new_pipeline().install();
// Create a tracing subscriber with the configured tracer
let telemetry = tracing_opentelemetry::subscriber().with_tracer(tracer);
// Use the tracing subscriber `Registry`, or any other subscriber
// that impls `LookupSpan`
let collector = Registry::default().with(telemetry);
// Trace executed code
tracing::collect::with_default(collector, || {
// Spans will be sent to the configured OpenTelemetry exporter
let root = span!(tracing::Level::TRACE, "app_start", work_units = 2);
let _enter = root.enter();
error!("This event will be logged in the root span.");
});
}
and here:
//! use opentelemetry::sdk::export::trace::stdout;
//! use tracing::{error, span};
//! use tracing_subscriber::subscribe::CollectExt;
//! use tracing_subscriber::Registry;
//!
//! // Create a new OpenTelemetry pipeline
//! let tracer = stdout::new_pipeline().install_simple();
//!
//! // Create a tracing layer with the configured tracer
//! let telemetry = tracing_opentelemetry::subscriber().with_tracer(tracer);
//!
//! // Use the tracing subscriber `Registry`, or any other subscriber
//! // that impls `LookupSpan`
//! let subscriber = Registry::default().with(telemetry);
//!
//! // Trace executed code
//! tracing::collect::with_default(subscriber, || {
//! // Spans will be sent to the configured OpenTelemetry exporter
//! let root = span!(tracing::Level::TRACE, "app_start", work_units = 2);
//! let _enter = root.enter();
//!
//! error!("This event will be logged in the root span.");
//! });

@hawkw hawkw enabled auto-merge (squash) June 6, 2022 18:41
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

looks good to me now that the CI build passes! would be nice to also add to the RustDoc examples, like i mentioned in #2140 (review)

@hawkw hawkw merged commit 388fff8 into tokio-rs:master Jun 6, 2022
@bryangarza bryangarza deleted the otel-update-example-comments branch June 7, 2022 14:50
hawkw added a commit that referenced this pull request Jun 22, 2022
This patch adds a bit more context around why we are creating a smaller
scope for the spans, and also what happens when we call
`global::shutdown_tracer_provider()` (that comment was copied from
the`rust-opentelemetry` repo).

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jun 22, 2022
This patch adds a bit more context around why we are creating a smaller
scope for the spans, and also what happens when we call
`global::shutdown_tracer_provider()` (that comment was copied from
the`rust-opentelemetry` repo).

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants