Skip to content
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

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
fcd5133
[Exporter.Prometheus] enable code analysis
Kielek Mar 4, 2025
dc1faa5
CA1822
Kielek Mar 4, 2025
5a4c569
CA1310
Kielek Mar 4, 2025
6cbb691
Disable CA2000 - ownership is transferred to other objects
Kielek Mar 4, 2025
9ceef3a
CA1859
Kielek Mar 4, 2025
c26527a
CA1823
Kielek Mar 4, 2025
34e363b
CA1835
Kielek Mar 4, 2025
c36a8a4
IDE0300
Kielek Mar 4, 2025
5c50b48
IDE0301
Kielek Mar 4, 2025
325db5b
IDE0090
Kielek Mar 4, 2025
72d7bd8
IDE0074
Kielek Mar 4, 2025
bf04484
IDE0028
Kielek Mar 4, 2025
b3fb89d
IDE0032
Kielek Mar 4, 2025
c412172
CA1062
Kielek Mar 4, 2025
260277c
Enable for tests
Kielek Mar 5, 2025
ede8224
CA1305 - Tests
Kielek Mar 5, 2025
582479e
CA1307 - Tests
Kielek Mar 5, 2025
45cf3ae
CA1310 - Tests
Kielek Mar 5, 2025
6f8db2c
CA1825 - Tests
Kielek Mar 5, 2025
0b270a7
CA1822 - Tests
Kielek Mar 5, 2025
984c4ed
IDE0301 - Tests
Kielek Mar 5, 2025
0f48075
IDE0305 - Tests
Kielek Mar 5, 2025
bf0ff33
IDE0046 - Tests
Kielek Mar 5, 2025
bf4fbf5
CS4032
Kielek Mar 5, 2025
f381f7e
CA1852 - Tests
Kielek Mar 5, 2025
d763a8e
CA1860 - Tests
Kielek Mar 5, 2025
5eacfb3
CS0176 - Tests
Kielek Mar 7, 2025
65d7460
CA2213 - Tests
Kielek Mar 7, 2025
781c1cf
CA2234 - Tests
Kielek Mar 7, 2025
515e7df
CA2201 - Tests
Kielek Mar 7, 2025
6d36eeb
Ignore CA2000 - Tests - it should be disposed by other object
Kielek Mar 7, 2025
26acef7
Disable CA2007 for tests
Kielek Mar 7, 2025
eee5b5f
Disable CA5394 - nothing to secure here
Kielek Mar 7, 2025
1fae378
IDE0028 - Tests
Kielek Mar 7, 2025
665de90
CA1852 - Tests
Kielek Mar 7, 2025
60109a1
CA1515 - Tests
Kielek Mar 7, 2025
6023b6c
Fix parsing Uri - tests
Kielek Mar 7, 2025
19a2212
#pragma warning description
Kielek Mar 11, 2025
e3e501b
Avoid same names in different contexts
Kielek Mar 11, 2025
3038ff0
Disable CA1849
Kielek Mar 12, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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?

Copy link
Contributor Author

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.


[**/obj/**.cs]
generated_code = true

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<PackageTags>$(PackageTags);prometheus;metrics</PackageTags>
<MinVerTagPrefix>coreunstable-</MinVerTagPrefix>
<DefineConstants>$(DefineConstants);PROMETHEUS_ASPNETCORE</DefineConstants>
<AnalysisLevel>latest-all</AnalysisLevel>
</PropertyGroup>

<ItemGroup Condition="'$(RunningDotNetPack)' != 'true'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ public static IApplicationBuilder UseOpenTelemetryPrometheusScrapingEndpoint(
Action<IApplicationBuilder>? configureBranchedPipeline,
string? optionsName)
{
Guard.ThrowIfNull(app);

// Note: Order is important here. MeterProvider is accessed before
// GetOptions<PrometheusAspNetCoreOptions> so that any changes made to
// PrometheusAspNetCoreOptions in deferred AddPrometheusExporter
Expand All @@ -114,7 +116,7 @@ public static IApplicationBuilder UseOpenTelemetryPrometheusScrapingEndpoint(
path = options.ScrapeEndpointPath ?? PrometheusAspNetCoreOptions.DefaultScrapeEndpointPath;
}

if (!path.StartsWith("/"))
if (!path.StartsWith('/'))
{
path = $"/{path}";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ public static IEndpointConventionBuilder MapPrometheusScrapingEndpoint(
Action<IApplicationBuilder>? configureBranchedPipeline,
string? optionsName)
{
Guard.ThrowIfNull(endpoints);

var builder = endpoints.CreateApplicationBuilder();

// Note: Order is important here. MeterProvider is accessed before
Expand All @@ -84,7 +86,7 @@ public static IEndpointConventionBuilder MapPrometheusScrapingEndpoint(
path = options.ScrapeEndpointPath ?? PrometheusAspNetCoreOptions.DefaultScrapeEndpointPath;
}

if (!path.StartsWith("/"))
if (!path.StartsWith('/'))
{
path = $"/{path}";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,11 @@ public static MeterProviderBuilder AddPrometheusExporter(
});
}

private static MetricReader BuildPrometheusExporterMetricReader(PrometheusAspNetCoreOptions options)
private static BaseExportingMetricReader BuildPrometheusExporterMetricReader(PrometheusAspNetCoreOptions options)
Copy link
Contributor

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

{
#pragma warning disable CA2000 // Dispose objects before losing scope
var exporter = new PrometheusExporter(options.ExporterOptions);
#pragma warning restore CA2000 // Dispose objects before losing scope

return new BaseExportingMetricReader(exporter)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public async Task InvokeAsync(HttpContext httpContext)
? "application/openmetrics-text; version=1.0.0; charset=utf-8"
: "text/plain; charset=utf-8; version=0.0.4";

await response.Body.WriteAsync(dataView.Array!, 0, dataView.Count).ConfigureAwait(false);
await response.Body.WriteAsync(dataView.Array.AsMemory(0, dataView.Count)).ConfigureAwait(false);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ public PrometheusCollectionManager(PrometheusExporter exporter)
this.exporter = exporter;
this.scrapeResponseCacheDurationMilliseconds = this.exporter.ScrapeResponseCacheDurationMilliseconds;
this.onCollectRef = this.OnCollect;
this.metricsCache = new Dictionary<Metric, PrometheusMetric>();
this.scopes = new HashSet<string>();
this.metricsCache = [];
this.scopes = [];
}

#if NET
Expand Down Expand Up @@ -68,10 +68,7 @@ public Task<CollectionResponse> EnterCollect(bool openMetricsRequested)
// If a collection is already running, return a task to wait on the result.
if (this.collectionRunning)
{
if (this.collectionTcs == null)
{
this.collectionTcs = new TaskCompletionSource<CollectionResponse>(TaskCreationOptions.RunContinuationsAsynchronously);
}
this.collectionTcs ??= new TaskCompletionSource<CollectionResponse>(TaskCreationOptions.RunContinuationsAsynchronously);

Interlocked.Increment(ref this.readerCount);
this.ExitGlobalLock();
Expand Down Expand Up @@ -148,6 +145,22 @@ public void ExitCollect()
Interlocked.Decrement(ref this.readerCount);
}

private static bool IncreaseBufferSize(ref byte[] buffer)
{
var newBufferSize = buffer.Length * 2;

if (newBufferSize > 100 * 1024 * 1024)
{
return false;
}

var newBuffer = new byte[newBufferSize];
buffer.CopyTo(newBuffer, 0);
buffer = newBuffer;

return true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void EnterGlobalLock()
{
Expand Down Expand Up @@ -230,7 +243,7 @@ private ExportResult OnCollect(in Batch<Metric> metrics)
}
catch (IndexOutOfRangeException)
{
if (!this.IncreaseBufferSize(ref buffer))
if (!IncreaseBufferSize(ref buffer))
{
// there are two cases we might run into the following condition:
// 1. we have many metrics to be exported - in this case we probably want
Expand Down Expand Up @@ -268,7 +281,7 @@ private ExportResult OnCollect(in Batch<Metric> metrics)
}
catch (IndexOutOfRangeException)
{
if (!this.IncreaseBufferSize(ref buffer))
if (!IncreaseBufferSize(ref buffer))
{
throw;
}
Expand All @@ -285,7 +298,7 @@ private ExportResult OnCollect(in Batch<Metric> metrics)
}
catch (IndexOutOfRangeException)
{
if (!this.IncreaseBufferSize(ref buffer))
if (!IncreaseBufferSize(ref buffer))
{
throw;
}
Expand All @@ -307,11 +320,11 @@ private ExportResult OnCollect(in Batch<Metric> metrics)
{
if (this.exporter.OpenMetricsRequested)
{
this.previousOpenMetricsDataView = new ArraySegment<byte>(Array.Empty<byte>(), 0, 0);
this.previousOpenMetricsDataView = new ArraySegment<byte>([], 0, 0);
}
else
{
this.previousPlainTextDataView = new ArraySegment<byte>(Array.Empty<byte>(), 0, 0);
this.previousPlainTextDataView = new ArraySegment<byte>([], 0, 0);
}

return ExportResult.Failure;
Expand All @@ -332,7 +345,7 @@ private int WriteTargetInfo(ref byte[] buffer)
}
catch (IndexOutOfRangeException)
{
if (!this.IncreaseBufferSize(ref buffer))
if (!IncreaseBufferSize(ref buffer))
{
throw;
}
Expand All @@ -343,22 +356,6 @@ private int WriteTargetInfo(ref byte[] buffer)
return this.targetInfoBufferLength;
}

private bool IncreaseBufferSize(ref byte[] buffer)
{
var newBufferSize = buffer.Length * 2;

if (newBufferSize > 100 * 1024 * 1024)
{
return false;
}

var newBuffer = new byte[newBufferSize];
buffer.CopyTo(newBuffer, 0);
buffer = newBuffer;

return true;
}

private PrometheusMetric GetPrometheusMetric(Metric metric)
{
// Optimize writing metrics with bounded cache that has pre-calculated Prometheus names.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ Histogram becomes histogram
UpDownCounter becomes gauge
* https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#otlp-metric-points-to-prometheus
*/
private static readonly PrometheusType[] MetricTypes = new PrometheusType[]
{
private static readonly PrometheusType[] MetricTypes =
[
PrometheusType.Untyped, PrometheusType.Counter, PrometheusType.Gauge, PrometheusType.Summary, PrometheusType.Histogram, PrometheusType.Histogram, PrometheusType.Histogram, PrometheusType.Histogram, PrometheusType.Gauge,
};
];

public PrometheusMetric(string name, string unit, PrometheusType type, bool disableTotalNameSuffixForCounters)
{
Expand All @@ -40,7 +40,7 @@ public PrometheusMetric(string name, string unit, PrometheusType type, bool disa
// [OpenMetrics UNIT metadata](https://github.com/prometheus/OpenMetrics/blob/v1.0.0/specification/OpenMetrics.md#metricfamily)
// and as a suffix to the metric name. The unit suffix comes before any type-specific suffixes.
// https://github.com/open-telemetry/opentelemetry-specification/blob/3dfb383fe583e3b74a2365c5a1d90256b273ee76/specification/compatibility/prometheus_and_openmetrics.md#metric-metadata-1
if (!sanitizedName.EndsWith(sanitizedUnit))
if (!sanitizedName.EndsWith(sanitizedUnit, StringComparison.Ordinal))
{
sanitizedName += $"_{sanitizedUnit}";
openMetricsName += $"_{sanitizedUnit}";
Expand All @@ -51,14 +51,14 @@ public PrometheusMetric(string name, string unit, PrometheusType type, bool disa
// Exporters SHOULD provide a configuration option to disable the addition of `_total` suffixes.
// https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#L286
// Note that we no longer append '_ratio' for units that are '1', see: https://github.com/open-telemetry/opentelemetry-specification/issues/4058
if (type == PrometheusType.Counter && !sanitizedName.EndsWith("_total") && !disableTotalNameSuffixForCounters)
if (type == PrometheusType.Counter && !sanitizedName.EndsWith("_total", StringComparison.Ordinal) && !disableTotalNameSuffixForCounters)
{
sanitizedName += "_total";
}

// For counters requested using OpenMetrics format, the MetricFamily name MUST be suffixed with '_total', regardless of the setting to disable the 'total' suffix.
// https://github.com/prometheus/OpenMetrics/blob/v1.0.0/specification/OpenMetrics.md#counter-1
if (type == PrometheusType.Counter && !openMetricsName.EndsWith("_total"))
if (type == PrometheusType.Counter && !openMetricsName.EndsWith("_total", StringComparison.Ordinal))
{
openMetricsName += "_total";
}
Expand Down Expand Up @@ -127,7 +127,7 @@ internal static string SanitizeMetricName(string metricName)

return sb?.ToString() ?? metricName;

static StringBuilder CreateStringBuilder(string name) => new StringBuilder(name.Length);
static StringBuilder CreateStringBuilder(string name) => new(name.Length);
}

internal static string RemoveAnnotations(string unit)
Expand Down Expand Up @@ -179,7 +179,7 @@ internal static string RemoveAnnotations(string unit)

private static string SanitizeOpenMetricsName(string metricName)
{
if (metricName.EndsWith("_total"))
if (metricName.EndsWith("_total", StringComparison.Ordinal))
{
return metricName.Substring(0, metricName.Length - 6);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ internal static partial class PrometheusSerializer
{
#pragma warning disable SA1310 // Field name should not contain an underscore
private const byte ASCII_QUOTATION_MARK = 0x22; // '"'
private const byte ASCII_FULL_STOP = 0x2E; // '.'
private const byte ASCII_HYPHEN_MINUS = 0x2D; // '-'
private const byte ASCII_REVERSE_SOLIDUS = 0x5C; // '\\'
private const byte ASCII_LINEFEED = 0x0A; // `\n`
#pragma warning restore SA1310 // Field name should not contain an underscore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<Description>Stand-alone HttpListener for hosting OpenTelemetry .NET Prometheus Exporter</Description>
<PackageTags>$(PackageTags);prometheus;metrics</PackageTags>
<MinVerTagPrefix>coreunstable-</MinVerTagPrefix>
<AnalysisLevel>latest-all</AnalysisLevel>
</PropertyGroup>

<ItemGroup Condition="'$(RunningDotNetPack)' != 'true'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,20 @@ public PrometheusHttpListener(PrometheusExporter exporter, PrometheusHttpListene

string path = options.ScrapeEndpointPath ?? PrometheusHttpListenerOptions.DefaultScrapeEndpointPath;

if (!path.StartsWith("/"))
#if NET
if (!path.StartsWith('/'))
#else
if (!path.StartsWith("/", StringComparison.Ordinal))
#endif
{
path = $"/{path}";
}

if (!path.EndsWith("/"))
#if NET
if (!path.EndsWith('/'))
#else
if (!path.EndsWith("/", StringComparison.Ordinal))
#endif
{
path = $"{path}/";
}
Expand Down Expand Up @@ -164,7 +172,11 @@ private async Task ProcessRequestAsync(HttpListenerContext context)
? "application/openmetrics-text; version=1.0.0; charset=utf-8"
: "text/plain; charset=utf-8; version=0.0.4";

await context.Response.OutputStream.WriteAsync(dataView.Array!, 0, dataView.Count).ConfigureAwait(false);
#if NET
await context.Response.OutputStream.WriteAsync(dataView.Array.AsMemory(0, dataView.Count)).ConfigureAwait(false);
#else
await context.Response.OutputStream.WriteAsync(dataView.Array, 0, dataView.Count).ConfigureAwait(false);
#endif
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public static MeterProviderBuilder AddPrometheusHttpListener(
});
}

private static MetricReader BuildPrometheusHttpListenerMetricReader(
private static BaseExportingMetricReader BuildPrometheusHttpListenerMetricReader(
PrometheusHttpListenerOptions options)
{
var exporter = new PrometheusExporter(new PrometheusExporterOptions
Expand All @@ -78,8 +78,10 @@ private static MetricReader BuildPrometheusHttpListenerMetricReader(

try
{
#pragma warning disable CA2000 // Dispose objects before losing scope
var listener = new PrometheusHttpListener(exporter, options);
exporter.OnDispose = () => listener.Dispose();
#pragma warning restore CA2000 // Dispose objects before losing scope
exporter.OnDispose = listener.Dispose;
listener.Start();
}
catch (Exception ex)
Expand Down
11 changes: 5 additions & 6 deletions src/OpenTelemetry/Metrics/Reader/BaseExportingMetricReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ public class BaseExportingMetricReader : MetricReader
/// </summary>
protected readonly BaseExporter<Metric> exporter;

private readonly ExportModes supportedExportModes = ExportModes.Push | ExportModes.Pull;
private readonly string exportCalledMessage;
private readonly string exportSucceededMessage;
private readonly string exportFailedMessage;
Expand All @@ -38,12 +37,12 @@ public BaseExportingMetricReader(BaseExporter<Metric> exporter)
if (attributes.Length > 0)
{
var attr = (ExportModesAttribute)attributes[attributes.Length - 1];
this.supportedExportModes = attr.Supported;
this.SupportedExportModes = attr.Supported;
}

if (exporter is IPullMetricExporter pullExporter)
{
if (this.supportedExportModes.HasFlag(ExportModes.Push))
if (this.SupportedExportModes.HasFlag(ExportModes.Push))
{
pullExporter.Collect = this.Collect;
}
Expand All @@ -69,7 +68,7 @@ public BaseExportingMetricReader(BaseExporter<Metric> exporter)
/// <summary>
/// Gets the supported <see cref="ExportModes"/>.
/// </summary>
protected ExportModes SupportedExportModes => this.supportedExportModes;
protected ExportModes SupportedExportModes { get; } = ExportModes.Push | ExportModes.Pull;

internal override void SetParentProvider(BaseProvider parentProvider)
{
Expand Down Expand Up @@ -106,12 +105,12 @@ internal override bool ProcessMetrics(in Batch<Metric> metrics, int timeoutMilli
/// <inheritdoc />
protected override bool OnCollect(int timeoutMilliseconds)
{
if (this.supportedExportModes.HasFlag(ExportModes.Push))
if (this.SupportedExportModes.HasFlag(ExportModes.Push))
{
return base.OnCollect(timeoutMilliseconds);
}

if (this.supportedExportModes.HasFlag(ExportModes.Pull) && PullMetricScope.IsPullAllowed)
if (this.SupportedExportModes.HasFlag(ExportModes.Pull) && PullMetricScope.IsPullAllowed)
{
return base.OnCollect(timeoutMilliseconds);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<Description>Unit test project for Prometheus Exporter AspNetCore for OpenTelemetry</Description>
<TargetFrameworks>$(TargetFrameworksForAspNetCoreTests)</TargetFrameworks>
<DefineConstants>$(DefineConstants);PROMETHEUS_ASPNETCORE</DefineConstants>
<AnalysisLevel>latest-all</AnalysisLevel>
</PropertyGroup>

<ItemGroup>
Expand All @@ -28,7 +29,7 @@
</ItemGroup>

<ItemGroup>
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Exporter.Prometheus.HttpListener.Tests\EventSourceTest.cs" Link="Includes\EventSourceTest.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Exporter.Prometheus.HttpListener.Tests\EventSourceTests.cs" Link="Includes\EventSourceTests.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Exporter.Prometheus.HttpListener.Tests\PrometheusCollectionManagerTests.cs" Link="Includes\PrometheusCollectionManagerTests.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Exporter.Prometheus.HttpListener.Tests\PrometheusSerializerTests.cs" Link="Includes\PrometheusSerializerTests.cs" />

Expand Down
Loading
Loading