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

Output EOF following the OpenMetrics exposition recommendation #3654

Merged
merged 5 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ instead of 200, when no metrics are collected
([#3648](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3648))
* Added support for OpenMetrics UNIT metadata
([#3651](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3651))
* Added `"# EOF\n"` ending following the [OpenMetrics
specification](https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md)
([#3654](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3654))

## 1.4.0-alpha.2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
([#3648](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3648))
* Added support for OpenMetrics UNIT metadata
([#3651](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3651))
* Added `"# EOF\n"` ending following the [OpenMetrics
specification](https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md)
([#3654](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3654))

## 1.4.0-alpha.2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,27 +191,37 @@ private ExportResult OnCollect(Batch<Metric> metrics)
}
catch (IndexOutOfRangeException)
{
var bufferSize = this.buffer.Length * 2;

// 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
// to put some upper limit and allow the user to configure it.
// 2. we got an IndexOutOfRangeException which was triggered by some other
// code instead of the buffer[cursor++] - in this case we should give up
// at certain point rather than allocating like crazy.
if (bufferSize > 100 * 1024 * 1024)
if (!this.IncreaseBufferSize())
{
// 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
// to put some upper limit and allow the user to configure it.
// 2. we got an IndexOutOfRangeException which was triggered by some other
// code instead of the buffer[cursor++] - in this case we should give up
// at certain point rather than allocating like crazy.
throw;
}
}
}
}

var newBuffer = new byte[bufferSize];
this.buffer.CopyTo(newBuffer, 0);
this.buffer = newBuffer;
while (true)
{
try
{
cursor = PrometheusSerializer.WriteEof(this.buffer, cursor);
break;
}
catch (IndexOutOfRangeException)
{
if (!this.IncreaseBufferSize())
{
throw;
}
}
}

this.previousDataView = new ArraySegment<byte>(this.buffer, 0, Math.Max(cursor - 1, 0));
this.previousDataView = new ArraySegment<byte>(this.buffer, 0, cursor);
utpilla marked this conversation as resolved.
Show resolved Hide resolved
return ExportResult.Success;
}
catch (Exception)
Expand All @@ -221,6 +231,22 @@ private ExportResult OnCollect(Batch<Metric> metrics)
}
}

private bool IncreaseBufferSize()
{
var newBufferSize = this.buffer.Length * 2;

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

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

return true;
}

public readonly struct CollectionResponse
{
public CollectionResponse(ArraySegment<byte> view, DateTime generatedAtUtc, bool fromCache)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,15 @@ public static int WriteMetricName(byte[] buffer, int cursor, string metricName,
return cursor;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int WriteEof(byte[] buffer, int cursor)
{
cursor = WriteAsciiStringNoEscape(buffer, cursor, "# EOF");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about prometheus, but I assume this won't cause an older prometheus scrapper to blow up?

Also, is simply adding # EOF enough to warrant a change to the content-type to application/openmetrics-text?

response.ContentType = "text/plain; charset=utf-8; version=0.0.4";

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about prometheus, but I assume this won't cause an older prometheus scrapper to blow up?

I've tested this with older version of scrappers.

image

Also, is simply adding # EOF enough to warrant a change to the content-type to application/openmetrics-text?

response.ContentType = "text/plain; charset=utf-8; version=0.0.4";

I guess the answer is no, I keep learning new things while reading the OpenMetrics spec, e.g. #3651. I think we need to do a full scan of the spec, maybe it's okay to flip the content-type earlier?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it's okay to flip the content-type earlier?

Probably fine to wait since content-type might be a thing that would affect older scrapers. But may become necessary with exemplar support?

buffer[cursor++] = ASCII_LINEFEED;

return cursor;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int WriteHelpMetadata(byte[] buffer, int cursor, string metricName, string metricUnit, string metricDescription)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
Expand Down Expand Up @@ -261,21 +262,18 @@ private static async Task RunPrometheusExporterMiddlewareIntegrationTest(

string content = await response.Content.ReadAsStringAsync().ConfigureAwait(false);

string[] lines = content.Split('\n');
var matches = Regex.Matches(
content,
("^"
+ "# TYPE counter_double counter\n"
+ "counter_double{key1='value1',key2='value2'} 101.17 (\\d+)\n"
+ "\n"
+ "# EOF\n"
+ "$").Replace('\'', '"'));

Assert.Equal(
$"# TYPE counter_double counter",
lines[0]);
Assert.Single(matches);

Assert.Contains(
$"counter_double{{key1=\"value1\",key2=\"value2\"}} 101.17",
lines[1]);

var index = content.LastIndexOf(' ');

Assert.Equal('\n', content[^1]);

var timestamp = long.Parse(content.Substring(index, content.Length - index - 1));
var timestamp = long.Parse(matches[0].Groups[1].Value);

Assert.True(beginTimestamp <= timestamp && timestamp <= endTimestamp);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ private async Task RunPrometheusExporterHttpServerIntegrationTest(bool skipMetri
Assert.Equal("text/plain; charset=utf-8; version=0.0.4", response.Content.Headers.ContentType.ToString());

Assert.Matches(
"^# TYPE counter_double counter\ncounter_double{key1='value1',key2='value2'} 101.17 \\d+\n$".Replace('\'', '"'),
"^# TYPE counter_double counter\ncounter_double{key1='value1',key2='value2'} 101.17 \\d+\n\n# EOF\n$".Replace('\'', '"'),
await response.Content.ReadAsStringAsync().ConfigureAwait(false));
}
else
Expand Down