-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[exporterhelper] Add data_type attribute to internal queue metrics #10593
[exporterhelper] Add data_type attribute to internal queue metrics #10593
Conversation
74aea3e
to
dc8e02d
Compare
c240cb9
to
6e55752
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10593 +/- ##
==========================================
- Coverage 92.39% 92.38% -0.01%
==========================================
Files 403 403
Lines 18739 18741 +2
==========================================
+ Hits 17313 17314 +1
- Misses 1066 1067 +1
Partials 360 360 ☔ View full report in Codecov by Sentry. |
90567ba
to
e642443
Compare
Add data_type attribute to the internal otelcol_exporter_queue_size metric to report the type of data being processed. All other metrics have the data type reported as part of their names. We could've done the same for queue metrics, but that would introduce a significant breaking change. We want to avoid that until we have all the metrics standardized with OpenTelemetry semantic conventions.
e642443
to
b4b1968
Compare
@@ -7,6 +7,9 @@ const ( | |||
// ExporterKey used to identify exporters in metrics and traces. | |||
ExporterKey = "exporter" | |||
|
|||
// DataTypeKey used to identify the data type in the queue size metric. | |||
DataTypeKey = "data_type" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be what the otep recommends otel.signal
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about it. We use data_type
as a field for logging. Once the OTEP is ready and we adopt it, the metric is going to change anyway. Keeping an attribute name doesn't seem to help with the migration. Also, otel.signal
could still potentially change before the OTEP is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@codeboten I'm open to any attribute name. Let me know what do you think considering my previous comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmitryax probably ok to go with an attribute consistent w/ logging for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I searched through semantic conventions and couldn't find guidance for attributes that specify signal types. Should an issue be opened there?
Yeah, that's something that can be done before the OTEP. I'll create one |
Add
data_type
attribute to the internal otelcol_exporter_queue_size metric to report the type of data being processed.All other metrics have the data type reported as part of their names. We could've done the same for queue metrics, but that would introduce a significant breaking change. We want to avoid that until we have all the metrics standardized with OpenTelemetry semantic conventions.
Fixes #9943