-
Notifications
You must be signed in to change notification settings - Fork 909
Update prom client with support for UTF-8 #7588
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
6a89d7e
28ae227
a5b2666
4746088
b701341
894b226
5842f02
9d9153e
b986d2f
4c214e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,9 @@ public MetricReader createMetricReader(ConfigProperties config) { | |
prometheusBuilder.setHost(host); | ||
} | ||
|
||
prometheusBuilder.setUtf8SupportEnabled( | ||
config.getBoolean("otel.exporter.prometheus.utf8", false)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the official property? I thought no new properties were being added to the spec? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #7588 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That doesn't answer my question... if we're adding new configuration properties, I'd like to make sure they're "official" in the spec, and we're not going off on our own, especially as there has been a moratorium set on the new configuration properties/env vars in the spec (at least, the last I heard). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it 😄 This is the part in the Go SDK: https://github.com/open-telemetry/opentelemetry-go/blob/d0cab8666b740c975f028236610cab2663f02031/exporters/prometheus/config.go#L44-L66 @ArthurSens it seems there is no env var to set the translation strategy - is that missing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. got the answer: it should be |
||
|
||
ExporterBuilderUtil.configureExporterMemoryMode(config, prometheusBuilder::setMemoryMode); | ||
|
||
String defaultHistogramAggregation = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,25 +133,24 @@ void endToEnd() { | |
assertThat(resourceMetrics.getResource().getAttributesList()) | ||
.containsExactlyInAnyOrder( | ||
// Resource attributes derived from the prometheus scrape config | ||
stringKeyValue("service.name", "app"), | ||
stringKeyValue("service.instance.id", "host.testcontainers.internal:" + prometheusPort), | ||
stringKeyValue("server.address", "host.testcontainers.internal"), | ||
stringKeyValue("server.port", String.valueOf(prometheusPort)), | ||
stringKeyValue("url.scheme", "http"), | ||
// Resource attributes from the metric SDK resource translated to target_info | ||
stringKeyValue( | ||
"service_name", | ||
"service.name", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why did these keys change, when we haven't explicitly changed the exporter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I set UTF-8 to true in most tests - including here |
||
Objects.requireNonNull(resource.getAttributes().get(stringKey("service.name")))), | ||
stringKeyValue( | ||
"telemetry_sdk_name", | ||
"telemetry.sdk.name", | ||
Objects.requireNonNull( | ||
resource.getAttributes().get(stringKey("telemetry.sdk.name")))), | ||
stringKeyValue( | ||
"telemetry_sdk_language", | ||
"telemetry.sdk.language", | ||
Objects.requireNonNull( | ||
resource.getAttributes().get(stringKey("telemetry.sdk.language")))), | ||
stringKeyValue( | ||
"telemetry_sdk_version", | ||
"telemetry.sdk.version", | ||
Objects.requireNonNull( | ||
resource.getAttributes().get(stringKey("telemetry.sdk.version"))))); | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.