Skip to content

Remove Cow from InstrumentationScope #2275

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 6, 2024
Merged

Remove Cow from InstrumentationScope #2275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa00c63
Remove Cow from the InstrumentationScope public API
stormshield-fabs Nov 5, 2024
91a3b45
update changelog
stormshield-fabs Nov 6, 2024
7f0d42a
Merge branch 'main' into instrumentation-scope-no-cow
lalitb Nov 6, 2024
7a198ae
Merge branch 'main' into instrumentation-scope-no-cow
cijothomas Nov 6, 2024
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
8 changes: 4 additions & 4 deletions opentelemetry-appender-tracing/src/layer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ mod tests {
.expect("Atleast one log is expected to be present.");

// Validate common fields
assert_eq!(log.instrumentation.name, "opentelemetry-appender-tracing");
assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
assert_eq!(log.record.severity_number, Some(Severity::Error));

// Validate trace context is none.
Expand Down Expand Up @@ -428,7 +428,7 @@ mod tests {
.expect("Atleast one log is expected to be present.");

// validate common fields.
assert_eq!(log.instrumentation.name, "opentelemetry-appender-tracing");
assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
assert_eq!(log.record.severity_number, Some(Severity::Error));

// validate trace context.
Expand Down Expand Up @@ -526,7 +526,7 @@ mod tests {
.expect("Atleast one log is expected to be present.");

// Validate common fields
assert_eq!(log.instrumentation.name, "opentelemetry-appender-tracing");
assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
assert_eq!(log.record.severity_number, Some(Severity::Error));

// Validate trace context is none.
Expand Down Expand Up @@ -605,7 +605,7 @@ mod tests {
.expect("Atleast one log is expected to be present.");

// validate common fields.
assert_eq!(log.instrumentation.name, "opentelemetry-appender-tracing");
assert_eq!(log.instrumentation.name(), "opentelemetry-appender-tracing");
assert_eq!(log.record.severity_number, Some(Severity::Error));

// validate trace context.
Expand Down
20 changes: 8 additions & 12 deletions opentelemetry-proto/src/transform/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@
}
} else {
InstrumentationScope {
name: library.name.into_owned(),
version: library.version.map(Cow::into_owned).unwrap_or_default(),
attributes: Attributes::from(library.attributes).0,
name: library.name().to_owned(),
version: library.version().map(ToOwned::to_owned).unwrap_or_default(),
attributes: Attributes::from(library.attributes().cloned()).0,

Check warning on line 69 in opentelemetry-proto/src/transform/common.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/common.rs#L67-L69 

Added lines #L67 - L69 were not covered by tests
..Default::default()
}
}
Expand Down Expand Up @@ -95,13 +95,9 @@
}
} else {
InstrumentationScope {
name: library.name.to_string(),
version: library
.version
.as_ref()
.map(ToString::to_string)
.unwrap_or_default(),
attributes: Attributes::from(library.attributes.clone()).0,
name: library.name().to_owned(),
version: library.version().map(ToOwned::to_owned).unwrap_or_default(),
attributes: Attributes::from(library.attributes().cloned()).0,
..Default::default()
}
}
Expand All @@ -112,8 +108,8 @@
#[derive(Default, Debug)]
pub struct Attributes(pub ::std::vec::Vec<crate::proto::tonic::common::v1::KeyValue>);

impl From<Vec<opentelemetry::KeyValue>> for Attributes {
fn from(kvs: Vec<opentelemetry::KeyValue>) -> Self {
impl<I: IntoIterator<Item = opentelemetry::KeyValue>> From<I> for Attributes {
fn from(kvs: I) -> Self {
Attributes(
kvs.into_iter()
.map(|api_kv| KeyValue {
Expand Down
7 changes: 3 additions & 4 deletions opentelemetry-proto/src/transform/logs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,8 @@
schema_url: resource.schema_url.clone().unwrap_or_default(),
scope_logs: vec![ScopeLogs {
schema_url: instrumentation
.schema_url
.clone()
.map(Into::into)
.schema_url()
.map(ToOwned::to_owned)

Check warning on line 171 in opentelemetry-proto/src/transform/logs.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/logs.rs#L170-L171 

Added lines #L170 - L171 were not covered by tests
.unwrap_or_default(),
scope: Some((instrumentation, log_record.target.clone()).into()),
log_records: vec![log_record.into()],
Expand All @@ -196,7 +195,7 @@
let key = log_record
.target
.clone()
.unwrap_or_else(|| Cow::Owned(instrumentation.name.clone().into_owned()));
.unwrap_or_else(|| Cow::Owned(instrumentation.name().to_owned()));
scope_map
.entry(key)
.or_default()
Expand Down
5 changes: 2 additions & 3 deletions opentelemetry-proto/src/transform/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,8 @@
metrics: sm.metrics.iter().map(Into::into).collect(),
schema_url: sm
.scope
.schema_url
.as_ref()
.map(ToString::to_string)
.schema_url()
.map(ToOwned::to_owned)

Check warning on line 141 in opentelemetry-proto/src/transform/metrics.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/metrics.rs#L140-L141 

Added lines #L140 - L141 were not covered by tests
.unwrap_or_default(),
}
}
Expand Down
5 changes: 2 additions & 3 deletions opentelemetry-proto/src/transform/trace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,8 @@
scope_spans: vec![ScopeSpans {
schema_url: source_span
.instrumentation_scope
.schema_url
.as_ref()
.map(ToString::to_string)
.schema_url()
.map(ToOwned::to_owned)

Check warning on line 106 in opentelemetry-proto/src/transform/trace.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-proto/src/transform/trace.rs#L105-L106 

Added lines #L105 - L106 were not covered by tests
.unwrap_or_default(),
scope: Some((source_span.instrumentation_scope, None).into()),
spans: vec![Span {
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/logs/log_emitter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl opentelemetry::logs::Logger for Logger {
|| processor.event_enabled(
level,
target,
self.instrumentation_scope().name.as_ref(),
self.instrumentation_scope().name().as_ref(),
);
}
enabled
Expand Down
15 changes: 7 additions & 8 deletions opentelemetry-sdk/src/logs/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ mod tests {
let log = exported_logs
.first()
.expect("Atleast one log is expected to be present.");
assert_eq!(log.instrumentation.name, "test-logger");
assert_eq!(log.instrumentation.name(), "test-logger");
assert_eq!(log.record.severity_number, Some(Severity::Error));
assert_eq!(log.record.attributes_len(), 10);
for i in 1..=10 {
Expand All @@ -111,14 +111,13 @@ mod tests {

let logger = provider.logger_with_scope(scope);
let instrumentation_scope = logger.instrumentation_scope();
let attributes = &instrumentation_scope.attributes;
assert_eq!(instrumentation_scope.name, "test_logger");
assert_eq!(instrumentation_scope.name(), "test_logger");
assert_eq!(
instrumentation_scope.schema_url,
Some("https://opentelemetry.io/schema/1.0.0".into())
instrumentation_scope.schema_url(),
Some("https://opentelemetry.io/schema/1.0.0")
);
assert_eq!(attributes.len(), 1);
assert_eq!(attributes[0].key, "test_k".into());
assert_eq!(attributes[0].value, "test_v".into());
assert!(instrumentation_scope
.attributes()
.eq(&[KeyValue::new("test_k", "test_v")]));
}
}
12 changes: 5 additions & 7 deletions opentelemetry-sdk/src/metrics/instrument.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,11 @@
}

pub(crate) fn matches_scope(&self, other: &Instrument) -> bool {
(self.scope.name.is_empty() || self.scope.name.as_ref() == other.scope.name.as_ref())
&& (self.scope.version.is_none()
|| self.scope.version.as_ref().map(AsRef::as_ref)
== other.scope.version.as_ref().map(AsRef::as_ref))
&& (self.scope.schema_url.is_none()
|| self.scope.schema_url.as_ref().map(AsRef::as_ref)
== other.scope.schema_url.as_ref().map(AsRef::as_ref))
(self.scope.name().is_empty() || self.scope.name() == other.scope.name())
&& (self.scope.version().is_none()
|| self.scope.version().as_ref() == other.scope.version().as_ref())

Check warning on line 165 in opentelemetry-sdk/src/metrics/instrument.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/instrument.rs#L165 

Added line #L165 was not covered by tests
&& (self.scope.schema_url().is_none()
|| self.scope.schema_url().as_ref() == other.scope.schema_url().as_ref())

Check warning on line 167 in opentelemetry-sdk/src/metrics/instrument.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/instrument.rs#L167 

Added line #L167 was not covered by tests
}
}

Expand Down
34 changes: 17 additions & 17 deletions opentelemetry-sdk/src/metrics/meter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
if let Err(err) = validation_result {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 79 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L79 

Added line #L79 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Measurements from this Counter will be ignored.",
reason = format!("{}", err)
Expand All @@ -98,7 +98,7 @@
Err(err) => {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 101 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L101 

Added line #L101 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Measurements from this Counter will be ignored.",
reason = format!("{}", err)
Expand All @@ -120,7 +120,7 @@
if let Err(err) = validation_result {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 123 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L123 

Added line #L123 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableCounter will not be invoked.",
reason = format!("{}", err));
Expand All @@ -138,7 +138,7 @@
if ms.is_empty() {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 141 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L141 

Added line #L141 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableCounter will not be invoked. Check View Configuration."
);
Expand All @@ -158,7 +158,7 @@
Err(err) => {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 161 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L161 

Added line #L161 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableCounter will not be invoked.",
reason = format!("{}", err));
Expand All @@ -179,7 +179,7 @@
if let Err(err) = validation_result {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 182 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L182 

Added line #L182 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableUpDownCounter will not be invoked.",
reason = format!("{}", err));
Expand All @@ -197,7 +197,7 @@
if ms.is_empty() {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 200 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L200 

Added line #L200 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableUpDownCounter will not be invoked. Check View Configuration."
);
Expand All @@ -217,7 +217,7 @@
Err(err) => {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 220 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L220 

Added line #L220 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableUpDownCounter will not be invoked.",
reason = format!("{}", err));
Expand All @@ -238,7 +238,7 @@
if let Err(err) = validation_result {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 241 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L241 

Added line #L241 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableGauge will not be invoked.",
reason = format!("{}", err));
Expand All @@ -256,7 +256,7 @@
if ms.is_empty() {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 259 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L259 

Added line #L259 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableGauge will not be invoked. Check View Configuration."
);
Expand All @@ -276,7 +276,7 @@
Err(err) => {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 279 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L279 

Added line #L279 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Callbacks for this ObservableGauge will not be invoked.",
reason = format!("{}", err));
Expand All @@ -297,7 +297,7 @@
if let Err(err) = validation_result {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 300 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L300 

Added line #L300 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Measurements from this UpDownCounter will be ignored.",
reason = format!("{}", err)
Expand All @@ -319,7 +319,7 @@
Err(err) => {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 322 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L322 

Added line #L322 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Measurements from this UpDownCounter will be ignored.",
reason = format!("{}", err)
Expand All @@ -341,7 +341,7 @@
if let Err(err) = validation_result {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 344 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L344 

Added line #L344 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Measurements from this Gauge will be ignored.",
reason = format!("{}", err)
Expand All @@ -363,7 +363,7 @@
Err(err) => {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 366 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L366 

Added line #L366 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Measurements from this Gauge will be ignored.",
reason = format!("{}", err)
Expand All @@ -385,7 +385,7 @@
if let Err(err) = validation_result {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 388 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L388 

Added line #L388 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Measurements from this Histogram will be ignored.",
reason = format!("{}", err)
Expand All @@ -407,7 +407,7 @@
Err(err) => {
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
meter_name = self.scope.name(),

Check warning on line 410 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L410 

Added line #L410 was not covered by tests
instrument_name = builder.name.as_ref(),
message = "Measurements from this Histogram will be ignored.",
reason = format!("{}", err)
Expand Down
20 changes: 9 additions & 11 deletions opentelemetry-sdk/src/metrics/meter_provider.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,18 +465,16 @@ mod tests {
assert_eq!(provider.inner.meters.lock().unwrap().len(), 2);

// these are different meters because meter names are case sensitive
let mut library = InstrumentationScope::builder("ABC")
.with_version("1.0.0")
.with_schema_url("http://example.com")
.build();

let _meter6 = provider.meter_with_scope(library.clone());

library.name = "Abc".into();
let _meter7 = provider.meter_with_scope(library.clone());
let make_scope = |name| {
InstrumentationScope::builder(name)
.with_version("1.0.0")
.with_schema_url("http://example.com")
.build()
};

library.name = "abc".into();
let _meter8 = provider.meter_with_scope(library);
let _meter6 = provider.meter_with_scope(make_scope("ABC"));
let _meter7 = provider.meter_with_scope(make_scope("Abc"));
let _meter8 = provider.meter_with_scope(make_scope("abc"));

assert_eq!(provider.inner.meters.lock().unwrap().len(), 5);
}
Expand Down
Loading
Loading