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.

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

Global error handler cleanup - Counter and Observable counter creation #2234

Merged
merged 7 commits into from
Oct 24, 2024
Merged

Global error handler cleanup - Counter and Observable counter creation #2234

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8a0e56c
internal-log counter and observable counter
lalitb Oct 24, 2024
47266ef
comment.
lalitb Oct 24, 2024
6c6d2d5
spell
lalitb Oct 24, 2024
35e62fb
Merge branch 'main' into instrument-creation-internal-log
lalitb Oct 24, 2024
2b71415
comment
lalitb Oct 24, 2024
ed5bedf
comment
lalitb Oct 24, 2024
d2d462f
comment
lalitb Oct 24, 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
41 changes: 35 additions & 6 deletions opentelemetry-sdk/src/metrics/meter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
InstrumentProvider, MetricsError, ObservableCounter, ObservableGauge,
ObservableUpDownCounter, Result, UpDownCounter,
},
otel_error,
};

use crate::instrumentation::Scope;
Expand Down Expand Up @@ -75,14 +76,20 @@
{
let validation_result = validate_instrument_config(builder.name.as_ref(), &builder.unit);
if let Err(err) = validation_result {
global::handle_error(err);
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
instrument_name = builder.name.as_ref(),
message = "Measurements from this counter will be ignored.",
reason = format!("{}", err)

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

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L81-L84 

Added lines #L81 - L84 were not covered by tests
);
Copy link
Member Author

Choose a reason for hiding this comment

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

Cijo's comment:
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
instrument_name = builder.name.as_ref(),
message = "Measurements from this instrument will be ignored."
reason = fmt(err))

^ I prefer this version. Please see if this is better for end users.

Copy link
Member Author

Choose a reason for hiding this comment

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

Utkarsh comment:
change
"Measurements from this instrument will be ignored." to
""Measurements from this counter will be ignored.",

Copy link
Member

Choose a reason for hiding this comment

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

2024-10-24T05:32:23.524520Z ERROR main opentelemetry_sdk meter_name="mylibraryname" instrument_name="-my_counter" Measurements from this counter will be ignored. reason="Invalid instrument configuration: instrument name must start with an alphabetic character"

^ This is what we get when I used fmt module. The reason is a slightly extra verbose:
Current: Invalid instrument configuration: instrument name must start with an alphabetic character
Proposal: Instrument name must start with an alphabetic character

This is a minor thing we can change in the future.

return Counter::new(Arc::new(NoopSyncInstrument::new()));
}

match resolver
.lookup(
InstrumentKind::Counter,
builder.name,
builder.name.clone(),
builder.description,
builder.unit,
None,
Expand All @@ -91,7 +98,13 @@
{
Ok(counter) => counter,
Err(err) => {
global::handle_error(err);
otel_error!(

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
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
instrument_name = builder.name.as_ref(),
message = "Measurements from this counter will be ignored.",
reason = format!("{}", err)

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

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L103-L106 

Added lines #L103 - L106 were not covered by tests
);
Counter::new(Arc::new(NoopSyncInstrument::new()))
}
}
Expand All @@ -107,19 +120,30 @@
{
let validation_result = validate_instrument_config(builder.name.as_ref(), &builder.unit);
if let Err(err) = validation_result {
global::handle_error(err);
otel_error!(
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
instrument_name = builder.name.as_ref(),
message = "Callbacks for this observable counter will not be invoked.",
reason = format!("{}", err));

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

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L125-L128 

Added lines #L125 - L128 were not covered by tests
return ObservableCounter::new();
}

match resolver.measures(
InstrumentKind::ObservableCounter,
builder.name,
builder.name.clone(),
builder.description,
builder.unit,
None,
) {
Ok(ms) => {
if ms.is_empty() {
otel_error!(

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
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
instrument_name = builder.name.as_ref(),

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

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L143-L144 

Added lines #L143 - L144 were not covered by tests
message = "Callbacks for this observable counter will not be invoked. Check View Configuration."
);
return ObservableCounter::new();
}

Expand All @@ -134,7 +158,12 @@
ObservableCounter::new()
}
Err(err) => {
global::handle_error(err);
otel_error!(

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
name: "InstrumentCreationFailed",
meter_name = self.scope.name.as_ref(),
instrument_name = builder.name.as_ref(),
message = "Callbacks for this observable counter will not be invoked.",
reason = format!("{}", err));

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

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L163-L166 

Added lines #L163 - L166 were not covered by tests
ObservableCounter::new()
}
}
Expand Down
Loading