Global error handler cleanup - Counter and Observable counter creation #2234
Conversation
| instrument_name = builder.name.as_ref(), | ||
| message = "Measurements from this counter will be ignored.", | ||
| reason = format!("{}", err) | ||
| ); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Utkarsh comment:
change
"Measurements from this instrument will be ignored." to
""Measurements from this counter will be ignored.",
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2234 +/- ##
=======================================
- Coverage 79.6% 79.5% -0.1%
=======================================
Files 121 121
Lines 21094 21113 +19
=======================================
Hits 16805 16805
- Misses 4289 4308 +19 ☔ View full report in Codecov by Sentry. |
| instrument_name = builder.name.as_ref(), | ||
| message = "Measurements from this counter will be ignored.", | ||
| reason = format!("{}", err) | ||
| ); |
There was a problem hiding this comment.
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.
| name: "InstrumentCreationFailed", | ||
| meter_name = self.scope.name.as_ref(), | ||
| instrument_name = builder.name.as_ref(), | ||
| message = "Measurements from this observable counter will be ignored.", |
There was a problem hiding this comment.
nit: if we are doing different message for each kind of instrument, then for observable, we need to call out that the registered callbacks will not be invoked.
There was a problem hiding this comment.
This is good point. Have updated it - Callbacks for this observable counter will not be invoked.
Fixes #
Design discussion issue (if applicable) #
Changes
Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes