-
Notifications
You must be signed in to change notification settings - Fork 772
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
Export OpenMetrics format for prometheus exporters #5107
Export OpenMetrics format for prometheus exporters #5107
Conversation
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializerExt.cs
Show resolved
Hide resolved
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusAspNetCoreOptions.cs
Outdated
Show resolved
Hide resolved
…b.com/robertcoltheart/opentelemetry-dotnet into feature/export-prometheus-openmetrics
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs
Outdated
Show resolved
Hide resolved
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.
LGTM with one suggestion https://github.com/open-telemetry/opentelemetry-dotnet/pull/5107/files#r1414630802.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #5107 +/- ##
==========================================
+ Coverage 83.06% 83.13% +0.06%
==========================================
Files 296 297 +1
Lines 12320 12369 +49
==========================================
+ Hits 10234 10283 +49
Misses 2086 2086
Flags with carried forward coverage won't be shown. Click here to find out more.
|
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs
Outdated
Show resolved
Hide resolved
…orterMiddleware.cs Co-authored-by: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com>
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs
Outdated
Show resolved
Hide resolved
…orterMiddleware.cs Co-authored-by: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com>
src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs
Outdated
Show resolved
Hide resolved
…ttpListener.cs Co-authored-by: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com>
src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusExporter.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusCollectionManager.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusHeadersParser.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusHeadersParser.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Exporter.Prometheus.HttpListener/PrometheusHttpListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/PrometheusExporterMiddleware.cs
Outdated
Show resolved
Hide resolved
.AddMeter(meter.Name) | ||
.AddPrometheusHttpListener(options => options.UriPrefixes = new string[] { address }) | ||
.Build(); | ||
try |
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.
nit: It's simpler to not have the try/catch
block here. This is a test so it's okay for it to fail with the original exception. Anyone troubleshooting the test failure would realize what the issue was anyway.
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.
In this case, I'm guessing its because there might be a potential conflict with an already open port? I can remove in the next PR in any case.
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.
In this case, I'm guessing its because there might be a potential conflict with an already open port?
The tests within a project are not run in parallel so it shouldn't be the case unless the listener instances are not getting disposed correctly.
Thanks very much! Will crack on with the next PR. |
@robertcoltheart Thanks for your contribution! |
Fixes #
First part to addressing #3972
Changes
Export OpenMetrics format for Prometheus. See here.
Accept
parser and set correctContent-Type
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial changes