-
Notifications
You must be signed in to change notification settings - Fork 834
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
Prometheus compatibility #5039
Prometheus compatibility #5039
Conversation
assertThat(resourceMetrics.getResource().getAttributesList()) | ||
.containsExactlyInAnyOrder( | ||
// Resource attributes derived from the prometheus scrape config | ||
stringKeyValue("service.name", "app"), |
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.
It's pretty confusing that we have both service.name
from scraping (and it contains a dot - shouldn't that be converted to an underscore?) and service_name
from the resource. Well, nothing we can do about that in this PR.
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.
service.name
comes from job_name
that produced the scrape in the promtheus receiver config. service_name
comes from the attributes of the target_info
metric from the app's prometheus exporter.
I agree it's confusing. Best case would be for the prometheus receiver to recognize that a target_info
is present, try to reverse the attribute key transformation based on semantic conventions (i.e. transform service_name
back to service.name
), and prioritize the attributes from target_info
over those generated from the job config.
…y-java into prometheus-compatibility
Codecov ReportBase: 91.19% // Head: 91.23% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #5039 +/- ##
============================================
+ Coverage 91.19% 91.23% +0.04%
- Complexity 4877 4883 +6
============================================
Files 553 553
Lines 14402 14449 +47
Branches 1375 1379 +4
============================================
+ Hits 13134 13183 +49
+ Misses 879 878 -1
+ Partials 389 388 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
* PrometheusHttpServer serializes resource attributes in target_info * PrometheusHttpServer serializes scope as otel_scope_info
Includes resource attribute in
target_info
metric. Previously attempted in #4569. This has been discussed in the spec and by the prometheus WG and the spec is unambiguous. We really need to get this behavior implemented.Example:
Serialize scope as
otel_scope_info
metric. Include scope name and version attributes on each metric to disambiguate from metrics with the same name in different scopes. Related to #5014, but doesn't solve the collision issue.Example:
Includes a new
PrometheusHttpServerIntegrationTest
to demonstrate the behavior end to end from: SDK -> collector prometheus receiver -> collector OTLP exporter -> mock OTLP server.Our prometheus exporter is really starting to fall behind the spec. This PR is the first of a couple needed to get it back on course.