-
Notifications
You must be signed in to change notification settings - Fork 802
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.Prometheus] - enable analysis #6171
base: main
Are you sure you want to change the base?
[Exporter.Prometheus] - enable analysis #6171
Conversation
8015a4c
to
50187dc
Compare
@@ -160,6 +160,9 @@ dotnet_diagnostic.RS0041.severity = suggestion | |||
# CA1515: Disable making types internal for Tests classes. It is required by xunit | |||
dotnet_diagnostic.CA1515.severity = none | |||
|
|||
# CA2007: Disable Consider calling ConfigureAwait on the awaited task. It is not working with xunit | |||
dotnet_diagnostic.CA2007.severity = none |
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.
did you intend to disable this for the entire repo or scope it to just the test projects?
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.
Tests only. It is done automatically by xunit, but CI executes dotnet format without restoring packages so it cannot be injected by xunit package.
{ | ||
#pragma warning disable CA2000 |
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: please include a comment here describing what is being disabled.
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.
@@ -62,9 +62,11 @@ public static MeterProviderBuilder AddPrometheusExporter( | |||
}); | |||
} | |||
|
|||
private static MetricReader BuildPrometheusExporterMetricReader(PrometheusAspNetCoreOptions options) | |||
private static BaseExportingMetricReader BuildPrometheusExporterMetricReader(PrometheusAspNetCoreOptions options) |
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.
FYI
CA1859 Change return type of method 'BuildPrometheusExporterMetricReader' from 'OpenTelemetry.Metrics.MetricReader' to 'OpenTelemetry.Metrics.BaseExportingMetricReader' for improved performance
db4d444
to
cbde98e
Compare
@TimothyMothra, I need to rebase, only new changes are in 3038ff0 (there is a instability after switching to the async) and e3e501b EDIT: Comment updates after nexr rebase |
dd19d54
to
3038ff0
Compare
Towards #3958
Changes
[Exporter.Prometheus] - enable analysis
Merge requirement checklist
[ ] Unit tests added/updated[ ] AppropriateCHANGELOG.md
files updated for non-trivial changes[ ] Changes in public API reviewed (if applicable)