Skip to content

Commit

Permalink
Fix collection output buffer management when its resized
Browse files Browse the repository at this point in the history
When buffer is too small to fit its initial 85kB size, its resized to fit the output, however the resized buffer is not used in final output. This ultimately results in overflow exception as cursor is correct and follows resized buffer.
I also changed to use the same buffer variable for WriteTargetInfo, it also can resize the buffer.
  • Loading branch information
jcin193 committed Jun 7, 2024
1 parent 90871e0 commit ef1ff92
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Fix collection output buffer management when its resized
([#5661](https://github.com/open-telemetry/opentelemetry-dotnet/issues/5661))

## 1.9.0-alpha.2

Released 2024-May-29
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,13 @@ private bool ExecuteCollect(bool openMetricsRequested)
private ExportResult OnCollect(Batch<Metric> metrics)
{
var cursor = 0;
var buffer = this.exporter.OpenMetricsRequested ? this.openMetricsBuffer : this.plainTextBuffer;
ref byte[] buffer = ref (this.exporter.OpenMetricsRequested ? ref this.openMetricsBuffer : ref this.plainTextBuffer);

try
{
if (this.exporter.OpenMetricsRequested)
{
cursor = this.WriteTargetInfo();
cursor = this.WriteTargetInfo(ref buffer);

this.scopes.Clear();

Expand Down Expand Up @@ -291,11 +291,11 @@ private ExportResult OnCollect(Batch<Metric> metrics)

if (this.exporter.OpenMetricsRequested)
{
this.previousOpenMetricsDataView = new ArraySegment<byte>(this.openMetricsBuffer, 0, cursor);
this.previousOpenMetricsDataView = new ArraySegment<byte>(buffer, 0, cursor);
}
else
{
this.previousPlainTextDataView = new ArraySegment<byte>(this.plainTextBuffer, 0, cursor);
this.previousPlainTextDataView = new ArraySegment<byte>(buffer, 0, cursor);
}

return ExportResult.Success;
Expand All @@ -315,21 +315,21 @@ private ExportResult OnCollect(Batch<Metric> metrics)
}
}

private int WriteTargetInfo()
private int WriteTargetInfo(ref byte[] buffer)
{
if (this.targetInfoBufferLength < 0)
{
while (true)
{
try
{
this.targetInfoBufferLength = PrometheusSerializer.WriteTargetInfo(this.openMetricsBuffer, 0, this.exporter.Resource);
this.targetInfoBufferLength = PrometheusSerializer.WriteTargetInfo(buffer, 0, this.exporter.Resource);

break;
}
catch (IndexOutOfRangeException)
{
if (!this.IncreaseBufferSize(ref this.openMetricsBuffer))
if (!this.IncreaseBufferSize(ref buffer))
{
throw;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,30 @@ public async Task PrometheusExporterMiddlewareIntegration_CanServeOpenMetricsAnd
await host.StopAsync();
}

[Fact]
public async Task PrometheusExporterMiddlewareIntegration_ALotOfMetrics()
{
using var host = await StartTestHostAsync(
app => app.UseOpenTelemetryPrometheusScrapingEndpoint());

using var meter = new Meter(MeterName, MeterVersion);

for (var x = 0; x < 1000; x++)
{
var counter = meter.CreateCounter<double>("counter_double_" + x, unit: "By");
counter.Add(1);
}

using var client = host.GetTestClient();

using var response = await client.GetAsync("/metrics");
var text = await response.Content.ReadAsStringAsync();

Assert.NotEmpty(text);

await host.StopAsync();
}

private static async Task RunPrometheusExporterMiddlewareIntegrationTest(
string path,
Action<IApplicationBuilder> configure,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System;
using System.Diagnostics.Metrics;
using System.Net;
#if NETFRAMEWORK
Expand All @@ -19,6 +20,48 @@ public class PrometheusHttpListenerTests

private static readonly string MeterName = Utils.GetCurrentMethodName();

private static MeterProvider BuildMeterProvider(Meter meter, IEnumerable<KeyValuePair<string, object>> attributes, out string address)
{
Random random = new Random();
int retryAttempts = 5;
int port = 0;
string generatedAddress = null;
MeterProvider provider = null;

while (retryAttempts-- != 0)
{
port = random.Next(2000, 5000);
generatedAddress = $"http://localhost:{port}/";

try
{
provider = Sdk.CreateMeterProviderBuilder()
.AddMeter(meter.Name)
.ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1").AddAttributes(attributes))
.AddPrometheusHttpListener(options =>
{
options.UriPrefixes = new string[] { generatedAddress };
})
.Build();

break;
}
catch
{
// ignored
}
}

address = generatedAddress;

if (provider == null)
{
throw new InvalidOperationException("HttpListener could not be started");
}

return provider;
}

[Theory]
[InlineData("http://+:9464")]
[InlineData("http://*:9464")]
Expand Down Expand Up @@ -144,6 +187,45 @@ public void PrometheusHttpListenerThrowsOnStart()
listener?.Dispose();
}

[Theory]
[InlineData("application/openmetrics-text")]
[InlineData("")]
public async Task PrometheusExporterHttpServerIntegration_LargePayload(string acceptHeader)
{
using var meter = new Meter(MeterName, MeterVersion);

var attributes = new List<KeyValuePair<string, object>>();
var oneKb = new string('A', 1024);
for (var x = 0; x < 8500; x++)
{
attributes.Add(new KeyValuePair<string, object>(x.ToString(), oneKb));
}

var provider = BuildMeterProvider(meter, attributes, out var address);

for (var x = 0; x < 1000; x++)
{
var counter = meter.CreateCounter<double>("counter_double_" + x, unit: "By");
counter.Add(1);
}

using HttpClient client = new HttpClient();

if (!string.IsNullOrEmpty(acceptHeader))
{
client.DefaultRequestHeaders.Add("Accept", acceptHeader);
}

using var response = await client.GetAsync($"{address}metrics");

Assert.Equal(HttpStatusCode.OK, response.StatusCode);
var content = await response.Content.ReadAsStringAsync();
Assert.Contains("counter_double_999", content);
Assert.DoesNotContain('\0', content);

provider.Dispose();
}

private static void TestPrometheusHttpListenerUriPrefixOptions(string[] uriPrefixes)
{
using var exporter = new PrometheusExporter(new());
Expand All @@ -159,42 +241,9 @@ private async Task RunPrometheusExporterHttpServerIntegrationTest(bool skipMetri
{
var requestOpenMetrics = acceptHeader.StartsWith("application/openmetrics-text");

Random random = new Random();
int retryAttempts = 5;
int port = 0;
string address = null;

MeterProvider provider = null;
using var meter = new Meter(MeterName, MeterVersion);

while (retryAttempts-- != 0)
{
port = random.Next(2000, 5000);
address = $"http://localhost:{port}/";

try
{
provider = Sdk.CreateMeterProviderBuilder()
.AddMeter(meter.Name)
.ConfigureResource(x => x.Clear().AddService("my_service", serviceInstanceId: "id1"))
.AddPrometheusHttpListener(options =>
{
options.UriPrefixes = new string[] { address };
})
.Build();

break;
}
catch
{
// ignored
}
}

if (provider == null)
{
throw new InvalidOperationException("HttpListener could not be started");
}
var provider = BuildMeterProvider(meter, [], out var address);

var tags = new KeyValuePair<string, object>[]
{
Expand Down

0 comments on commit ef1ff92

Please sign in to comment.