-
Notifications
You must be signed in to change notification settings - Fork 773
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
[Exporter.PrometheusAspNetCore] update Public API #3717
[Exporter.PrometheusAspNetCore] update Public API #3717
Conversation
00c7fab
to
2506e06
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3717 +/- ##
==========================================
- Coverage 87.32% 87.08% -0.24%
==========================================
Files 274 275 +1
Lines 10009 10012 +3
==========================================
- Hits 8740 8719 -21
- Misses 1269 1293 +24
|
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
@open-telemetry/dotnet-maintainers please check and possibly merge. |
@@ -51,5 +52,7 @@ public IReadOnlyCollection<string> UriPrefixes | |||
this.uriPrefixes = value; | |||
} | |||
} | |||
|
|||
internal PrometheusExporterOptions ExporterOptions { get; } = new(); |
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.
This is not required.
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.
Left a suggestion.
Fixed: db2f4fc |
Public API changes from #3694 proposed by @CodeBlanch.
Changes
It will allow us to add more properties later for all Prometheus exporters.
There is no stable release for this package, so it should be fine to adjust class name.
CHANGELOG.md
updated for non-trivial changes[ ] Design discussion issue #