Skip to content

Commit

Permalink
Feat/remove timeout from detectors (#2332)
Browse files Browse the repository at this point in the history
  • Loading branch information
pitoniak32 authored Dec 4, 2024
1 parent 506a4f9 commit b35c0d6
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 37 deletions.
12 changes: 7 additions & 5 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

## vNext

- *Breaking* SimpleLogProcessor modified to be generic over `LogExporter` to
avoid dynamic dispatch to invoke exporter. If you were using
`with_simple_exporter` to add `LogExporter` with SimpleLogProcessor, this is a
transparent change.
[#2338](https://github.com/open-telemetry/opentelemetry-rust/pull/2338)
- *Breaking*
- SimpleLogProcessor modified to be generic over `LogExporter` to
avoid dynamic dispatch to invoke exporter. If you were using
`with_simple_exporter` to add `LogExporter` with SimpleLogProcessor, this is a
transparent change.
[#2338](https://github.com/open-telemetry/opentelemetry-rust/pull/2338)
- `ResourceDetector.detect()` no longer supports timeout option.

## 0.27.1

Expand Down
20 changes: 9 additions & 11 deletions opentelemetry-sdk/src/resource/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use crate::resource::{Resource, ResourceDetector};
use opentelemetry::{Key, KeyValue, Value};
use std::env;
use std::time::Duration;

const OTEL_RESOURCE_ATTRIBUTES: &str = "OTEL_RESOURCE_ATTRIBUTES";
const OTEL_SERVICE_NAME: &str = "OTEL_SERVICE_NAME";
Expand All @@ -20,7 +19,7 @@ pub struct EnvResourceDetector {
}

impl ResourceDetector for EnvResourceDetector {
fn detect(&self, _timeout: Duration) -> Resource {
fn detect(&self) -> Resource {
match env::var(OTEL_RESOURCE_ATTRIBUTES) {
Ok(s) if !s.is_empty() => construct_otel_resources(s),
Ok(_) | Err(_) => Resource::new(vec![]), // return empty resource
Expand Down Expand Up @@ -72,7 +71,7 @@ fn construct_otel_resources(s: String) -> Resource {
pub struct SdkProvidedResourceDetector;

impl ResourceDetector for SdkProvidedResourceDetector {
fn detect(&self, _timeout: Duration) -> Resource {
fn detect(&self) -> Resource {
Resource::new(vec![KeyValue::new(
super::SERVICE_NAME,
env::var(OTEL_SERVICE_NAME)
Expand All @@ -81,7 +80,7 @@ impl ResourceDetector for SdkProvidedResourceDetector {
.map(Value::from)
.or_else(|| {
EnvResourceDetector::new()
.detect(Duration::from_secs(0))
.detect()
.get(Key::new(super::SERVICE_NAME))
})
.unwrap_or_else(|| "unknown_service".into()),
Expand All @@ -96,7 +95,6 @@ mod tests {
};
use crate::resource::{EnvResourceDetector, Resource, ResourceDetector};
use opentelemetry::{Key, KeyValue, Value};
use std::time::Duration;

#[test]
fn test_read_from_env() {
Expand All @@ -110,7 +108,7 @@ mod tests {
],
|| {
let detector = EnvResourceDetector::new();
let resource = detector.detect(Duration::from_secs(5));
let resource = detector.detect();
assert_eq!(
resource,
Resource::new(vec![
Expand All @@ -124,21 +122,21 @@ mod tests {
);

let detector = EnvResourceDetector::new();
let resource = detector.detect(Duration::from_secs(5));
let resource = detector.detect();
assert!(resource.is_empty());
}

#[test]
fn test_sdk_provided_resource_detector() {
// Ensure no env var set
let no_env = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
let no_env = SdkProvidedResourceDetector.detect();
assert_eq!(
no_env.get(Key::from_static_str(crate::resource::SERVICE_NAME)),
Some(Value::from("unknown_service")),
);

temp_env::with_var(OTEL_SERVICE_NAME, Some("test service"), || {
let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
let with_service = SdkProvidedResourceDetector.detect();
assert_eq!(
with_service.get(Key::from_static_str(crate::resource::SERVICE_NAME)),
Some(Value::from("test service")),
Expand All @@ -149,7 +147,7 @@ mod tests {
OTEL_RESOURCE_ATTRIBUTES,
Some("service.name=test service1"),
|| {
let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
let with_service = SdkProvidedResourceDetector.detect();
assert_eq!(
with_service.get(Key::from_static_str(crate::resource::SERVICE_NAME)),
Some(Value::from("test service1")),
Expand All @@ -164,7 +162,7 @@ mod tests {
(OTEL_RESOURCE_ATTRIBUTES, Some("service.name=test service3")),
],
|| {
let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
let with_service = SdkProvidedResourceDetector.detect();
assert_eq!(
with_service.get(Key::from_static_str(crate::resource::SERVICE_NAME)),
Some(Value::from("test service"))
Expand Down
26 changes: 9 additions & 17 deletions opentelemetry-sdk/src/resource/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use std::borrow::Cow;
use std::collections::{hash_map, HashMap};
use std::ops::Deref;
use std::sync::Arc;
use std::time::Duration;

/// Inner structure of `Resource` holding the actual data.
/// This structure is designed to be shared among `Resource` instances via `Arc`.
Expand All @@ -54,14 +53,11 @@ pub struct Resource {

impl Default for Resource {
fn default() -> Self {
Self::from_detectors(
Duration::from_secs(0),
vec![
Box::new(SdkProvidedResourceDetector),
Box::new(TelemetryResourceDetector),
Box::new(EnvResourceDetector::new()),
],
)
Self::from_detectors(vec![
Box::new(SdkProvidedResourceDetector),
Box::new(TelemetryResourceDetector),
Box::new(EnvResourceDetector::new()),
])
}
}

Expand Down Expand Up @@ -137,10 +133,10 @@ impl Resource {
/// Create a new `Resource` from resource detectors.
///
/// timeout will be applied to each detector.
pub fn from_detectors(timeout: Duration, detectors: Vec<Box<dyn ResourceDetector>>) -> Self {
pub fn from_detectors(detectors: Vec<Box<dyn ResourceDetector>>) -> Self {
let mut resource = Resource::empty();
for detector in detectors {
let detected_res = detector.detect(timeout);
let detected_res = detector.detect();
// This call ensures that if the Arc is not uniquely owned,
// the data is cloned before modification, preserving safety.
// If the Arc is uniquely owned, it simply returns a mutable reference to the data.
Expand Down Expand Up @@ -262,13 +258,12 @@ pub trait ResourceDetector {
///
/// If source information to construct a Resource is invalid, for example,
/// missing required values. an empty Resource should be returned.
fn detect(&self, timeout: Duration) -> Resource;
fn detect(&self) -> Resource;
}

#[cfg(test)]
mod tests {
use super::*;
use std::time;

#[test]
fn new_resource() {
Expand Down Expand Up @@ -368,10 +363,7 @@ mod tests {
],
|| {
let detector = EnvResourceDetector::new();
let resource = Resource::from_detectors(
time::Duration::from_secs(5),
vec![Box::new(detector)],
);
let resource = Resource::from_detectors(vec![Box::new(detector)]);
assert_eq!(
resource,
Resource::new(vec![
Expand Down
3 changes: 1 addition & 2 deletions opentelemetry-sdk/src/resource/telemetry.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::resource::ResourceDetector;
use crate::Resource;
use opentelemetry::KeyValue;
use std::time::Duration;

/// Detect the telemetry SDK information used to capture data recorded by the instrumentation libraries.
///
Expand All @@ -16,7 +15,7 @@ use std::time::Duration;
pub struct TelemetryResourceDetector;

impl ResourceDetector for TelemetryResourceDetector {
fn detect(&self, _timeout: Duration) -> Resource {
fn detect(&self) -> Resource {
Resource::new(vec![
KeyValue::new(super::TELEMETRY_SDK_NAME, "opentelemetry"),
KeyValue::new(super::TELEMETRY_SDK_LANGUAGE, "rust"),
Expand Down
3 changes: 1 addition & 2 deletions opentelemetry-zipkin/src/exporter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use opentelemetry_semantic_conventions as semcov;
use std::borrow::Cow;
use std::net::SocketAddr;
use std::sync::Arc;
use std::time::Duration;

/// Zipkin span exporter
#[derive(Debug)]
Expand Down Expand Up @@ -112,7 +111,7 @@ impl ZipkinPipelineBuilder {
(config, Endpoint::new(service_name, self.service_addr))
} else {
let service_name = SdkProvidedResourceDetector
.detect(Duration::from_secs(0))
.detect()
.get(semcov::resource::SERVICE_NAME.into())
.unwrap()
.to_string();
Expand Down

0 comments on commit b35c0d6

Please sign in to comment.