From 0d0bfb3e3ebf8f0648a603fd37fcd7c026ba2148 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 27 Oct 2022 17:27:26 -0700 Subject: [PATCH 01/10] Support long event source names better in EventCounters instrumentation --- .../EventCountersMetrics.cs | 63 +++++++++++++++- .../EventCountersMetricsTests.cs | 74 +++++++++++++++++-- 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index c236463144..efa8e0d4ed 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -19,6 +19,7 @@ using System.Collections.Generic; using System.Diagnostics.Metrics; using System.Diagnostics.Tracing; +using System.Text; namespace OpenTelemetry.Instrumentation.EventCounters; @@ -29,11 +30,13 @@ 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 preInitEventSources = new(); private readonly ConcurrentDictionary<(string, string), Instrument> instruments = new(); private readonly ConcurrentDictionary<(string, string), double> values = new(); - /// /// Initializes a new instance of the class. /// @@ -120,6 +123,62 @@ protected override void OnEventWritten(EventWrittenEventArgs eventData) private static Dictionary GetEnableEventsArguments(EventCountersInstrumentationOptions options) => new() { { "EventCounterIntervalSec", options.RefreshIntervalSecs.ToString() } }; + /// + /// If instrument name is too long, abbreviates event source name. + /// E.g. instrument for `Microsoft-AspNetCore-Server-Kestrel`, `tls-handshakes-per-second` + /// would be too long (64 chars), so it's abbreviated to `ec.m.a.s.k.tls-handshakes-per-second` + /// instead of `ec.Microsoft-AspNetCore-Server-Kestrel.tls-handshakes-per-second`. + /// + /// If after that instrument name is still invalid, it will be validated and ignored later in the pipeline. + /// + 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); + } + + int maxEventSourceNameLength = MaxInstrumentNameLength - Prefix.Length - 2 - eventName.Length; + var abbreviation = new StringBuilder(maxEventSourceNameLength); + int ind = 0; + while (ind >= 0 && ind < sourceName.Length) + { + while (ind < sourceName.Length && (sourceName[ind] == '.' || sourceName[ind] == '-')) + { + ind++; + } + + if (ind < sourceName.Length) + { + abbreviation.Append(char.ToLowerInvariant(sourceName[ind])).Append('.'); + } + + int nextDot = sourceName.IndexOf('.', ind); + int nextDash = sourceName.IndexOf('-', ind); + + if (nextDot < 0) + { + if (nextDash < 0) + { + break; + } + + ind = nextDash; + } + else if (nextDash < 0) + { + ind = nextDot; + } + else + { + ind = Math.Min(nextDot, nextDash); + } + } + + return string.Concat(Prefix, ".", abbreviation.ToString(), eventName); + } + private void EnableEvents(EventSource eventSource) { this.EnableEvents(eventSource, EventLevel.Critical, EventKeywords.None, GetEnableEventsArguments(this.options)); @@ -132,7 +191,7 @@ private void UpdateInstrumentWithEvent(bool isGauge, string eventSourceName, str ValueTuple metricKey = new(eventSourceName, name); _ = this.values.AddOrUpdate(metricKey, value, isGauge ? (_, _) => value : (_, existing) => existing + value); - var instrumentName = $"EventCounters.{eventSourceName}.{name}"; + var instrumentName = GetInstrumentName(eventSourceName, name); if (!this.instruments.ContainsKey(metricKey)) { diff --git a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs index 26beba4deb..1c3069c410 100644 --- a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs +++ b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs @@ -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; @@ -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)); @@ -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)); @@ -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)); @@ -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)); @@ -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); @@ -214,6 +215,69 @@ 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.m.a.s.k.1.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.m.a.h.c.1.connections-timed-out-longer")] + public async Task EventSourceNameAbbreviation(string sourceName, string eventName, string expectedInstrumentName) + { + // Arrange + List 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 metricItems = new(); + EventSource source = new("source"); + + // ec.s. + event name is 63; + string veryLongEventName = new string('e', 59); + 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) From e302ad9e4e0946d503dec8282bfea7c7d5646bff Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 27 Oct 2022 17:41:12 -0700 Subject: [PATCH 02/10] lint --- .../EventCountersMetrics.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index efa8e0d4ed..deec1840e7 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -37,6 +37,7 @@ internal sealed class EventCountersMetrics : EventListener private readonly List preInitEventSources = new(); private readonly ConcurrentDictionary<(string, string), Instrument> instruments = new(); private readonly ConcurrentDictionary<(string, string), double> values = new(); + /// /// Initializes a new instance of the class. /// From 941acad59d6cfa66dbbd85aac28d4bc93274fdd7 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Wed, 9 Nov 2022 09:36:28 -0800 Subject: [PATCH 03/10] review --- .../CHANGELOG.md | 3 +++ .../EventCountersMetrics.cs | 6 ++---- .../EventCountersMetricsTests.cs | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md index f3f36b2d94..aecb87cb95 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md @@ -3,6 +3,9 @@ ## Unreleased * Update OpenTelemetry.Api to 1.3.1. +* Changed `EventCounter` prefix to `ec` and started abbreviating event source name + when instrument name is longer that 63 characters. + ([#740](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/740)) ## 1.0.0-alpha.1 diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index deec1840e7..9096e20ed1 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -140,8 +140,7 @@ private static string GetInstrumentName(string sourceName, string eventName) return string.Concat(Prefix, ".", sourceName, ".", eventName); } - int maxEventSourceNameLength = MaxInstrumentNameLength - Prefix.Length - 2 - eventName.Length; - var abbreviation = new StringBuilder(maxEventSourceNameLength); + var abbreviation = new StringBuilder(sourceName.Length); int ind = 0; while (ind >= 0 && ind < sourceName.Length) { @@ -192,10 +191,9 @@ private void UpdateInstrumentWithEvent(bool isGauge, string eventSourceName, str ValueTuple metricKey = new(eventSourceName, name); _ = this.values.AddOrUpdate(metricKey, value, isGauge ? (_, _) => value : (_, existing) => existing + value); - var instrumentName = GetInstrumentName(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]); diff --git a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs index 1c3069c410..4f7736469c 100644 --- a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs +++ b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs @@ -254,7 +254,7 @@ public async Task InstrumentNameTooLong() EventSource source = new("source"); // ec.s. + event name is 63; - string veryLongEventName = new string('e', 59); + string veryLongEventName = new string('e', 100); IncrementingEventCounter connections = new(veryLongEventName, source); var meterProvider = Sdk.CreateMeterProviderBuilder() From 807463b7df89174497808849516e32a7defb2648 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Wed, 9 Nov 2022 18:07:38 -0800 Subject: [PATCH 04/10] shorten source name if abbreviation is not enough --- .../EventCountersMetrics.cs | 15 ++++++++++++++- .../EventCountersMetricsTests.cs | 3 +++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index 9096e20ed1..18ed7441da 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -140,7 +140,15 @@ private static string GetInstrumentName(string sourceName, string eventName) return string.Concat(Prefix, ".", sourceName, ".", eventName); } - var abbreviation = new StringBuilder(sourceName.Length); + var maxEventSourceLength = MaxInstrumentNameLength - Prefix.Length - 1 - eventName.Length; + if (maxEventSourceLength < 2) + { + // event name is too long, there is not enough space for sourceName. + // let ec. flow to metrics SDK and filter if needed. + return string.Concat(Prefix, ".", eventName); + } + + var abbreviation = new StringBuilder(maxEventSourceLength); int ind = 0; while (ind >= 0 && ind < sourceName.Length) { @@ -151,6 +159,11 @@ private static string GetInstrumentName(string sourceName, string eventName) if (ind < sourceName.Length) { + if (abbreviation.Length + 2 >= maxEventSourceLength) + { + break; + } + abbreviation.Append(char.ToLowerInvariant(sourceName[ind])).Append('.'); } diff --git a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs index 4f7736469c..737c9d35ea 100644 --- a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs +++ b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs @@ -220,6 +220,9 @@ public void ThrowExceptionForUnsupportedEventSources() [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.m.a.h.c.1.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.m.a.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.m.a.o.t.very-very-very-very-very-very-very-long-event-name")] public async Task EventSourceNameAbbreviation(string sourceName, string eventName, string expectedInstrumentName) { // Arrange From 218e9560ab894025f66d2bdd4ac6eb9a59b3a97c Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Wed, 9 Nov 2022 18:16:01 -0800 Subject: [PATCH 05/10] lint --- .../CHANGELOG.md | 4 ++-- .../EventCountersMetrics.cs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md index aecb87cb95..0bc56cdbaf 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md @@ -3,8 +3,8 @@ ## Unreleased * Update OpenTelemetry.Api to 1.3.1. -* Changed `EventCounter` prefix to `ec` and started abbreviating event source name - when instrument name is longer that 63 characters. +* Change `EventCounter` prefix to `ec` and abbreviate event source name + when instrument name is longer than 63 characters. ([#740](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/740)) ## 1.0.0-alpha.1 diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index 18ed7441da..7fa32ee95d 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -141,10 +141,10 @@ private static string GetInstrumentName(string sourceName, string eventName) } var maxEventSourceLength = MaxInstrumentNameLength - Prefix.Length - 1 - eventName.Length; - if (maxEventSourceLength < 2) + if (maxEventSourceLength < 2) { // event name is too long, there is not enough space for sourceName. - // let ec. flow to metrics SDK and filter if needed. + // let ec. flow to metrics SDK and it will suppress it if needed. return string.Concat(Prefix, ".", eventName); } @@ -159,12 +159,12 @@ private static string GetInstrumentName(string sourceName, string eventName) if (ind < sourceName.Length) { + abbreviation.Append(char.ToLowerInvariant(sourceName[ind])).Append('.'); if (abbreviation.Length + 2 >= maxEventSourceLength) { + // stop if there is no room to add another letter and a dot after break; } - - abbreviation.Append(char.ToLowerInvariant(sourceName[ind])).Append('.'); } int nextDot = sourceName.IndexOf('.', ind); From 3519fe293369980b467d1c77e0290b2d32f7b607 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 10 Nov 2022 16:01:57 -0800 Subject: [PATCH 06/10] shortening instead of abbreviation --- .../EventCountersMetrics.cs | 44 +------------------ .../EventCountersMetricsTests.cs | 10 ++--- 2 files changed, 6 insertions(+), 48 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index 7fa32ee95d..71a6b1cac1 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -19,7 +19,6 @@ using System.Collections.Generic; using System.Diagnostics.Metrics; using System.Diagnostics.Tracing; -using System.Text; namespace OpenTelemetry.Instrumentation.EventCounters; @@ -148,48 +147,7 @@ private static string GetInstrumentName(string sourceName, string eventName) return string.Concat(Prefix, ".", eventName); } - var abbreviation = new StringBuilder(maxEventSourceLength); - int ind = 0; - while (ind >= 0 && ind < sourceName.Length) - { - while (ind < sourceName.Length && (sourceName[ind] == '.' || sourceName[ind] == '-')) - { - ind++; - } - - if (ind < sourceName.Length) - { - abbreviation.Append(char.ToLowerInvariant(sourceName[ind])).Append('.'); - if (abbreviation.Length + 2 >= maxEventSourceLength) - { - // stop if there is no room to add another letter and a dot after - break; - } - } - - int nextDot = sourceName.IndexOf('.', ind); - int nextDash = sourceName.IndexOf('-', ind); - - if (nextDot < 0) - { - if (nextDash < 0) - { - break; - } - - ind = nextDash; - } - else if (nextDash < 0) - { - ind = nextDot; - } - else - { - ind = Math.Min(nextDot, nextDash); - } - } - - return string.Concat(Prefix, ".", abbreviation.ToString(), eventName); + return string.Concat(Prefix, ".", sourceName.Substring(0, maxEventSourceLength - 1), ".", eventName); } private void EnableEvents(EventSource eventSource) diff --git a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs index 737c9d35ea..2529fd3d1e 100644 --- a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs +++ b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs @@ -216,14 +216,14 @@ public void ThrowExceptionForUnsupportedEventSources() } [Theory] - [InlineData("Microsoft-AspNetCore-Server-Kestrel-1", "tls-handshakes-per-second", "ec.m.a.s.k.1.tls-handshakes-per-second")] + [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.m.a.h.c.1.connections-timed-out-longer")] + [InlineData("Microsoft.AspNetCore.Http.Connections-1", "connections-timed-out-longer", "ec.Microsoft.AspNetCore.Http.Conne.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.m.a.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.m.a.o.t.very-very-very-very-very-very-very-long-event-name")] - public async Task EventSourceNameAbbreviation(string sourceName, string eventName, string expectedInstrumentName) + [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 metricItems = new(); From 7c8248e4b2d6cd9e06f66bf5bd5c328a1aadd1ff Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 10 Nov 2022 16:06:49 -0800 Subject: [PATCH 07/10] nits --- .../EventCountersMetrics.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index 71a6b1cac1..c71be7c85f 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -126,10 +126,10 @@ private static Dictionary GetEnableEventsArguments(EventCounters /// /// If instrument name is too long, abbreviates event source name. /// E.g. instrument for `Microsoft-AspNetCore-Server-Kestrel`, `tls-handshakes-per-second` - /// would be too long (64 chars), so it's abbreviated to `ec.m.a.s.k.tls-handshakes-per-second` - /// instead of `ec.Microsoft-AspNetCore-Server-Kestrel.tls-handshakes-per-second`. - /// - /// If after that instrument name is still invalid, it will be validated and ignored later in the pipeline. + /// 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. /// private static string GetInstrumentName(string sourceName, string eventName) { From 28bdab2c0379b7cc27c293decaddfa13f6ea3f54 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 10 Nov 2022 16:45:03 -0800 Subject: [PATCH 08/10] review comments --- .../CHANGELOG.md | 3 +-- .../EventCountersMetrics.cs | 18 ++++++++++++------ .../EventCountersMetricsTests.cs | 1 + 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md index 0bc56cdbaf..31fee80f79 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md @@ -3,8 +3,7 @@ ## Unreleased * Update OpenTelemetry.Api to 1.3.1. -* Change `EventCounter` prefix to `ec` and abbreviate event source name - when instrument name is longer than 63 characters. +* 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 diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index c71be7c85f..aab398e64d 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -124,11 +124,12 @@ private static Dictionary GetEnableEventsArguments(EventCounters new() { { "EventCounterIntervalSec", options.RefreshIntervalSecs.ToString() } }; /// - /// If instrument name is too long, abbreviates event source name. + /// 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 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. /// private static string GetInstrumentName(string sourceName, string eventName) @@ -139,15 +140,20 @@ private static string GetInstrumentName(string sourceName, string eventName) return string.Concat(Prefix, ".", sourceName, ".", eventName); } - var maxEventSourceLength = MaxInstrumentNameLength - Prefix.Length - 1 - eventName.Length; - if (maxEventSourceLength < 2) + 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. flow to metrics SDK and it will suppress it if needed. return string.Concat(Prefix, ".", eventName); } - return string.Concat(Prefix, ".", sourceName.Substring(0, maxEventSourceLength - 1), ".", 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) diff --git a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs index 2529fd3d1e..dba23b470f 100644 --- a/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs +++ b/test/OpenTelemetry.Instrumentation.EventCounters.Tests/EventCountersMetricsTests.cs @@ -220,6 +220,7 @@ public void ThrowExceptionForUnsupportedEventSources() [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")] From 767cc3777794e1358210b65e9f34276ac5f8f673 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 10 Nov 2022 17:04:16 -0800 Subject: [PATCH 09/10] lint --- .../EventCountersMetrics.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs index aab398e64d..fe8f89dacc 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.EventCounters/EventCountersMetrics.cs @@ -148,9 +148,9 @@ private static string GetInstrumentName(string sourceName, string eventName) return string.Concat(Prefix, ".", eventName); } - while (maxEventSourceLength > 0 && (sourceName[maxEventSourceLength - 1] == '.' || sourceName[maxEventSourceLength - 1] == '-')) + while (maxEventSourceLength > 0 && (sourceName[maxEventSourceLength - 1] == '.' || sourceName[maxEventSourceLength - 1] == '-')) { - maxEventSourceLength --; + maxEventSourceLength--; } return string.Concat(Prefix, ".", sourceName.Substring(0, maxEventSourceLength), ".", eventName); From 5803f8a1195deae98c8b2dd76d2b4539784bb18d Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 10 Nov 2022 17:15:03 -0800 Subject: [PATCH 10/10] lint --- src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md index 31fee80f79..f58d0aeb96 100644 --- a/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EventCounters/CHANGELOG.md @@ -3,7 +3,8 @@ ## 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. +* 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