From 7f1b57c65e0c681cb0176be9003020408854a435 Mon Sep 17 00:00:00 2001 From: Zhongyang Wu Date: Mon, 5 Oct 2020 20:23:54 -0400 Subject: [PATCH 1/3] [exporter] Export instrument library information. Add default instrument version for Jaeger. (#243) According to spec https://github.com/open-telemetry/opentelemetry-specification/pull/800 and https://github.com/open-telemetry/opentelemetry-specification/pull/801, We MUST export instrumentation library information for Jaeger and Zipkin exporter. --- opentelemetry-jaeger/src/lib.rs | 22 +++++++++++++++++++++- opentelemetry-zipkin/src/model/mod.rs | 21 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index f9a8d587b6b..18d5f4f15af 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -142,6 +142,7 @@ //! ``` #![deny(missing_docs, unreachable_pub, missing_debug_implementations)] #![cfg_attr(test, deny(warnings))] + mod agent; #[cfg(feature = "collector_client")] mod collector; @@ -176,6 +177,12 @@ const DEFAULT_SERVICE_NAME: &str = "OpenTelemetry"; /// Default agent endpoint if none is provided const DEFAULT_AGENT_ENDPOINT: &str = "127.0.0.1:6831"; +/// Instrument Library name MUST be reported in Jaeger Span tags with the following key +const INSTRUMENTATION_LIBRARY_NAME: &str = "otel.library.name"; + +/// Instrument Library version MUST be reported in Jaeger Span tags with the following key +const INSTRUMENTATION_LIBRARY_VERSION: &str = "otel.library.version"; + /// Create a new Jaeger exporter pipeline builder. pub fn new_pipeline() -> PipelineBuilder { PipelineBuilder::default() @@ -348,7 +355,8 @@ impl PipelineBuilder { /// Install a Jaeger pipeline with the recommended defaults. pub fn install(self) -> Result<(sdk::Tracer, Uninstall), Box> { let tracer_provider = self.build()?; - let tracer = tracer_provider.get_tracer("opentelemetry-jaeger", None); + let tracer = + tracer_provider.get_tracer("opentelemetry-jaeger", Some(env!("CARGO_PKG_VERSION"))); let provider_guard = global::set_tracer_provider(tracer_provider); @@ -523,6 +531,18 @@ fn build_span_tags(span_data: &Arc) -> Option> }) .collect::>(); + // Set instrument library tags + tags.push( + api::KeyValue::new( + INSTRUMENTATION_LIBRARY_NAME, + span_data.instrumentation_lib.name, + ) + .into(), + ); + if let Some(version) = span_data.instrumentation_lib.version { + tags.push(api::KeyValue::new(INSTRUMENTATION_LIBRARY_VERSION, version).into()) + } + // Ensure error status is set if span_data.status_code != api::StatusCode::OK && !user_overrides.error { tags.push(api::Key::new(ERROR).bool(true).into()) diff --git a/opentelemetry-zipkin/src/model/mod.rs b/opentelemetry-zipkin/src/model/mod.rs index 23b452d2b57..85b9dd55ce2 100644 --- a/opentelemetry-zipkin/src/model/mod.rs +++ b/opentelemetry-zipkin/src/model/mod.rs @@ -9,6 +9,12 @@ pub(crate) mod span; use endpoint::Endpoint; +/// Instrument Library name MUST be reported in Jaeger Span tags with the following key +const INSTRUMENTATION_LIBRARY_NAME: &str = "otel.library.name"; + +/// Instrument Library version MUST be reported in Jaeger Span tags with the following key +const INSTRUMENTATION_LIBRARY_VERSION: &str = "otel.library.version"; + /// Converts `api::Event` into an `annotation::Annotation` impl Into for api::Event { fn into(self) -> annotation::Annotation { @@ -81,6 +87,21 @@ pub(crate) fn into_zipkin_span( .resource .iter() .map(|(k, v)| api::KeyValue::new(k.clone(), v.clone())), + ) + .chain( + [ + ( + INSTRUMENTATION_LIBRARY_NAME, + Some(span_data.instrumentation_lib.name), + ), + ( + INSTRUMENTATION_LIBRARY_VERSION, + span_data.instrumentation_lib.version, + ), + ] + .iter() + .filter(|(_, val)| val.is_some()) + .map(|(k, v)| api::KeyValue::new(<&str>::clone(k), v.unwrap_or(""))), ), ); From 90a6e3602a1b0ae1329fefaea8b9af49c3c2fd36 Mon Sep 17 00:00:00 2001 From: Julian Tescher Date: Mon, 5 Oct 2020 18:09:53 -0700 Subject: [PATCH 2/3] Use latest baggage header (#246) --- src/api/baggage/mod.rs | 4 ++-- src/api/baggage/propagation.rs | 2 +- src/api/context/propagation/composite_propagator.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/api/baggage/mod.rs b/src/api/baggage/mod.rs index b21b7a93c71..bf88de1731b 100644 --- a/src/api/baggage/mod.rs +++ b/src/api/baggage/mod.rs @@ -20,7 +20,7 @@ //! //! // Example baggage value passed in externally via http headers //! let mut headers = HashMap::new(); -//! headers.insert("otcorrelations".to_string(), "user_id=1".to_string()); +//! headers.insert("baggage".to_string(), "user_id=1".to_string()); //! //! let propagator = BaggagePropagator::new(); //! // can extract from any type that impls `Extractor`, usually an HTTP header map @@ -37,7 +37,7 @@ //! // Inject aggage into http request //! propagator.inject_context(&cx_with_additions, &mut headers); //! -//! let header_value = headers.get("otcorrelations").expect("header is injected"); +//! let header_value = headers.get("baggage").expect("header is injected"); //! assert!(header_value.contains("user_id=1"), "still contains previous name / value"); //! assert!(header_value.contains("server_id=42"), "contains new name / value pair"); //! ``` diff --git a/src/api/baggage/propagation.rs b/src/api/baggage/propagation.rs index 276cac1b53f..d9c66e4f49c 100644 --- a/src/api/baggage/propagation.rs +++ b/src/api/baggage/propagation.rs @@ -4,7 +4,7 @@ use crate::api::{self, Context, KeyValue}; use percent_encoding::{percent_decode_str, utf8_percent_encode, AsciiSet, CONTROLS}; use std::iter; -static BAGGAGE_HEADER: &str = "otcorrelations"; +static BAGGAGE_HEADER: &str = "baggage"; const FRAGMENT: &AsciiSet = &CONTROLS.add(b' ').add(b'"').add(b';').add(b',').add(b'='); lazy_static::lazy_static! { diff --git a/src/api/context/propagation/composite_propagator.rs b/src/api/context/propagation/composite_propagator.rs index 1d8f4920a7e..85968c18b50 100644 --- a/src/api/context/propagation/composite_propagator.rs +++ b/src/api/context/propagation/composite_propagator.rs @@ -42,8 +42,8 @@ use std::fmt::Debug; /// .with_baggage(vec![KeyValue::new("test", "example")]), /// &mut injector); /// -/// // The injector now has both `otcorrelations` and `traceparent` headers -/// assert!(injector.get("otcorrelations").is_some()); +/// // The injector now has both `baggage` and `traceparent` headers +/// assert!(injector.get("baggage").is_some()); /// assert!(injector.get("traceparent").is_some()); /// ``` #[derive(Debug)] From 503503caee4e583bba6056bd02da8e0c7cb3dd94 Mon Sep 17 00:00:00 2001 From: Julian Tescher Date: Mon, 5 Oct 2020 19:03:33 -0700 Subject: [PATCH 3/3] Use latest sampler decision names (#247) --- src/sdk/trace/sampler.rs | 22 +++++++++++----------- src/sdk/trace/tracer.rs | 8 ++++---- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/sdk/trace/sampler.rs b/src/sdk/trace/sampler.rs index c2cba46f956..c91a3253544 100644 --- a/src/sdk/trace/sampler.rs +++ b/src/sdk/trace/sampler.rs @@ -73,11 +73,11 @@ pub struct SamplingResult { pub enum SamplingDecision { /// `is_recording() == false`, span will not be recorded and all events and /// attributes will be dropped. - NotRecord, + Drop, /// `is_recording() == true`, but `Sampled` flag MUST NOT be set. - Record, + RecordOnly, /// `is_recording() == true` AND `Sampled` flag` MUST be set. - RecordAndSampled, + RecordAndSample, } /// Sampling options @@ -107,9 +107,9 @@ impl ShouldSample for Sampler { ) -> SamplingResult { let decision = match self { // Always sample the trace - Sampler::AlwaysOn => SamplingDecision::RecordAndSampled, + Sampler::AlwaysOn => SamplingDecision::RecordAndSample, // Never sample the trace - Sampler::AlwaysOff => SamplingDecision::NotRecord, + Sampler::AlwaysOff => SamplingDecision::Drop, // The parent decision if sampled; otherwise the decision of delegate_sampler Sampler::ParentBased(delegate_sampler) => parent_context.map_or( delegate_sampler @@ -117,25 +117,25 @@ impl ShouldSample for Sampler { .decision, |ctx| { if ctx.is_sampled() { - SamplingDecision::RecordAndSampled + SamplingDecision::RecordAndSample } else { - SamplingDecision::NotRecord + SamplingDecision::Drop } }, ), // Probabilistically sample the trace. Sampler::TraceIdRatioBased(prob) => { if *prob >= 1.0 { - SamplingDecision::RecordAndSampled + SamplingDecision::RecordAndSample } else { let prob_upper_bound = (prob.max(0.0) * (1u64 << 63) as f64) as u64; // The trace_id is already randomly generated, so we don't need a new one here let rnd_from_trace_id = (trace_id.to_u128() as u64) >> 1; if rnd_from_trace_id < prob_upper_bound { - SamplingDecision::RecordAndSampled + SamplingDecision::RecordAndSample } else { - SamplingDecision::NotRecord + SamplingDecision::Drop } } } @@ -241,7 +241,7 @@ mod tests { &[], ) .decision - == SamplingDecision::RecordAndSampled + == SamplingDecision::RecordAndSample { sampled += 1; } diff --git a/src/sdk/trace/tracer.rs b/src/sdk/trace/tracer.rs index e922cea1b90..532a8721d33 100644 --- a/src/sdk/trace/tracer.rs +++ b/src/sdk/trace/tracer.rs @@ -81,11 +81,11 @@ impl Tracer { ) -> Option<(u8, Vec, TraceState)> { match sampling_result { sdk::SamplingResult { - decision: sdk::SamplingDecision::NotRecord, + decision: sdk::SamplingDecision::Drop, .. } => None, sdk::SamplingResult { - decision: sdk::SamplingDecision::Record, + decision: sdk::SamplingDecision::RecordOnly, attributes, trace_state, } => { @@ -97,7 +97,7 @@ impl Tracer { )) } sdk::SamplingResult { - decision: sdk::SamplingDecision::RecordAndSampled, + decision: sdk::SamplingDecision::RecordAndSample, attributes, trace_state, } => { @@ -302,7 +302,7 @@ mod tests { ) -> SamplingResult { let trace_state = parent_context.unwrap().trace_state().clone(); SamplingResult { - decision: SamplingDecision::RecordAndSampled, + decision: SamplingDecision::RecordAndSample, attributes: Vec::new(), trace_state: trace_state.insert("foo".into(), "notbar".into()).unwrap(), }