From 1f2a0753e945c50a4f749a3c02d4ca96326b90ee Mon Sep 17 00:00:00 2001 From: DCjanus Date: Sat, 6 Nov 2021 22:31:44 +0800 Subject: [PATCH 1/4] fix: TraceState::valid_key crashes (#665) --- opentelemetry/src/trace/span_context.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/opentelemetry/src/trace/span_context.rs b/opentelemetry/src/trace/span_context.rs index 8a72605c27..640b54ab51 100644 --- a/opentelemetry/src/trace/span_context.rs +++ b/opentelemetry/src/trace/span_context.rs @@ -210,7 +210,7 @@ impl TraceState { if i == 0 && (!b.is_ascii_lowercase() && !b.is_ascii_digit()) { return false; } else if b == b'@' { - if vendor_start.is_some() || i < key.len() - 14 { + if vendor_start.is_some() || i + 14 < key.len() { return false; } vendor_start = Some(i); @@ -565,6 +565,23 @@ mod tests { } } + #[test] + fn test_trace_state_key() { + let test_data: Vec<(&'static str, bool)> = vec![ + ("123", true), + ("bar", true), + ("foo@bar", true), + ("foo@0123456789abcdef", false), + ("foo@012345678", true), + ("FOO@BAR", false), + ("你好", false), + ]; + + for (key, expected) in test_data { + assert_eq!(TraceState::valid_key(key), expected, "test key: {:?}", key); + } + } + #[test] fn test_trace_state_insert() { let trace_state = TraceState::from_key_value(vec![("foo", "bar")]).unwrap(); From 96e1df98f405abc9e5bf5db9e1c1acfb3b97dc7d Mon Sep 17 00:00:00 2001 From: Zhongyang Wu Date: Tue, 9 Nov 2021 20:55:01 -0500 Subject: [PATCH 2/4] fix: Mapping between Jaeger processes and Otel process. (#663) * fix: Mapping between Jaeger processes and Otel process. The spec maps resource tags to process tags and `service.name` entry in resource to be the service name of the process. #### `Unknown_service` was used as `service.name` process tags even though users provided another name via `with_service_name` function. Service name serves a special purpose for Jaeger as it requires every span to have a service name. In the open telemetry model, `service.name` is just a resource, which should be provided by users or have a default value but users can override it to be empty. To address the difference between those two models. We need to answer the following questions. 1. Should we store the service name within the exporter or store it in resource and extract when exporting? 2. What's the priority of different ways to set the service name? 3. Should we report the `service.name` as a process tags/resource for jaeger spans? In this PR, we implemented the following process 1. We store the service name as part of the `process` field in the exporter 2. The priority of different methods are listed below from high to low - `with_service_name` method in `PipelineBuilder` - passing `service.name` resource from `with_trace_config` method in `PipelineBuilder` - SDK provided service name(it can come from env vars or default `unknown_service`) 3. We append a `service.name` process tag for each Jaeger span #### Duplicate process tags Process tags can be set via `with_tags` function in Jaeger pipeline or by the resource within the trace config(`with_trace_config` function in Jaeger pipeline). We didn't de-duplicate entries from those two functions. For this problem, we should deprecate the `with_tags` method and asks users to use `with_trace_config` and store the process tags/resource only in one place. We can store the process tags in either of the following places - exporter's process tags - trace config resources From a performance standpoint, we should store the tags in the exporter's process tags. Jaeger clients only require a process instance for one batch. If we store the tags as resources, we will copy them for each span in the batch, which will be discarded later in Jaeger clients. However, storing tags in exporters may cause confusion when users install multiple exporters in the tracer provider. Other exporters could get the resource from the trace config while Jaeger exporter will use resources/tags stored in itself. * fix: deprecated methods * docs: add docs around tags, process tags and service name. * fix: make clippy happy --- examples/actix-udp/README.md | 2 +- examples/actix-udp/src/main.rs | 9 +- examples/basic/src/main.rs | 11 +- examples/multiple-span-processors/src/main.rs | 8 +- opentelemetry-jaeger/src/exporter/mod.rs | 235 ++++++++++++------ opentelemetry-jaeger/src/lib.rs | 30 ++- opentelemetry/src/sdk/trace/provider.rs | 38 ++- 7 files changed, 230 insertions(+), 103 deletions(-) diff --git a/examples/actix-udp/README.md b/examples/actix-udp/README.md index 0992ffca7e..5afeeaa41b 100644 --- a/examples/actix-udp/README.md +++ b/examples/actix-udp/README.md @@ -19,5 +19,5 @@ $ firefox http://localhost:16686/ Fire a request: ```bash -curl http://localhost:8088 +curl http://localhost:8080 ``` diff --git a/examples/actix-udp/src/main.rs b/examples/actix-udp/src/main.rs index 70ad2e77e2..02a0348b5c 100644 --- a/examples/actix-udp/src/main.rs +++ b/examples/actix-udp/src/main.rs @@ -12,6 +12,13 @@ fn init_tracer() -> Result { opentelemetry_jaeger::new_pipeline() .with_agent_endpoint("localhost:6831") .with_service_name("trace-udp-demo") + .with_trace_config(opentelemetry::sdk::trace::config().with_resource( + opentelemetry::sdk::Resource::new(vec![ + opentelemetry::KeyValue::new("service.name", "my-service"), // this will not override the trace-udp-demo + opentelemetry::KeyValue::new("service.namespace", "my-namespace"), + opentelemetry::KeyValue::new("exporter", "jaeger"), + ]), + )) .install_simple() } @@ -42,7 +49,7 @@ async fn main() -> std::io::Result<()> { }) .route("/", web::get().to(index)) }) - .bind("127.0.0.1:8088") + .bind("127.0.0.1:8080") .unwrap() .run() .await diff --git a/examples/basic/src/main.rs b/examples/basic/src/main.rs index b9bed86c74..13d1faa8cd 100644 --- a/examples/basic/src/main.rs +++ b/examples/basic/src/main.rs @@ -1,7 +1,8 @@ use futures::stream::{Stream, StreamExt}; use opentelemetry::global; use opentelemetry::global::shutdown_tracer_provider; -use opentelemetry::sdk::{metrics::PushController, trace as sdktrace}; +use opentelemetry::sdk::trace::Config; +use opentelemetry::sdk::{metrics::PushController, trace as sdktrace, Resource}; use opentelemetry::trace::TraceError; use opentelemetry::{ baggage::BaggageExt, @@ -15,10 +16,10 @@ use std::time::Duration; fn init_tracer() -> Result { opentelemetry_jaeger::new_pipeline() .with_service_name("trace-demo") - .with_tags(vec![ - KeyValue::new("exporter", "jaeger"), - KeyValue::new("float", 312.23), - ]) + .with_trace_config(Config::default().with_resource(Resource::new(vec![ + KeyValue::new("service.name", "new_service"), + KeyValue::new("exporter", "otlp-jaeger"), + ]))) .install_batch(opentelemetry::runtime::Tokio) } diff --git a/examples/multiple-span-processors/src/main.rs b/examples/multiple-span-processors/src/main.rs index 5c460e15ad..e0a502124c 100644 --- a/examples/multiple-span-processors/src/main.rs +++ b/examples/multiple-span-processors/src/main.rs @@ -1,6 +1,7 @@ use opentelemetry::global::{self, shutdown_tracer_provider}; use opentelemetry::sdk::export::trace::stdout::Exporter as StdoutExporter; -use opentelemetry::sdk::trace::{BatchSpanProcessor, TracerProvider}; +use opentelemetry::sdk::trace::{BatchSpanProcessor, Config, TracerProvider}; +use opentelemetry::sdk::Resource; use opentelemetry::trace::{mark_span_as_active, TraceError, Tracer}; use opentelemetry::KeyValue; use std::io::stdout; @@ -11,7 +12,10 @@ fn init_tracer() -> Result<(), TraceError> { let jaeger_processor = BatchSpanProcessor::builder( opentelemetry_jaeger::new_pipeline() .with_service_name("trace-demo") - .with_tags(vec![KeyValue::new("exporter", "jaeger")]) + .with_trace_config( + Config::default() + .with_resource(Resource::new(vec![KeyValue::new("exporter", "jaeger")])), + ) .init_async_exporter(opentelemetry::runtime::Tokio)?, opentelemetry::runtime::Tokio, ) diff --git a/opentelemetry-jaeger/src/exporter/mod.rs b/opentelemetry-jaeger/src/exporter/mod.rs index 9c04b26b8e..bf752dc76f 100644 --- a/opentelemetry-jaeger/src/exporter/mod.rs +++ b/opentelemetry-jaeger/src/exporter/mod.rs @@ -33,6 +33,7 @@ use opentelemetry::{ }; #[cfg(feature = "collector_client")] use opentelemetry_http::HttpClient; +use std::collections::HashSet; use std::{ net, time::{Duration, SystemTime}, @@ -48,8 +49,6 @@ use uploader::{AsyncUploader, SyncUploader, Uploader}; not(feature = "isahc_collector_client") ))] use headers::authorization::Credentials; -use opentelemetry::sdk::resource::ResourceDetector; -use opentelemetry::sdk::resource::SdkProvidedResourceDetector; use opentelemetry::sdk::trace::Config; use opentelemetry::sdk::Resource; use std::sync::Arc; @@ -91,18 +90,9 @@ impl trace::SpanExporter for Exporter { /// Export spans to Jaeger async fn export(&mut self, batch: Vec) -> trace::ExportResult { let mut jaeger_spans: Vec = Vec::with_capacity(batch.len()); - let mut process = self.process.clone(); - - for (idx, span) in batch.into_iter().enumerate() { - if idx == 0 { - if let Some(span_process_tags) = build_process_tags(&span) { - if let Some(process_tags) = &mut process.tags { - process_tags.extend(span_process_tags); - } else { - process.tags = Some(span_process_tags.collect()) - } - } - } + let process = self.process.clone(); + + for span in batch.into_iter() { jaeger_spans.push(convert_otel_span_into_jaeger_span( span, self.export_instrumentation_lib, @@ -263,7 +253,16 @@ impl PipelineBuilder { self } - /// Assign the process service tags. + /// Assign the process tags. + /// + /// Note that resource in trace [Config](sdk::trace::Config) is also reported as process tags + /// in jaeger. If there is duplicate tags between resource and tags. Resource's value take + /// priority even if it's empty. + #[deprecated( + since = "0.16.0", + note = "please pass those tags as resource in sdk::trace::Config. Then use with_trace_config \ + method to pass the config. All key value pairs in resources will be reported as process tags" + )] pub fn with_tags>(mut self, tags: T) -> Self { self.tags = Some(tags.into_iter().collect()); self @@ -290,6 +289,22 @@ impl PipelineBuilder { } /// Assign the SDK config for the exporter pipeline. + /// + /// # Examples + /// Set service name via resource. + /// ```rust + /// use opentelemetry_jaeger::PipelineBuilder; + /// use opentelemetry::sdk; + /// use opentelemetry::sdk::Resource; + /// use opentelemetry::KeyValue; + /// + /// let pipeline = PipelineBuilder::default() + /// .with_trace_config( + /// sdk::trace::Config::default() + /// .with_resource(Resource::new(vec![KeyValue::new("service.name", "my-service")])) + /// ); + /// + /// ``` pub fn with_trace_config(self, config: sdk::trace::Config) -> Self { PipelineBuilder { config: Some(config), @@ -325,60 +340,66 @@ impl PipelineBuilder { Ok(tracer) } - // To reduce the overhead of copying service name in every spans. We remove service.name - // from the resource in config. Instead, we store it in process. - // The service name tag will attch to spans when it's exported. - fn build_config_and_process(&mut self) -> (Config, Process) { - let service_name = self.service_name.take(); - if let Some(service_name) = service_name { - let config = if let Some(mut cfg) = self.config.take() { - cfg.resource = cfg.resource.map(|r| { - let without_service_name = r - .iter() - .filter(|(k, _v)| **k != semcov::resource::SERVICE_NAME) - .map(|(k, v)| KeyValue::new(k.clone(), v.clone())) - .collect::>(); - Arc::new(Resource::new(without_service_name)) - }); - cfg - } else { - Config { - resource: Some(Arc::new(Resource::empty())), - ..Default::default() - } - }; - ( - config, - Process { - service_name, - tags: self.tags.take().unwrap_or_default(), - }, - ) + // To reduce the overhead of copying service name in every spans. We convert resource into jaeger tags + // and store them into process. And set the resource in trace config to empty. + // + // There are multiple ways to set the service name. A `service.name` tag will be always added + // to the process tags. + fn build_config_and_process(&mut self, sdk_provided_resource: Resource) -> (Config, Process) { + let (config, resource) = if let Some(mut config) = self.config.take() { + let resource = + if let Some(resource) = config.resource.replace(Arc::new(Resource::empty())) { + sdk_provided_resource.merge(resource) + } else { + sdk_provided_resource + }; + + (config, resource) } else { - let service_name = SdkProvidedResourceDetector - .detect(Duration::from_secs(0)) + (Config::default(), sdk_provided_resource) + }; + + let service_name = self.service_name.clone().unwrap_or_else(|| { + resource .get(semcov::resource::SERVICE_NAME) - .unwrap() - .to_string(); - ( - Config { - // use a empty resource to prevent TracerProvider to assign a service name. - resource: Some(Arc::new(Resource::empty())), - ..Default::default() - }, - Process { - service_name, - tags: self.tags.take().unwrap_or_default(), - }, - ) + .map(|v| v.to_string()) + .unwrap_or_else(|| "unknown_service".to_string()) + }); + + // merge the tags and resource. Resources take priority. + let mut tags = resource + .into_iter() + .filter(|(key, _)| *key != semcov::resource::SERVICE_NAME) + .map(|(key, value)| KeyValue::new(key, value)) + .collect::>(); + + tags.push(KeyValue::new( + semcov::resource::SERVICE_NAME, + service_name.clone(), + )); + + // if users provide key list + if let Some(provided_tags) = self.tags.take() { + let key_set: HashSet = tags + .iter() + .map(|key_value| key_value.key.clone()) + .collect::>(); + for tag in provided_tags.into_iter() { + if !key_set.contains(&tag.key) { + tags.push(tag) + } + } } + + (config, Process { service_name, tags }) } /// Build a configured `sdk::trace::TracerProvider` with a simple span processor. pub fn build_simple(mut self) -> Result { - let (config, process) = self.build_config_and_process(); + let mut builder = sdk::trace::TracerProvider::builder(); + let (config, process) = self.build_config_and_process(builder.sdk_provided_resource()); let exporter = self.init_sync_exporter_with_process(process)?; - let mut builder = sdk::trace::TracerProvider::builder().with_simple_exporter(exporter); + builder = builder.with_simple_exporter(exporter); builder = builder.with_config(config); Ok(builder.build()) @@ -390,10 +411,10 @@ impl PipelineBuilder { mut self, runtime: R, ) -> Result { - let (config, process) = self.build_config_and_process(); + let mut builder = sdk::trace::TracerProvider::builder(); + let (config, process) = self.build_config_and_process(builder.sdk_provided_resource()); let exporter = self.init_async_exporter_with_process(process, runtime.clone())?; - let mut builder = - sdk::trace::TracerProvider::builder().with_batch_exporter(exporter, runtime); + builder = builder.with_batch_exporter(exporter, runtime); builder = builder.with_config(config); Ok(builder.build()) @@ -403,7 +424,8 @@ impl PipelineBuilder { /// /// This is useful if you are manually constructing a pipeline. pub fn init_sync_exporter(mut self) -> Result { - let (_, process) = self.build_config_and_process(); + let builder = sdk::trace::TracerProvider::builder(); + let (_, process) = self.build_config_and_process(builder.sdk_provided_resource()); self.init_sync_exporter_with_process(process) } @@ -425,7 +447,8 @@ impl PipelineBuilder { mut self, runtime: R, ) -> Result { - let (_, process) = self.build_config_and_process(); + let builder = sdk::trace::TracerProvider::builder(); + let (_, process) = self.build_config_and_process(builder.sdk_provided_resource()); self.init_async_exporter_with_process(process, runtime) } @@ -676,20 +699,6 @@ fn convert_otel_span_into_jaeger_span( } } -fn build_process_tags( - span_data: &trace::SpanData, -) -> Option + '_> { - span_data - .resource - .as_ref() - .filter(|resource| !resource.is_empty()) - .map(|resource| { - resource - .iter() - .map(|(k, v)| KeyValue::new(k.clone(), v.clone()).into()) - }) -} - fn build_span_tags( attrs: sdk::trace::EvictedHashMap, instrumentation_lib: Option, @@ -818,7 +827,9 @@ mod collector_client_tests { use crate::exporter::thrift::jaeger::Batch; use crate::new_pipeline; use opentelemetry::runtime::Tokio; + use opentelemetry::sdk::Resource; use opentelemetry::trace::TraceError; + use opentelemetry::KeyValue; mod test_http_client { use async_trait::async_trait; @@ -848,7 +859,9 @@ mod collector_client_tests { let mut builder = new_pipeline() .with_collector_endpoint("localhost:6831") .with_http_client(test_http_client::TestHttpClient); - let (_, process) = builder.build_config_and_process(); + let sdk_provided_resource = + Resource::new(vec![KeyValue::new("service.name", "unknown_service")]); + let (_, process) = builder.build_config_and_process(sdk_provided_resource); let mut uploader = builder.init_async_uploader(Tokio)?; let res = futures::executor::block_on(async { uploader @@ -893,9 +906,12 @@ mod tests { use super::SPAN_KIND; use crate::exporter::thrift::jaeger::Tag; use crate::exporter::{build_span_tags, OTEL_STATUS_CODE, OTEL_STATUS_DESCRIPTION}; - use opentelemetry::sdk::trace::EvictedHashMap; + use opentelemetry::sdk::trace::{Config, EvictedHashMap}; + use opentelemetry::sdk::Resource; use opentelemetry::trace::{SpanKind, StatusCode}; use opentelemetry::KeyValue; + use std::env; + use std::sync::Arc; fn assert_tag_contains(tags: Vec, key: &'static str, expect_val: &'static str) { assert_eq!( @@ -997,4 +1013,59 @@ mod tests { assert_tag_contains(tags.clone(), OTEL_STATUS_CODE, user_status_code.as_str()); assert_tag_contains(tags, OTEL_STATUS_DESCRIPTION, user_status_description); } + + #[test] + fn test_set_service_name() { + let service_name = "halloween_service"; + + // set via builder's service name, it has highest priority + let mut builder = crate::PipelineBuilder::default(); + builder = builder.with_service_name(service_name); + let (_, process) = builder.build_config_and_process(Resource::empty()); + assert_eq!(process.service_name, service_name); + + // make sure the tags in resource are moved to process + builder = crate::PipelineBuilder::default(); + builder = builder.with_service_name(service_name); + builder = builder.with_trace_config( + Config::default() + .with_resource(Resource::new(vec![KeyValue::new("test-key", "test-value")])), + ); + let (config, process) = builder.build_config_and_process(Resource::empty()); + assert_eq!(config.resource, Some(Arc::new(Resource::empty()))); + assert_eq!(process.tags.len(), 2); + + // sdk provided resource can override service name if users didn't provided service name to builder + builder = crate::PipelineBuilder::default(); + let (_, process) = builder.build_config_and_process(Resource::new(vec![KeyValue::new( + "service.name", + "halloween_service", + )])); + assert_eq!(process.service_name, "halloween_service"); + + // users can also provided service.name from config's resource, in this case, it will override the + // sdk provided service name + builder = crate::PipelineBuilder::default(); + builder = builder.with_trace_config(Config::default().with_resource(Resource::new(vec![ + KeyValue::new("service.name", "override_service"), + ]))); + let (_, process) = builder.build_config_and_process(Resource::new(vec![KeyValue::new( + "service.name", + "halloween_service", + )])); + + assert_eq!(process.service_name, "override_service"); + assert_eq!(process.tags.len(), 1); + assert_eq!( + process.tags[0], + KeyValue::new("service.name", "override_service") + ); + + // OTEL_SERVICE_NAME env var also works + env::set_var("OTEL_SERVICE_NAME", "test service"); + builder = crate::PipelineBuilder::default(); + let exporter = builder.init_sync_exporter().unwrap(); + assert_eq!(exporter.process.service_name, "test service"); + env::set_var("OTEL_SERVICE_NAME", "") + } } diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index 687f2ff564..75e3328a85 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -106,6 +106,32 @@ //! Ok(()) //! } //! ``` +//! ## Resource, tags and service name +//! In order to export the spans in different format. opentelemetry uses its own +//! model internally. Most of the jaeger spans' concept can be found in this model. +//! The full list of this mapping can be found in [OpenTelemetry to Jaeger Transformation]. +//! +//! The **process tags** in jaeger spans will be mapped as resource in opentelemetry. You can +//! set it through `OTEL_RESOURCE_ATTRIBUTES` environment variable or using [`PipelineBuilder::with_trace_config`]. +//! +//! Note that to avoid copying data multiple times. Jaeger exporter will uses resource stored in [`Exporter`]. +//! +//! The **tags** in jaeger spans will be mapped as attributes in opentelemetry spans. You can +//! set it through [`set_attribute`] method. +//! +//! Each jaeger span requires a **service name**. This will be mapped as a resource with `service.name` key. +//! You can set it using one of the following methods from highest priority to lowest priority. +//! 1. [`PipelineBuilder::with_service_name`]. +//! 2. include a `service.name` key value pairs when configure resource using [`PipelineBuilder::with_trace_config`]. +//! 3. set the service name as `OTEL_SERVCE_NAME` environment variable. +//! 4. set the `service.name` attributes in `OTEL_RESOURCE_ATTRIBUTES`. +//! 5. if the service name is not provided by the above method. `unknown_service` will be used. +//! +//! Based on the service name, we update/append the `service.name` process tags in jaeger spans. +//! +//! [`set_attribute`]: https://docs.rs/opentelemetry/0.16.0/opentelemetry/trace/trait.Span.html#tymethod.set_attribute +//! +//! [OpenTelemetry to Jaeger Transformation]:https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk_exporters/jaeger.md //! //! ## Kitchen Sink Full Configuration //! @@ -123,7 +149,6 @@ //! let tracer = opentelemetry_jaeger::new_pipeline() //! .with_agent_endpoint("localhost:6831") //! .with_service_name("my_app") -//! .with_tags(vec![KeyValue::new("process_key", "process_value")]) //! .with_max_packet_size(9_216) //! .with_trace_config( //! trace::config() @@ -132,7 +157,8 @@ //! .with_max_events_per_span(64) //! .with_max_attributes_per_span(16) //! .with_max_events_per_span(16) -//! .with_resource(Resource::new(vec![KeyValue::new("key", "value")])), +//! .with_resource(Resource::new(vec![KeyValue::new("key", "value"), +//! KeyValue::new("process_key", "process_value")])), //! ) //! .install_batch(opentelemetry::runtime::Tokio)?; //! diff --git a/opentelemetry/src/sdk/trace/provider.rs b/opentelemetry/src/sdk/trace/provider.rs index f0f067b1a3..1bb0e7f1d9 100644 --- a/opentelemetry/src/sdk/trace/provider.rs +++ b/opentelemetry/src/sdk/trace/provider.rs @@ -100,10 +100,27 @@ impl crate::trace::TracerProvider for TracerProvider { } /// Builder for provider attributes. -#[derive(Default, Debug)] +#[derive(Debug)] pub struct Builder { processors: Vec>, config: sdk::trace::Config, + sdk_provided_resource: Resource, +} + +impl Default for Builder { + fn default() -> Self { + Builder { + processors: Default::default(), + config: Default::default(), + sdk_provided_resource: Resource::from_detectors( + Duration::from_secs(0), + vec![ + Box::new(SdkProvidedResourceDetector), + Box::new(EnvResourceDetector::new()), + ], + ), + } + } } impl Builder { @@ -140,24 +157,25 @@ impl Builder { Builder { config, ..self } } + /// Return the clone of sdk provided resource. + /// + /// See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#sdk-provided-resource-attributes + /// for details. + pub fn sdk_provided_resource(&self) -> Resource { + self.sdk_provided_resource.clone() + } + /// Create a new provider from this configuration. pub fn build(self) -> TracerProvider { let mut config = self.config; - let sdk_provided_resource = Resource::from_detectors( - Duration::from_secs(0), - vec![ - Box::new(SdkProvidedResourceDetector), - Box::new(EnvResourceDetector::new()), - ], - ); config.resource = match config.resource { - None => Some(Arc::new(sdk_provided_resource)), + None => Some(Arc::new(self.sdk_provided_resource)), // User provided resource information has higher priority. Some(resource) => { if resource.is_empty() { None } else { - Some(Arc::new(sdk_provided_resource.merge(resource))) + Some(Arc::new(self.sdk_provided_resource.merge(resource))) } } }; From bd72d1070a22f3c146f29b13502b9d69b2585627 Mon Sep 17 00:00:00 2001 From: Will Brown Date: Sun, 14 Nov 2021 21:46:18 -0500 Subject: [PATCH 3/4] Add with_aggregator_selector for Prometheus (#667) * Allow set AggregatorSelector on prometheus * make fn generic --- opentelemetry-prometheus/src/lib.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/opentelemetry-prometheus/src/lib.rs b/opentelemetry-prometheus/src/lib.rs index 24b1629dfe..b9c41b2d6b 100644 --- a/opentelemetry-prometheus/src/lib.rs +++ b/opentelemetry-prometheus/src/lib.rs @@ -69,7 +69,9 @@ pub use prometheus::{Encoder, TextEncoder}; use opentelemetry::global; use opentelemetry::sdk::{ - export::metrics::{CheckpointSet, ExportKindSelector, Histogram, LastValue, Record, Sum}, + export::metrics::{ + AggregatorSelector, CheckpointSet, ExportKindSelector, Histogram, LastValue, Record, Sum, + }, metrics::{ aggregators::{HistogramAggregator, LastValueAggregator, SumAggregator}, controllers, @@ -149,6 +151,9 @@ pub struct ExporterBuilder { /// /// If not set it will be defaulted to port 9464 port: Option, + + /// The aggregator selector used by the prometheus exporter. + aggegator_selector: Option>, } impl Default for ExporterBuilder { @@ -176,6 +181,7 @@ impl Default for ExporterBuilder { registry: None, host: env::var(ENV_EXPORTER_HOST).ok().filter(|s| !s.is_empty()), port, + aggegator_selector: None, } } } @@ -240,6 +246,17 @@ impl ExporterBuilder { } } + /// Set the aggregation selector for the prometheus exporter + pub fn with_aggregator_selector(self, aggregator_selector: T) -> Self + where + T: AggregatorSelector + Send + Sync + 'static, + { + ExporterBuilder { + aggegator_selector: Some(Box::new(aggregator_selector)), + ..self + } + } + /// Sets up a complete export pipeline with the recommended setup, using the /// recommended selector and standard processor. pub fn try_init(self) -> Result { @@ -251,7 +268,9 @@ impl ExporterBuilder { let default_histogram_boundaries = self .default_histogram_boundaries .unwrap_or_else(|| vec![0.5, 0.9, 0.99]); - let selector = Box::new(Selector::Histogram(default_histogram_boundaries)); + let selector = self + .aggegator_selector + .unwrap_or_else(|| Box::new(Selector::Histogram(default_histogram_boundaries))); let mut controller_builder = controllers::pull(selector, Box::new(EXPORT_KIND_SELECTOR)) .with_cache_period(self.cache_period.unwrap_or(DEFAULT_CACHE_PERIOD)) .with_memory(true); From e606cf1804917d2347a27d4943eddf2b614fdf3f Mon Sep 17 00:00:00 2001 From: Zhongyang Wu Date: Wed, 17 Nov 2021 09:51:29 -0500 Subject: [PATCH 4/4] Ignore the time/chrono related issue and examples in cargo deny (#671) --- Cargo.toml | 1 - deny.toml | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2c93b1d2c8..5a27bec6cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,6 @@ members = [ "opentelemetry-stackdriver", "opentelemetry-zipkin", "opentelemetry-zpages", - "examples/actix-udp", "examples/actix-http", "examples/actix-http-tracing", "examples/actix-udp", diff --git a/deny.toml b/deny.toml index edba673b9c..95ba1a7be5 100644 --- a/deny.toml +++ b/deny.toml @@ -1,3 +1,10 @@ +exclude=[ + "actix-http", + "actix-http-tracing", + "actix-udp", + "actix-udp-example" +] + [licenses] unlicensed = "deny" allow = [ @@ -17,4 +24,11 @@ version = "*" expression = "MIT AND ISC AND OpenSSL" license-files = [ { path = "LICENSE", hash = 0xbd0eed23 } +] + +[advisories] +ignore = [ + # time/chrono problems, have not been a problem in practice, not much we can do at this moment. + "RUSTSEC-2020-0071", + "RUSTSEC-2020-0159" ] \ No newline at end of file