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

[Instrumentation.EventCounters] Support long event source names better in EventCounters instrumentation #740

Merged
merged 10 commits into from
Nov 11, 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 @@ -3,6 +3,9 @@
## Unreleased

* Update OpenTelemetry.Api to 1.3.1.
* Change `EventCounter` prefix to `ec` and trim the event source name to keep
instrument name under 63 characters.
([#740](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/740))

## 1.0.0-alpha.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ internal sealed class EventCountersMetrics : EventListener
{
internal static readonly Meter MeterInstance = new(typeof(EventCountersMetrics).Assembly.GetName().Name, typeof(EventCountersMetrics).Assembly.GetName().Version.ToString());

private const string Prefix = "ec";
private const int MaxInstrumentNameLength = 63;

private readonly EventCountersInstrumentationOptions options;
private readonly List<EventSource> preInitEventSources = new();
private readonly ConcurrentDictionary<(string, string), Instrument> instruments = new();
Expand Down Expand Up @@ -120,6 +123,39 @@ protected override void OnEventWritten(EventWrittenEventArgs eventData)
private static Dictionary<string, string> GetEnableEventsArguments(EventCountersInstrumentationOptions options) =>
new() { { "EventCounterIntervalSec", options.RefreshIntervalSecs.ToString() } };

/// <summary>
/// If the resulting instrument name is too long, it trims the event source name
/// to fit in as many characters as possible keeping the event name intact.
/// E.g. instrument for `Microsoft-AspNetCore-Server-Kestrel`, `tls-handshakes-per-second`
/// would be too long (64 chars), so it's shortened to `ec.Microsoft-AspNetCore-Server-Kestre.tls-handshakes-per-second`.
///
/// If there is no room for event source name, returns `ec.{event name}` and
/// if it's still too long, it will be validated and ignored later in the pipeline.
/// </summary>
private static string GetInstrumentName(string sourceName, string eventName)
{
int totalLength = Prefix.Length + 1 + sourceName.Length + 1 + eventName.Length;
if (totalLength <= MaxInstrumentNameLength)
{
return string.Concat(Prefix, ".", sourceName, ".", eventName);
}

var maxEventSourceLength = MaxInstrumentNameLength - Prefix.Length - 2 - eventName.Length;
if (maxEventSourceLength < 1)
{
// event name is too long, there is not enough space for sourceName.
// let ec.<eventName> flow to metrics SDK and it will suppress it if needed.
return string.Concat(Prefix, ".", eventName);
}

while (maxEventSourceLength > 0 && (sourceName[maxEventSourceLength - 1] == '.' || sourceName[maxEventSourceLength - 1] == '-'))
{
maxEventSourceLength--;
}

return string.Concat(Prefix, ".", sourceName.Substring(0, maxEventSourceLength), ".", eventName);
}

private void EnableEvents(EventSource eventSource)
{
this.EnableEvents(eventSource, EventLevel.Critical, EventKeywords.None, GetEnableEventsArguments(this.options));
Expand All @@ -132,10 +168,9 @@ private void UpdateInstrumentWithEvent(bool isGauge, string eventSourceName, str
ValueTuple<string, string> metricKey = new(eventSourceName, name);
_ = this.values.AddOrUpdate(metricKey, value, isGauge ? (_, _) => value : (_, existing) => existing + value);

var instrumentName = $"EventCounters.{eventSourceName}.{name}";

if (!this.instruments.ContainsKey(metricKey))
{
var instrumentName = GetInstrumentName(eventSourceName, name);
Instrument instrument = isGauge
? MeterInstance.CreateObservableGauge(instrumentName, () => this.values[metricKey])
: MeterInstance.CreateObservableCounter(instrumentName, () => this.values[metricKey]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.Tracing;
using System.Linq;
using System.Threading.Tasks;
using OpenTelemetry.Metrics;
using Xunit;
Expand Down Expand Up @@ -68,7 +69,7 @@ public async Task EventCounter()
meterProvider.ForceFlush();

// Assert
var metric = metricItems.Find(x => x.Name == "EventCounters.a.c");
var metric = metricItems.Find(x => x.Name == "ec.a.c");
Assert.NotNull(metric);
Assert.Equal(MetricType.DoubleGauge, metric.MetricType);
Assert.Equal(1997.0202, GetActualValue(metric));
Expand Down Expand Up @@ -98,7 +99,7 @@ public async Task IncrementingEventCounter()
meterProvider.ForceFlush();

// Assert
var metric = metricItems.Find(x => x.Name == "EventCounters.b.inc-c");
var metric = metricItems.Find(x => x.Name == "ec.b.inc-c");
Assert.NotNull(metric);
Assert.Equal(MetricType.DoubleSum, metric.MetricType);
Assert.Equal(3, GetActualValue(metric));
Expand Down Expand Up @@ -126,7 +127,7 @@ public async Task PollingCounter()
meterProvider.ForceFlush();

// Assert
var metric = metricItems.Find(x => x.Name == "EventCounters.c.poll-c");
var metric = metricItems.Find(x => x.Name == "ec.c.poll-c");
Assert.NotNull(metric);
Assert.Equal(MetricType.DoubleGauge, metric.MetricType);
Assert.Equal(20, GetActualValue(metric));
Expand Down Expand Up @@ -154,7 +155,7 @@ public async Task IncrementingPollingCounter()
meterProvider.ForceFlush();

// Assert
var metric = metricItems.Find(x => x.Name == "EventCounters.d.inc-poll-c");
var metric = metricItems.Find(x => x.Name == "ec.d.inc-poll-c");
Assert.NotNull(metric);
Assert.Equal(MetricType.DoubleSum, metric.MetricType);
Assert.Equal(2, GetActualValue(metric));
Expand Down Expand Up @@ -184,7 +185,7 @@ public async Task EventCounterSameNameUsesNewestCreated()
meterProvider.ForceFlush();

// Assert
var metric = metricItems.Find(x => x.Name == "EventCounters.a.c");
var metric = metricItems.Find(x => x.Name == "ec.a.c");
Assert.NotNull(metric);
Assert.Equal(MetricType.DoubleGauge, metric.MetricType);

Expand Down Expand Up @@ -214,6 +215,73 @@ public void ThrowExceptionForUnsupportedEventSources()
Assert.Equal("Use the `OpenTelemetry.Instrumentation.Runtime` or `OpenTelemetry.Instrumentation.Process` instrumentations.", ex.Message);
}

[Theory]
[InlineData("Microsoft-AspNetCore-Server-Kestrel-1", "tls-handshakes-per-second", "ec.Microsoft-AspNetCore-Server-Kestre.tls-handshakes-per-second")]
[InlineData("Microsoft-AspNetCore-Server-Kestrel-1", "tls-handshakes-per-sec", "ec.Microsoft-AspNetCore-Server-Kestrel-1.tls-handshakes-per-sec")]
[InlineData("Microsoft.AspNetCore.Http.Connections-1", "connections-stopped", "ec.Microsoft.AspNetCore.Http.Connections-1.connections-stopped")]
[InlineData("Microsoft.AspNetCore.Http.Connections-1", "connections-timed-out-longer", "ec.Microsoft.AspNetCore.Http.Conne.connections-timed-out-longer")]
[InlineData("Microsoft.AspNetCore.Http.Conn.Something", "connections-timed-out-longer", "ec.Microsoft.AspNetCore.Http.Conn.connections-timed-out-longer")]
[InlineData("Microsoft.AspNetCore.One.Two", "very-very-very-very-very-very-very-very-very-long-event-name", "ec.very-very-very-very-very-very-very-very-very-long-event-name")]
[InlineData("Microsoft.AspNetCore.One.Two", "very-very-very-very-very-very-very-very-long-event-name", "ec.Micr.very-very-very-very-very-very-very-very-long-event-name")]
[InlineData("Microsoft.AspNetCore.One.Two", "very-very-very-very-very-very-very-long-event-name", "ec.Microsoft.very-very-very-very-very-very-very-long-event-name")]
public async Task EventSourceNameShortening(string sourceName, string eventName, string expectedInstrumentName)
{
// Arrange
List<Metric> metricItems = new();
EventSource source = new(sourceName);
IncrementingEventCounter connections = new(eventName, source);

var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddEventCountersInstrumentation(options =>
{
options.AddEventSources(source.Name);
})
.AddInMemoryExporter(metricItems)
.Build();

// Act
connections.Increment(1);
await Task.Delay(Delay);
meterProvider.ForceFlush();

// Assert
Metric metric = metricItems.Find(m => m.Name == expectedInstrumentName);
Assert.NotNull(metric);
Assert.Equal(1, GetActualValue(metric));
}

[Fact]
public async Task InstrumentNameTooLong()
{
// Arrange
List<Metric> metricItems = new();
EventSource source = new("source");

// ec.s. + event name is 63;
string veryLongEventName = new string('e', 100);
IncrementingEventCounter connections = new(veryLongEventName, source);

var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddEventCountersInstrumentation(options =>
{
options.AddEventSources(source.Name);
})
.AddInMemoryExporter(metricItems)
.Build();

// Act
connections.Increment(1);
await Task.Delay(Delay);
meterProvider.ForceFlush();

// Assert
foreach (var item in metricItems)
{
Assert.False(item.Name.StartsWith("ec.source.ee"));
Assert.False(item.Name.StartsWith("ec.s.ee"));
}
}

// polling and eventcounter with same instrument name?

private static double GetActualValue(Metric metric)
Expand Down