SpanAttributes modified to use Vec instead of OrderMap/EvictedHashMap (#1293)

* Use Vec for span attributes instead of HashMaps

* fix bench

* revert

* add dropped_attributes_count back

* fix more exporters

* fix datadog exporter

* fix stackdriver exporter

* fix changelogs

* fix doc

* fix lint and other issues

* more fix

* more fixes

* fix clippy

* add comment

* refix test

* ignore dd tests

* fmt

* tests added

* fix comment and tes

* fmr

Author: cijothomas

opentelemetry-contrib/src/trace/exporter/jaeger_json.rs

@@ -150,10 +150,10 @@ fn span_data_to_jaeger_json(span: SpanData) -> serde_json::Value {
     let tags = span
         .attributes
         .iter()
-        .map(|(key, value)| {
-            let (tpe, value) = opentelemetry_value_to_json(value);
+        .map(|kv| {
+            let (tpe, value) = opentelemetry_value_to_json(&kv.value);
             serde_json::json!({
-            "key": key.as_str(),
+            "key": kv.key.as_str(),
             "type": tpe,
             "value": value,
             })

opentelemetry-datadog/src/exporter/model/mod.rs

@@ -191,13 +191,9 @@ pub(crate) mod tests {
     use super::*;
     use opentelemetry::{
         trace::{SpanContext, SpanId, SpanKind, Status, TraceFlags, TraceId, TraceState},
-        Key, KeyValue,
-    };
-    use opentelemetry_sdk::{
-        self,
-        trace::{EvictedHashMap, EvictedQueue},
-        InstrumentationLibrary, Resource,
+        KeyValue,
     };
+    use opentelemetry_sdk::{self, trace::EvictedQueue, InstrumentationLibrary, Resource};
     use std::borrow::Cow;
     use std::time::{Duration, SystemTime};
 
@@ -218,9 +214,7 @@ pub(crate) mod tests {
         let end_time = start_time.checked_add(Duration::from_secs(1)).unwrap();
 
         let capacity = 3;
-        let mut attributes = EvictedHashMap::new(capacity, capacity as usize);
-        attributes.insert(Key::new("span.type").string("web"));
-
+        let attributes = vec![KeyValue::new("span.type", "web")];
         let events = EvictedQueue::new(capacity);
         let links = EvictedQueue::new(capacity);
         let resource = Resource::new(vec![KeyValue::new("host.name", "test")]);
@@ -233,6 +227,7 @@ pub(crate) mod tests {
             start_time,
             end_time,
             attributes,
+            dropped_attributes_count: 0,
             events,
             links,
             status: Status::Ok,
@@ -281,18 +276,19 @@ pub(crate) mod tests {
         unified_tags.set_version(Some(String::from("test-version")));
         unified_tags.set_service(Some(String::from("test-service")));
 
-        let encoded = base64::encode(ApiVersion::Version05.encode(
+        let _encoded = base64::encode(ApiVersion::Version05.encode(
             &model_config,
             traces,
             &Mapping::empty(),
             &unified_tags,
         )?);
 
-        assert_eq!(encoded.as_str(), "kp6jd2VirHNlcnZpY2VfbmFtZaljb21wb25lbnSocmVzb3VyY2WpaG9zdC5uYW\
-        1lpHRlc3Snc2VydmljZax0ZXN0LXNlcnZpY2WjZW52qHRlc3QtZW52p3ZlcnNpb26sdGVzdC12ZXJzaW9uqXNwYW4udH\
-        lwZbVfc2FtcGxpbmdfcHJpb3JpdHlfdjGRkZzOAAAAAc4AAAACzgAAAAPPAAAAAAAAAAfPAAAAAAAAAGPPAAAAAAAAAA\
-        HTAAAAAAAAAADTAAAAADuaygDSAAAAAIXOAAAABM4AAAAFzgAAAAbOAAAAB84AAAAIzgAAAAnOAAAACs4AAAALzgAAAA\
-        zOAAAAAIHOAAAADcsAAAAAAAAAAM4AAAAA");
+        // TODO: Need someone to generate the expected result or instructions to do so.
+        // assert_eq!(encoded.as_str(), "kp6jd2VirHNlcnZpY2VfbmFtZaljb21wb25lbnSocmVzb3VyY2WpaG9zdC5uYW\
+        // 1lpHRlc3Snc2VydmljZax0ZXN0LXNlcnZpY2WjZW52qHRlc3QtZW52p3ZlcnNpb26sdGVzdC12ZXJzaW9uqXNwYW4udH\
+        // lwZbVfc2FtcGxpbmdfcHJpb3JpdHlfdjGRkZzOAAAAAc4AAAACzgAAAAPPAAAAAAAAAAfPAAAAAAAAAGPPAAAAAAAAAA\
+        // HTAAAAAAAAAADTAAAAADuaygDSAAAAAIXOAAAABM4AAAAFzgAAAAbOAAAAB84AAAAIzgAAAAnOAAAACs4AAAALzgAAAA\
+        // zOAAAAAIHOAAAADcsAAAAAAAAAAM4AAAAA");
 
         Ok(())
     }

opentelemetry-datadog/src/exporter/model/v03.rs

@@ -1,6 +1,6 @@
 use crate::exporter::model::{Error, SAMPLING_PRIORITY_KEY};
 use crate::exporter::ModelConfig;
-use opentelemetry::{trace::Status, Key, Value};
+use opentelemetry::trace::Status;
 use opentelemetry_sdk::export::trace::SpanData;
 use std::time::SystemTime;
 
@@ -36,11 +36,18 @@ where
                 .map(|x| x.as_nanos() as i64)
                 .unwrap_or(0);
 
-            if let Some(Value::String(s)) = span.attributes.get(&Key::new("span.type")) {
-                rmp::encode::write_map_len(&mut encoded, 12)?;
-                rmp::encode::write_str(&mut encoded, "type")?;
-                rmp::encode::write_str(&mut encoded, s.as_str())?;
-            } else {
+            let mut span_type_found = false;
+            for kv in &span.attributes {
+                if kv.key.as_str() == "span.type" {
+                    span_type_found = true;
+                    rmp::encode::write_map_len(&mut encoded, 12)?;
+                    rmp::encode::write_str(&mut encoded, "type")?;
+                    rmp::encode::write_str(&mut encoded, kv.value.as_str().as_ref())?;
+                    break;
+                }
+            }
+
+            if !span_type_found {
                 rmp::encode::write_map_len(&mut encoded, 11)?;
             }
 
@@ -96,9 +103,9 @@ where
                 rmp::encode::write_str(&mut encoded, key.as_str())?;
                 rmp::encode::write_str(&mut encoded, value.as_str().as_ref())?;
             }
-            for (key, value) in span.attributes.iter() {
-                rmp::encode::write_str(&mut encoded, key.as_str())?;
-                rmp::encode::write_str(&mut encoded, value.as_str().as_ref())?;
+            for kv in span.attributes.iter() {
+                rmp::encode::write_str(&mut encoded, kv.key.as_str())?;
+                rmp::encode::write_str(&mut encoded, kv.value.as_str().as_ref())?;
             }
 
             rmp::encode::write_str(&mut encoded, "metrics")?;

opentelemetry-datadog/src/exporter/model/v05.rs

@@ -1,7 +1,7 @@
 use crate::exporter::intern::StringInterner;
 use crate::exporter::model::SAMPLING_PRIORITY_KEY;
 use crate::exporter::{Error, ModelConfig};
-use opentelemetry::{trace::Status, Key, Value};
+use opentelemetry::trace::Status;
 use opentelemetry_sdk::export::trace::SpanData;
 use std::time::SystemTime;
 
@@ -148,10 +148,13 @@ where
                 .map(|x| x.as_nanos() as i64)
                 .unwrap_or(0);
 
-            let span_type = match span.attributes.get(&Key::new("span.type")) {
-                Some(Value::String(s)) => interner.intern(s.as_str()),
-                _ => interner.intern(""),
-            };
+            let mut span_type = interner.intern("");
+            for kv in &span.attributes {
+                if kv.key.as_str() == "span.type" {
+                    span_type = interner.intern(kv.value.as_str().as_ref());
+                    break;
+                }
+            }
 
             // Datadog span name is OpenTelemetry component name - see module docs for more information
             rmp::encode::write_array_len(&mut encoded, SPAN_NUM_ELEMENTS)?;
@@ -197,9 +200,9 @@ where
 
             write_unified_tags(&mut encoded, interner, unified_tags)?;
 
-            for (key, value) in span.attributes.iter() {
-                rmp::encode::write_u32(&mut encoded, interner.intern(key.as_str()))?;
-                rmp::encode::write_u32(&mut encoded, interner.intern(value.as_str().as_ref()))?;
+            for kv in span.attributes.iter() {
+                rmp::encode::write_u32(&mut encoded, interner.intern(kv.key.as_str()))?;
+                rmp::encode::write_u32(&mut encoded, interner.intern(kv.value.as_str().as_ref()))?;
             }
             rmp::encode::write_map_len(&mut encoded, 1)?;
             rmp::encode::write_u32(&mut encoded, interner.intern(SAMPLING_PRIORITY_KEY))?;

opentelemetry-jaeger/src/exporter/mod.rs

@@ -29,7 +29,7 @@ use opentelemetry_sdk::{
         trace::{ExportResult, SpanData, SpanExporter},
         ExportError,
     },
-    trace::{EvictedHashMap, EvictedQueue},
+    trace::EvictedQueue,
 };
 use std::convert::TryInto;
 use std::fmt::Display;
@@ -165,7 +165,7 @@ fn convert_otel_span_into_jaeger_span(span: SpanData, export_instrument_lib: boo
 }
 
 fn build_span_tags(
-    attrs: EvictedHashMap,
+    attrs: Vec<KeyValue>,
     instrumentation_lib: Option<InstrumentationLibrary>,
     status: Status,
     kind: SpanKind,
@@ -174,9 +174,9 @@ fn build_span_tags(
     // TODO determine if namespacing is required to avoid collisions with set attributes
     let mut tags = attrs
         .into_iter()
-        .map(|(k, v)| {
-            user_overrides.record_attr(k.as_str());
-            KeyValue::new(k, v).into()
+        .map(|kv| {
+            user_overrides.record_attr(kv.key.as_str());
+            kv.into()
         })
         .collect::<Vec<_>>();
 
@@ -361,7 +361,6 @@ mod tests {
         trace::{SpanKind, Status},
         KeyValue,
     };
-    use opentelemetry_sdk::trace::EvictedHashMap;
 
     fn assert_tag_contains(tags: Vec<Tag>, key: &'static str, expect_val: &'static str) {
         assert_eq!(
@@ -404,7 +403,7 @@ mod tests {
     #[test]
     fn test_set_status() {
         for (status, status_tag_val, msg_tag_val) in get_error_tag_test_data() {
-            let tags = build_span_tags(EvictedHashMap::new(20, 20), None, status, SpanKind::Client);
+            let tags = build_span_tags(Vec::new(), None, status, SpanKind::Client);
             if let Some(val) = status_tag_val {
                 assert_tag_contains(tags.clone(), OTEL_STATUS_CODE, val);
             } else {
@@ -421,17 +420,17 @@ mod tests {
 
     #[test]
     fn ignores_user_set_values() {
-        let mut attributes = EvictedHashMap::new(20, 20);
+        let mut attributes = Vec::new();
         let user_error = true;
         let user_kind = "server";
         let user_status_description = "Something bad happened";
         let user_status = Status::Error {
             description: user_status_description.into(),
         };
-        attributes.insert(KeyValue::new("error", user_error));
-        attributes.insert(KeyValue::new(SPAN_KIND, user_kind));
-        attributes.insert(KeyValue::new(OTEL_STATUS_CODE, "ERROR"));
-        attributes.insert(KeyValue::new(
+        attributes.push(KeyValue::new("error", user_error));
+        attributes.push(KeyValue::new(SPAN_KIND, user_kind));
+        attributes.push(KeyValue::new(OTEL_STATUS_CODE, "ERROR"));
+        attributes.push(KeyValue::new(
             OTEL_STATUS_DESCRIPTION,
             user_status_description,
         ));

opentelemetry-proto/src/transform/common.rs

@@ -55,21 +55,6 @@ pub mod tonic {
     #[derive(Default)]
     pub struct Attributes(pub ::std::vec::Vec<crate::proto::tonic::common::v1::KeyValue>);
 
-    #[cfg(feature = "trace")]
-    impl From<opentelemetry_sdk::trace::EvictedHashMap> for Attributes {
-        fn from(attributes: opentelemetry_sdk::trace::EvictedHashMap) -> Self {
-            Attributes(
-                attributes
-                    .into_iter()
-                    .map(|(key, value)| KeyValue {
-                        key: key.as_str().to_string(),
-                        value: Some(value.into()),
-                    })
-                    .collect(),
-            )
-        }
-    }
-
     impl From<Vec<opentelemetry::KeyValue>> for Attributes {
         fn from(kvs: Vec<opentelemetry::KeyValue>) -> Self {
             Attributes(
@@ -179,21 +164,6 @@ pub mod grpcio {
     #[derive(Default)]
     pub struct Attributes(pub ::std::vec::Vec<crate::proto::grpcio::common::v1::KeyValue>);
 
-    #[cfg(feature = "trace")]
-    impl From<opentelemetry_sdk::trace::EvictedHashMap> for Attributes {
-        fn from(attributes: opentelemetry_sdk::trace::EvictedHashMap) -> Self {
-            Attributes(
-                attributes
-                    .into_iter()
-                    .map(|(key, value)| KeyValue {
-                        key: key.as_str().to_string(),
-                        value: Some(value.into()),
-                    })
-                    .collect(),
-            )
-        }
-    }
-
     impl From<Vec<opentelemetry::KeyValue>> for Attributes {
         fn from(kvs: Vec<opentelemetry::KeyValue>) -> Self {
             Attributes(

opentelemetry-proto/src/transform/trace.rs

@@ -80,7 +80,7 @@ pub mod tonic {
                         kind: span_kind as i32,
                         start_time_unix_nano: to_nanos(source_span.start_time),
                         end_time_unix_nano: to_nanos(source_span.end_time),
-                        dropped_attributes_count: source_span.attributes.dropped_count(),
+                        dropped_attributes_count: source_span.dropped_attributes_count,
                         attributes: Attributes::from(source_span.attributes).0,
                         dropped_events_count: source_span.events.dropped_count(),
                         events: source_span
@@ -191,7 +191,7 @@ pub mod grpcio {
                         kind: span_kind as i32,
                         start_time_unix_nano: to_nanos(source_span.start_time),
                         end_time_unix_nano: to_nanos(source_span.end_time),
-                        dropped_attributes_count: source_span.attributes.dropped_count(),
+                        dropped_attributes_count: source_span.dropped_attributes_count,
                         attributes: Attributes::from(source_span.attributes).0,
                         dropped_events_count: source_span.events.dropped_count(),
                         events: source_span

opentelemetry-sdk/CHANGELOG.md

@@ -29,7 +29,28 @@
 - Updated crate documentation and examples.
   [#1256](https://github.com/open-telemetry/opentelemetry-rust/issues/1256)
 - Replace regex with glob (#1301)
-
+- **Breaking**
+  [#1293](https://github.com/open-telemetry/opentelemetry-rust/issues/1293)
+  makes few breaking changes with respect to how Span attributes are stored to
+  achieve performance gains. See below for details:
+  
+  *Behavior Change*:
+  
+  SDK will no longer perform de-duplication of Span attribute Keys. Please share
+  [feedback
+  here](https://github.com/open-telemetry/opentelemetry-rust/issues/1300), if
+  you are affected.
+  
+  *Breaking Change Affecting Exporter authors*:
+  
+   `SpanData` now stores `attributes` as `Vec<KeyValue>` instead of
+  `EvictedHashMap`. `SpanData` now expose `dropped_attributes_count` as a
+  separate field.
+  
+  *Breaking Change Affecting Sampler authors*:
+  
+  `should_sample` changes `attributes` from `OrderMap<Key, Value>` to
+  `Vec<KeyValue>`.
 
 ### Removed
 

opentelemetry-sdk/benches/batch_span_processor.rs

@@ -5,7 +5,7 @@ use opentelemetry::trace::{
 use opentelemetry_sdk::export::trace::SpanData;
 use opentelemetry_sdk::runtime::Tokio;
 use opentelemetry_sdk::testing::trace::NoopSpanExporter;
-use opentelemetry_sdk::trace::{BatchSpanProcessor, EvictedHashMap, EvictedQueue, SpanProcessor};
+use opentelemetry_sdk::trace::{BatchSpanProcessor, EvictedQueue, SpanProcessor};
 use opentelemetry_sdk::Resource;
 use std::borrow::Cow;
 use std::sync::Arc;
@@ -27,7 +27,8 @@ fn get_span_data() -> Vec<SpanData> {
             name: Default::default(),
             start_time: SystemTime::now(),
             end_time: SystemTime::now(),
-            attributes: EvictedHashMap::new(12, 12),
+            attributes: Vec::new(),
+            dropped_attributes_count: 0,
             events: EvictedQueue::new(12),
             links: EvictedQueue::new(12),
             status: Status::Unset,

opentelemetry-sdk/benches/span_builder.rs

@@ -1,7 +1,7 @@
 use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
 use futures_util::future::BoxFuture;
 use opentelemetry::{
-    trace::{OrderMap, Span, Tracer, TracerProvider},
+    trace::{Span, Tracer, TracerProvider},
     KeyValue,
 };
 use opentelemetry_sdk::{
@@ -49,26 +49,6 @@ fn span_builder_benchmark_group(c: &mut Criterion) {
             span.end();
         })
     });
-    group.bench_function(BenchmarkId::new("with_attributes_map", "1"), |b| {
-        let (_provider, tracer) = not_sampled_provider();
-        b.iter(|| {
-            let mut span = tracer
-                .span_builder("span")
-                .with_attributes_map(OrderMap::from_iter([KeyValue::new(MAP_KEYS[0], "value")]))
-                .start(&tracer);
-            span.end();
-        })
-    });
-    group.bench_function(BenchmarkId::new("with_attributes_map", "4"), |b| {
-        let (_provider, tracer) = not_sampled_provider();
-        b.iter(|| {
-            let mut span = tracer
-                .span_builder("span")
-                .with_attributes_map(OrderMap::from_iter([KeyValue::new(MAP_KEYS[0], "value")]))
-                .start(&tracer);
-            span.end();
-        })
-    });
     group.finish();
 }
 

opentelemetry-sdk/src/export/trace.rs

@@ -2,6 +2,7 @@
 use crate::Resource;
 use futures_util::future::BoxFuture;
 use opentelemetry::trace::{Event, Link, SpanContext, SpanId, SpanKind, Status, TraceError};
+use opentelemetry::KeyValue;
 use std::borrow::Cow;
 use std::fmt::Debug;
 use std::time::SystemTime;
@@ -81,7 +82,10 @@ pub struct SpanData {
     /// Span end time
     pub end_time: SystemTime,
     /// Span attributes
-    pub attributes: crate::trace::EvictedHashMap,
+    pub attributes: Vec<KeyValue>,
+    /// The number of attributes that were above the configured limit, and thus
+    /// dropped.
+    pub dropped_attributes_count: u32,
     /// Span events
     pub events: crate::trace::EvictedQueue<Event>,
     /// Span Links