Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/add resource builder #2322

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions examples/metrics-advanced/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,14 @@ fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider {
.with_temporality(Temporality::Delta)
.build();

let resource = Resource::builder()
.with_key_value(KeyValue::new("service.name", "metrics-advanced-example"))
.build();

let reader = PeriodicReader::builder(exporter, runtime::Tokio).build();
let provider = SdkMeterProvider::builder()
.with_reader(reader)
.with_resource(Resource::new([KeyValue::new(
"service.name",
"metrics-advanced-example",
)]))
.with_resource(resource)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if we can do with_servicename_resource("metrics-advanced-example") ? This will be the most common use case.
Or just Resource::builder().with_servicename("metrics-advanced-example")......build() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that, I was using the go implementation for Resource builder this weekend and they had something similar to this and it felt pretty nice

sResource, err := resource.New(
  ctx,
  resource.WithTelemetrySDK(),
  resource.WithAttributes(semconv.ServiceName("trace-export-service")),
)

.with_view(my_view_rename_and_unit)
.with_view(my_view_drop_attributes)
.with_view(my_view_change_aggregation)
Expand Down
1 change: 1 addition & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## vNext

- Add `ResourceBuilder` for an easy way to create new `Resource`s
- **DEPRECATED**:
- `trace::Config` methods are moving onto `TracerProvider` Builder to be consistent with other signals. See https://github.com/open-telemetry/opentelemetry-rust/pull/2303 for migration guide.
`trace::Config` is scheduled to be removed from public API in `v0.28.0`.
Expand Down
108 changes: 108 additions & 0 deletions opentelemetry-sdk/src/resource/builder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
use std::time::Duration;

use opentelemetry::{KeyValue, Value};

use super::{Resource, ResourceDetector, SERVICE_NAME};

/// Builder to allow easy composition of a Resource
#[derive(Debug, Default)]
pub struct ResourceBuilder {
resource: Resource,
}

impl ResourceBuilder {
/// Create ResourceBuilder with an empty [Resource].
pub fn new_empty() -> Self {
pitoniak32 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn new_empty() -> Self {
pub fn new() -> Self {

This makes it easier to reason about. Since on Resource the new is the same as empty but it accepts key values. Since this is a builder most would assume it's empty.

ResourceBuilder {
resource: Resource::empty(),
}
}

/// Create ResourceBuilder with a default [Resource].
pub fn new_default() -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn new_default() -> Self {
pub fn default() -> Self {

Probably should be done as an impl Default for ResourceBuilder to make it more generally compatible.

ResourceBuilder {
resource: Resource::default(),
}
}

Check warning on line 26 in opentelemetry-sdk/src/resource/builder.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/resource/builder.rs#L22-L26

Added lines #L22 - L26 were not covered by tests

/// Add a single [ResourceDetector] to your resource.
pub fn with_detector(self, detector: Box<dyn ResourceDetector>) -> Self {
self.with_detectors(vec![detector])
}

/// Add multiple [ResourceDetector] to your resource.
pub fn with_detectors(mut self, detectors: Vec<Box<dyn ResourceDetector>>) -> Self {
self.resource = self
.resource
.merge(&Resource::from_detectors(Duration::from_secs(0), detectors));
self
}

/// Add a [KeyValue] to the resource.
pub fn with_key_value(self, kv: KeyValue) -> Self {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting why this does not need mut self...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be since the with_detectors consumes the builder and its not a &mut reference? That's just a guess I'm actually not sure about that 😅

self.with_key_values(vec![kv])
}

/// Add an [Attribute] to your resource for "service.name"
pub fn with_service_name<V>(self, service_name: V) -> Self
where
V: Into<Value>,
{
self.with_key_value(KeyValue::new(SERVICE_NAME, service_name))
}

Check warning on line 52 in opentelemetry-sdk/src/resource/builder.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/resource/builder.rs#L47-L52

Added lines #L47 - L52 were not covered by tests

/// Add multiple [KeyValue]s to the resource.
pub fn with_key_values<T: IntoIterator<Item = KeyValue>>(mut self, kvs: T) -> Self {
self.resource = self.resource.merge(&Resource::new(kvs));
self
}

/// Create a [Resource] with the options provided to the [ResourceBuilder].
pub fn build(self) -> Resource {
self.resource
}
}

#[cfg(test)]
mod tests {
use opentelemetry::KeyValue;

use crate::resource::EnvResourceDetector;

use super::*;

#[test]
fn detect_resource() {
temp_env::with_vars(
[
(
"OTEL_RESOURCE_ATTRIBUTES",
Some("key=value, k = v , a= x, a=z"),
),
("IRRELEVANT", Some("20200810")),
],
|| {
let resource = Resource::builder()
.with_detector(Box::new(EnvResourceDetector::new()))
.with_key_value(KeyValue::new("test1", "test_value"))
.with_key_values(vec![
KeyValue::new("test1", "test_value1"),
KeyValue::new("test2", "test_value2"),
])
.build();

assert_eq!(
resource,
Resource::new(vec![
KeyValue::new("key", "value"),
KeyValue::new("test1", "test_value1"),
KeyValue::new("test2", "test_value2"),
KeyValue::new("k", "v"),
KeyValue::new("a", "x"),
KeyValue::new("a", "z"),
])
)
},
)
}
}
18 changes: 18 additions & 0 deletions opentelemetry-sdk/src/resource/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
//!
//! The OS and Process resource detectors are packaged separately in the
//! [`opentelemetry-resource-detector` crate](https://github.com/open-telemetry/opentelemetry-rust-contrib/tree/main/opentelemetry-resource-detectors).
mod builder;
mod env;
mod telemetry;

mod attributes;
pub(crate) use attributes::*;

pub use builder::ResourceBuilder;
pub use env::EnvResourceDetector;
pub use env::SdkProvidedResourceDetector;
pub use telemetry::TelemetryResourceDetector;
Expand Down Expand Up @@ -66,6 +68,22 @@
}

impl Resource {
/// Creates a Builder that allows you to configure multiple aspects of the Resource.
///
/// If you want to start from a [Resource::default()] see [Resource::builder_default()].
///
/// Starts with a [Resource::empty()].
pub fn builder() -> ResourceBuilder {
ResourceBuilder::new_empty()
}

/// Creates a Builder that allows you to configure multiple aspects of the Resource.
///
/// Starts with a [Resource::default()].
pub fn builder_default() -> ResourceBuilder {
ResourceBuilder::new_default()
}

Check warning on line 85 in opentelemetry-sdk/src/resource/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/resource/mod.rs#L83-L85

Added lines #L83 - L85 were not covered by tests

/// Creates an empty resource.
/// This is the basic constructor that initializes a resource with no attributes and no schema URL.
pub fn empty() -> Self {
Expand Down
Loading