Skip to content

Commit 02e15b2

Browse files
authored
Switch to static resource references (open-telemetry#790)
The spec suggests all spans should have an associated `Resource`. This change switches trace config and span data from `Option<Arc<Resource>>` to `Cow<'static, Resource>` and removes `Config::with_no_resource` to accommodate this requirement.
1 parent 90a70ee commit 02e15b2

File tree

17 files changed

+122
-196
lines changed

17 files changed

+122
-196
lines changed

opentelemetry-datadog/src/exporter/mod.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ pub use model::ApiVersion;
55
pub use model::Error;
66
pub use model::FieldMappingFn;
77

8+
use std::borrow::Cow;
89
use std::fmt::{Debug, Formatter};
910

1011
use crate::exporter::model::FieldMapping;
@@ -165,18 +166,16 @@ impl DatadogPipelineBuilder {
165166
let service_name = self.service_name.take();
166167
if let Some(service_name) = service_name {
167168
let config = if let Some(mut cfg) = self.trace_config.take() {
168-
cfg.resource = cfg.resource.map(|r| {
169-
let without_service_name = r
169+
cfg.resource = Cow::Owned(Resource::new(
170+
cfg.resource
170171
.iter()
171172
.filter(|(k, _v)| **k != semcov::resource::SERVICE_NAME)
172-
.map(|(k, v)| KeyValue::new(k.clone(), v.clone()))
173-
.collect::<Vec<KeyValue>>();
174-
Arc::new(Resource::new(without_service_name))
175-
});
173+
.map(|(k, v)| KeyValue::new(k.clone(), v.clone())),
174+
));
176175
cfg
177176
} else {
178177
Config {
179-
resource: Some(Arc::new(Resource::empty())),
178+
resource: Cow::Owned(Resource::empty()),
180179
..Default::default()
181180
}
182181
};
@@ -190,7 +189,7 @@ impl DatadogPipelineBuilder {
190189
(
191190
Config {
192191
// use a empty resource to prevent TracerProvider to assign a service name.
193-
resource: Some(Arc::new(Resource::empty())),
192+
resource: Cow::Owned(Resource::empty()),
194193
..Default::default()
195194
},
196195
service_name,

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,12 +179,13 @@ impl ApiVersion {
179179
#[cfg(test)]
180180
pub(crate) mod tests {
181181
use super::*;
182-
use opentelemetry::sdk;
183182
use opentelemetry::sdk::InstrumentationLibrary;
183+
use opentelemetry::sdk::{self, Resource};
184184
use opentelemetry::{
185185
trace::{SpanContext, SpanId, SpanKind, Status, TraceFlags, TraceId, TraceState},
186186
Key,
187187
};
188+
use std::borrow::Cow;
188189
use std::time::{Duration, SystemTime};
189190

190191
fn get_traces() -> Vec<Vec<trace::SpanData>> {
@@ -221,7 +222,7 @@ pub(crate) mod tests {
221222
events,
222223
links,
223224
status: Status::Ok,
224-
resource: None,
225+
resource: Cow::Owned(Resource::empty()),
225226
instrumentation_lib: InstrumentationLibrary::new("component", None, None),
226227
}
227228
}

opentelemetry-jaeger/src/exporter/config/agent.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,6 @@ impl AgentPipeline {
232232
let mut builder = sdk::trace::TracerProvider::builder();
233233

234234
let (config, process) = build_config_and_process(
235-
builder.sdk_provided_resource(),
236235
self.trace_config.take(),
237236
self.transformation_config.service_name.take(),
238237
);
@@ -270,7 +269,6 @@ impl AgentPipeline {
270269
// build sdk trace config and jaeger process.
271270
// some attributes like service name has attributes like service name
272271
let (config, process) = build_config_and_process(
273-
builder.sdk_provided_resource(),
274272
self.trace_config.take(),
275273
self.transformation_config.service_name.take(),
276274
);
@@ -312,12 +310,10 @@ impl AgentPipeline {
312310
where
313311
R: JaegerTraceRuntime,
314312
{
315-
let builder = sdk::trace::TracerProvider::builder();
316313
let export_instrument_library = self.transformation_config.export_instrument_library;
317314
// build sdk trace config and jaeger process.
318315
// some attributes like service name has attributes like service name
319316
let (_, process) = build_config_and_process(
320-
builder.sdk_provided_resource(),
321317
self.trace_config.take(),
322318
self.transformation_config.service_name.take(),
323319
);
@@ -331,9 +327,7 @@ impl AgentPipeline {
331327

332328
/// Build an jaeger exporter targeting a jaeger agent and running on the sync runtime.
333329
pub fn build_sync_agent_exporter(mut self) -> Result<crate::Exporter, TraceError> {
334-
let builder = sdk::trace::TracerProvider::builder();
335330
let (_, process) = build_config_and_process(
336-
builder.sdk_provided_resource(),
337331
self.trace_config.take(),
338332
self.transformation_config.service_name.take(),
339333
);

opentelemetry-jaeger/src/exporter/config/collector/http_client.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,18 +179,14 @@ mod collector_client_tests {
179179
use crate::exporter::thrift::jaeger::Batch;
180180
use crate::new_collector_pipeline;
181181
use opentelemetry::runtime::Tokio;
182-
use opentelemetry::sdk::Resource;
183182
use opentelemetry::trace::TraceError;
184-
use opentelemetry::KeyValue;
185183

186184
#[test]
187185
fn test_bring_your_own_client() -> Result<(), TraceError> {
188186
let invalid_uri_builder = new_collector_pipeline()
189187
.with_endpoint("localhost:6831")
190188
.with_http_client(test_http_client::TestHttpClient);
191-
let sdk_provided_resource =
192-
Resource::new(vec![KeyValue::new("service.name", "unknown_service")]);
193-
let (_, process) = build_config_and_process(sdk_provided_resource, None, None);
189+
let (_, process) = build_config_and_process(None, None);
194190
let mut uploader = invalid_uri_builder.build_uploader::<Tokio>()?;
195191
let res = futures_executor::block_on(async {
196192
uploader

opentelemetry-jaeger/src/exporter/config/collector/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,6 @@ impl CollectorPipeline {
407407
// some attributes like service name has attributes like service name
408408
let export_instrument_library = self.transformation_config.export_instrument_library;
409409
let (config, process) = build_config_and_process(
410-
builder.sdk_provided_resource(),
411410
self.trace_config.take(),
412411
self.transformation_config.service_name.take(),
413412
);

opentelemetry-jaeger/src/exporter/config/mod.rs

Lines changed: 11 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,9 @@
1010
//! [jaeger deployment guide]: https://www.jaegertracing.io/docs/1.31/deployment
1111
1212
use crate::Process;
13-
use opentelemetry::sdk::trace::Config;
14-
use opentelemetry::sdk::Resource;
1513
use opentelemetry::trace::{TraceError, TracerProvider};
1614
use opentelemetry::{global, sdk, KeyValue};
1715
use opentelemetry_semantic_conventions as semcov;
18-
use std::sync::Arc;
1916

2017
/// Config a exporter that sends the spans to a [jaeger agent](https://www.jaegertracing.io/docs/1.31/deployment/#agent).
2118
pub mod agent;
@@ -54,35 +51,25 @@ trait HasRequiredConfig {
5451
// There are multiple ways to set the service name. A `service.name` tag will be always added
5552
// to the process tags.
5653
fn build_config_and_process(
57-
sdk_resource: sdk::Resource,
58-
mut config: Option<sdk::trace::Config>,
54+
config: Option<sdk::trace::Config>,
5955
service_name_opt: Option<String>,
6056
) -> (sdk::trace::Config, Process) {
61-
let (config, resource) = if let Some(mut config) = config.take() {
62-
let resource = if let Some(resource) = config.resource.replace(Arc::new(Resource::empty()))
63-
{
64-
sdk_resource.merge(resource)
65-
} else {
66-
sdk_resource
67-
};
68-
69-
(config, resource)
70-
} else {
71-
(Config::default(), sdk_resource)
72-
};
57+
let config = config.unwrap_or_default();
7358

7459
let service_name = service_name_opt.unwrap_or_else(|| {
75-
resource
60+
config
61+
.resource
7662
.get(semcov::resource::SERVICE_NAME)
7763
.map(|v| v.to_string())
7864
.unwrap_or_else(|| "unknown_service".to_string())
7965
});
8066

8167
// merge the tags and resource. Resources take priority.
82-
let mut tags = resource
83-
.into_iter()
84-
.filter(|(key, _)| *key != semcov::resource::SERVICE_NAME)
85-
.map(|(key, value)| KeyValue::new(key, value))
68+
let mut tags = config
69+
.resource
70+
.iter()
71+
.filter(|(key, _)| **key != semcov::resource::SERVICE_NAME)
72+
.map(|(key, value)| KeyValue::new(key.clone(), value.clone()))
8673
.collect::<Vec<KeyValue>>();
8774

8875
tags.push(KeyValue::new(
@@ -101,51 +88,20 @@ mod tests {
10188
use opentelemetry::sdk::Resource;
10289
use opentelemetry::KeyValue;
10390
use std::env;
104-
use std::sync::Arc;
10591

10692
#[test]
10793
fn test_set_service_name() {
10894
let service_name = "halloween_service".to_string();
10995

11096
// set via builder's service name, it has highest priority
111-
let (_, process) =
112-
build_config_and_process(Resource::empty(), None, Some(service_name.clone()));
97+
let (_, process) = build_config_and_process(None, Some(service_name.clone()));
11398
assert_eq!(process.service_name, service_name);
11499

115100
// make sure the tags in resource are moved to process
116101
let trace_config = Config::default()
117102
.with_resource(Resource::new(vec![KeyValue::new("test-key", "test-value")]));
118-
let (config, process) =
119-
build_config_and_process(Resource::empty(), Some(trace_config), Some(service_name));
120-
assert_eq!(config.resource, Some(Arc::new(Resource::empty())));
103+
let (_, process) = build_config_and_process(Some(trace_config), Some(service_name));
121104
assert_eq!(process.tags.len(), 2);
122-
123-
// sdk provided resource can override service name if users didn't provided service name to builder
124-
let (_, process) = build_config_and_process(
125-
Resource::new(vec![KeyValue::new("service.name", "halloween_service")]),
126-
None,
127-
None,
128-
);
129-
assert_eq!(process.service_name, "halloween_service");
130-
131-
// users can also provided service.name from config's resource, in this case, it will override the
132-
// sdk provided service name
133-
let trace_config = Config::default().with_resource(Resource::new(vec![KeyValue::new(
134-
"service.name",
135-
"override_service",
136-
)]));
137-
let (_, process) = build_config_and_process(
138-
Resource::new(vec![KeyValue::new("service.name", "halloween_service")]),
139-
Some(trace_config),
140-
None,
141-
);
142-
143-
assert_eq!(process.service_name, "override_service");
144-
assert_eq!(process.tags.len(), 1);
145-
assert_eq!(
146-
process.tags[0],
147-
KeyValue::new("service.name", "override_service")
148-
);
149105
}
150106

151107
#[test]

opentelemetry-otlp/src/transform/metrics.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,16 @@ mod tests {
417417
// If we changed the sink function to process the input in parallel, we will have to sort other vectors
418418
// like data points in Metrics.
419419
fn assert_resource_metrics(mut expect: ResourceMetrics, mut actual: ResourceMetrics) {
420-
assert_eq!(expect.resource, actual.resource);
420+
assert_eq!(
421+
expect
422+
.resource
423+
.as_mut()
424+
.map(|r| r.attributes.sort_by_key(|kv| kv.key.to_string())),
425+
actual
426+
.resource
427+
.as_mut()
428+
.map(|r| r.attributes.sort_by_key(|kv| kv.key.to_string()))
429+
);
421430
assert_eq!(
422431
expect.instrumentation_library_metrics.len(),
423432
actual.instrumentation_library_metrics.len()

opentelemetry-proto/src/transform/traces.rs

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,13 @@ pub mod tonic {
5151
let span_kind: span::SpanKind = source_span.span_kind.into();
5252
ResourceSpans {
5353
resource: Some(Resource {
54-
attributes: resource_attributes(
55-
source_span.resource.as_ref().map(AsRef::as_ref),
56-
)
57-
.0,
54+
attributes: resource_attributes(&source_span.resource).0,
5855
dropped_attributes_count: 0,
5956
}),
6057
schema_url: source_span
6158
.resource
62-
.and_then(|resource| resource.schema_url().map(|url| url.to_string()))
59+
.schema_url()
60+
.map(|url| url.to_string())
6361
.unwrap_or_default(),
6462
instrumentation_library_spans: vec![InstrumentationLibrarySpans {
6563
schema_url: source_span
@@ -112,14 +110,11 @@ pub mod tonic {
112110
}
113111
}
114112

115-
fn resource_attributes(resource: Option<&sdk::Resource>) -> Attributes {
113+
fn resource_attributes(resource: &sdk::Resource) -> Attributes {
116114
resource
117-
.map(|res| {
118-
res.iter()
119-
.map(|(k, v)| opentelemetry::KeyValue::new(k.clone(), v.clone()))
120-
.collect::<Vec<_>>()
121-
})
122-
.unwrap_or_default()
115+
.iter()
116+
.map(|(k, v)| opentelemetry::KeyValue::new(k.clone(), v.clone()))
117+
.collect::<Vec<_>>()
123118
.into()
124119
}
125120
}
@@ -175,10 +170,7 @@ pub mod grpcio {
175170
fn from(source_span: SpanData) -> Self {
176171
ResourceSpans {
177172
resource: SingularPtrField::from(Some(Resource {
178-
attributes: resource_attributes(
179-
source_span.resource.as_ref().map(AsRef::as_ref),
180-
)
181-
.0,
173+
attributes: resource_attributes(&source_span.resource).0,
182174
dropped_attributes_count: 0,
183175
..Default::default()
184176
})),
@@ -246,15 +238,11 @@ pub mod grpcio {
246238
}
247239
}
248240

249-
fn resource_attributes(resource: Option<&sdk::Resource>) -> Attributes {
241+
fn resource_attributes(resource: &sdk::Resource) -> Attributes {
250242
resource
251-
.map(|resource| {
252-
resource
253-
.iter()
254-
.map(|(k, v)| opentelemetry::KeyValue::new(k.clone(), v.clone()))
255-
.collect::<Vec<_>>()
256-
})
257-
.unwrap_or_default()
243+
.iter()
244+
.map(|(k, v)| opentelemetry::KeyValue::new(k.clone(), v.clone()))
245+
.collect::<Vec<_>>()
258246
.into()
259247
}
260248
}

opentelemetry-sdk/Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,24 @@ version = "0.1.0"
44
edition = "2018"
55

66
[dependencies]
7-
opentelemetry-api = { version = "0.1", path = "../opentelemetry-api/" }
87
async-std = { version = "1.6", features = ["unstable"], optional = true }
98
async-trait = { version = "0.1", optional = true }
9+
crossbeam-channel = { version = "0.5", optional = true }
1010
dashmap = { version = "4.0.1", optional = true }
1111
fnv = { version = "1.0", optional = true }
1212
futures-channel = "0.3"
1313
futures-executor = "0.3"
1414
futures-util = { version = "0.3", default-features = false, features = ["std", "sink"] }
1515
lazy_static = "1.4"
16+
once_cell = "1.10"
17+
opentelemetry-api = { version = "0.1", path = "../opentelemetry-api/" }
1618
percent-encoding = { version = "2.0", optional = true }
1719
pin-project = { version = "1.0.2", optional = true }
1820
rand = { version = "0.8", default-features = false, features = ["std", "std_rng"], optional = true }
1921
serde = { version = "1.0", features = ["derive", "rc"], optional = true }
2022
thiserror = "1"
2123
tokio = { version = "1.0", default-features = false, features = ["rt", "time"], optional = true }
2224
tokio-stream = { version = "0.1", optional = true }
23-
crossbeam-channel = { version = "0.5", optional = true }
2425

2526
[package.metadata.docs.rs]
2627
all-features = true

opentelemetry-sdk/benches/batch_span_processor.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ use opentelemetry_sdk::export::trace::SpanData;
66
use opentelemetry_sdk::runtime::Tokio;
77
use opentelemetry_sdk::testing::trace::NoopSpanExporter;
88
use opentelemetry_sdk::trace::{BatchSpanProcessor, EvictedHashMap, EvictedQueue, SpanProcessor};
9+
use opentelemetry_sdk::Resource;
10+
use std::borrow::Cow;
911
use std::sync::Arc;
1012
use std::time::SystemTime;
1113
use tokio::runtime::Runtime;
@@ -30,7 +32,7 @@ fn get_span_data() -> Vec<SpanData> {
3032
events: EvictedQueue::new(12),
3133
links: EvictedQueue::new(12),
3234
status: Status::Unset,
33-
resource: None,
35+
resource: Cow::Owned(Resource::empty()),
3436
instrumentation_lib: Default::default(),
3537
})
3638
.collect::<Vec<SpanData>>()

opentelemetry-sdk/src/export/trace/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
//! Trace exporters
2+
use crate::Resource;
23
use async_trait::async_trait;
34
use opentelemetry_api::trace::{Event, Link, SpanContext, SpanId, SpanKind, Status, TraceError};
45
use std::borrow::Cow;
56
use std::fmt::Debug;
6-
use std::sync::Arc;
77
use std::time::SystemTime;
88

99
pub mod stdout;
@@ -73,7 +73,7 @@ pub struct SpanData {
7373
/// Span status
7474
pub status: Status,
7575
/// Resource contains attributes representing an entity that produced this span.
76-
pub resource: Option<Arc<crate::Resource>>,
76+
pub resource: Cow<'static, Resource>,
7777
/// Instrumentation library that produced this span
7878
pub instrumentation_lib: crate::InstrumentationLibrary,
7979
}

0 commit comments

Comments
 (0)