-
Notifications
You must be signed in to change notification settings - Fork 786
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
Update default host and port for PrometheusExporter to align with the OTel spec. #2783
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2783 +/- ##
=======================================
Coverage 83.75% 83.75%
=======================================
Files 251 251
Lines 8864 8864
=======================================
Hits 7424 7424
Misses 1440 1440
|
Couple things to discuss:
|
Ah apologies, didn't see the open questions. Something made me think the default had already been decided.
Looks like Java's default is 9464 Appears there is some precedent for 9464 as here's the OpenCensus exporter for node https://github.com/census-instrumentation/opencensus-node/blob/master/packages/opencensus-exporter-prometheus/README.md. The spec references it too https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/sdk-environment-variables.md#prometheus-exporter |
Interesting! Not sure how did people come up with 9464. BTW, here contains the list of ports that we may need to avoid when choosing default ports. |
src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterOptions.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Reiley Yang <reyang@microsoft.com>
Fixes #.
#2780
Changes
Update default httpListenerPrefixes for PrometheusExporter to be http://localhost:9464/ to align with the spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/sdk-environment-variables.md#prometheus-exporter
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes