From 975accc310f928dc658aca090e7036be29af9b5b Mon Sep 17 00:00:00 2001 From: Timothy Mothra Lee Date: Fri, 11 Aug 2023 16:34:13 -0700 Subject: [PATCH 1/7] update histogram metrics --- .../CHANGELOG.md | 6 ++++++ .../Implementation/HttpInMetricsListener.cs | 8 ++++++-- .../MeterProviderBuilderExtensions.cs | 4 ++++ .../README.md | 4 ++-- .../MetricTests.cs | 13 +++++++++++++ 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index b44b5fb8b1e..ab398e976bb 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* **Breaking change** The metric `http.server.duration` has been updated to the + latest spec for [metrics](https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md). + Unit has been changed from millisecond to second and the histogram bounds + have been updated accordingly. + ([]()) + ## 1.5.1-beta.1 Released 2023-Jul-20 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs index ce06402bc34..f940607e5e9 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs @@ -27,7 +27,11 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation; internal sealed class HttpInMetricsListener : ListenerHandler { - private const string HttpServerDurationMetricName = "http.server.duration"; + internal const string HttpServerDurationMetricName = "http.server.duration"; + + // Http Metrics use custom histogram boundaries. See the spec: https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md + internal static double[] HttpServerDurationMetricExplicitBounds = new double[] { 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10 }; + private const string OnStopEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop"; private const string EventName = "OnStopActivity"; @@ -42,7 +46,7 @@ internal HttpInMetricsListener(string name, Meter meter, AspNetCoreMetricsInstru { this.meter = meter; this.options = options; - this.httpServerDuration = meter.CreateHistogram(HttpServerDurationMetricName, "ms", "Measures the duration of inbound HTTP requests."); + this.httpServerDuration = meter.CreateHistogram(HttpServerDurationMetricName, "s", "Measures the duration of inbound HTTP requests."); this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs index 99330ac220a..5e66aaec7ac 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs @@ -78,6 +78,10 @@ public static MeterProviderBuilder AddAspNetCoreInstrumentation( builder.AddMeter(AspNetCoreMetrics.InstrumentationName); + builder.AddView( + instrumentName: HttpInMetricsListener.HttpServerDurationMetricName, + metricStreamConfiguration: new ExplicitBucketHistogramConfiguration { Boundaries = HttpInMetricsListener.HttpServerDurationMetricExplicitBounds }); + builder.AddInstrumentation(sp => { var options = sp.GetRequiredService>().Get(name); diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md index 131f98a4b37..7c0c33d59eb 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/README.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/README.md @@ -91,12 +91,12 @@ public void ConfigureServices(IServiceCollection services) #### List of metrics produced The instrumentation is implemented based on [metrics semantic -conventions](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#metric-httpserverduration). +conventions](https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md#metric-httpserverduration). Currently, the instrumentation supports the following metric. | Name | Instrument Type | Unit | Description | |-------|-----------------|------|-------------| -| `http.server.duration` | Histogram | `ms` | Measures the duration of inbound HTTP requests. | +| `http.server.duration` | Histogram | `s` | Measures the duration of inbound HTTP requests. | ## Advanced configuration diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 607472c83a4..928abd9be9c 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -15,6 +15,7 @@ // using System.Diagnostics; +using System.Linq; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Testing; @@ -244,6 +245,18 @@ private static KeyValuePair[] AssertMetricPoint( Assert.Contains(route, attributes); Assert.Equal(expectedTagsCount, attributes.Length); + // Inspect Histogram Bounds + var histogramBuckets = metricPoint.GetHistogramBuckets(); + var histogramBounds = new List(); + foreach (var t in histogramBuckets) + { + histogramBounds.Add(t.ExplicitBound); + } + + Assert.Equal( + expected: new List { 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10, double.PositiveInfinity }, + actual: histogramBounds); + return attributes; } } From b8d69ac7b959f6e8f1c303acc1c0e38f37cdcc21 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Lee Date: Fri, 11 Aug 2023 16:36:21 -0700 Subject: [PATCH 2/7] changelog --- src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index ab398e976bb..75370e7fbee 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -6,7 +6,7 @@ latest spec for [metrics](https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md). Unit has been changed from millisecond to second and the histogram bounds have been updated accordingly. - ([]()) + ([#4766](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4766)) ## 1.5.1-beta.1 From 72b429efb95ff0d594694d23ddd2fd502c6e9a13 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Lee Date: Fri, 11 Aug 2023 16:51:42 -0700 Subject: [PATCH 3/7] remove extra using --- .../MetricTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 928abd9be9c..05ae091b4b7 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -15,7 +15,6 @@ // using System.Diagnostics; -using System.Linq; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Testing; From 4f016cfdb736bf7fa1da5ad7aa26aa383e3e16e1 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Lee Date: Mon, 14 Aug 2023 11:26:33 -0700 Subject: [PATCH 4/7] record seconds --- .../Implementation/HttpInMetricsListener.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs index f940607e5e9..d224b0f7ab8 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs @@ -148,7 +148,7 @@ public override void OnEventWritten(string name, object payload) // We are relying here on ASP.NET Core to set duration before writing the stop event. // https://github.com/dotnet/aspnetcore/blob/d6fa351048617ae1c8b47493ba1abbe94c3a24cf/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L449 // TODO: Follow up with .NET team if we can continue to rely on this behavior. - this.httpServerDuration.Record(Activity.Current.Duration.TotalMilliseconds, tags); + this.httpServerDuration.Record(Activity.Current.Duration.TotalSeconds, tags); } } } From 99d54c1b427fa926dd5f9f2813ade44879ccc418 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Lee Date: Tue, 22 Aug 2023 14:09:38 -0700 Subject: [PATCH 5/7] EnvVar will now affect metric. old: http.server.duration. new: http.server.request.duration --- .../Implementation/HttpInMetricsListener.cs | 15 +- .../MeterProviderBuilderExtensions.cs | 16 +- .../MetricTests.cs | 165 ++++++++++++------ 3 files changed, 142 insertions(+), 54 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs index d224b0f7ab8..54387869b3e 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs @@ -28,6 +28,7 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation; internal sealed class HttpInMetricsListener : ListenerHandler { internal const string HttpServerDurationMetricName = "http.server.duration"; + internal const string HttpServerRequestDurationMetricName = "http.server.request.duration"; // Http Metrics use custom histogram boundaries. See the spec: https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md internal static double[] HttpServerDurationMetricExplicitBounds = new double[] { 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10 }; @@ -38,6 +39,7 @@ internal sealed class HttpInMetricsListener : ListenerHandler private readonly Meter meter; private readonly AspNetCoreMetricsInstrumentationOptions options; private readonly Histogram httpServerDuration; + private readonly Histogram httpServerRequestDuration; private readonly bool emitOldAttributes; private readonly bool emitNewAttributes; @@ -46,7 +48,8 @@ internal HttpInMetricsListener(string name, Meter meter, AspNetCoreMetricsInstru { this.meter = meter; this.options = options; - this.httpServerDuration = meter.CreateHistogram(HttpServerDurationMetricName, "s", "Measures the duration of inbound HTTP requests."); + this.httpServerDuration = meter.CreateHistogram(HttpServerDurationMetricName, "ms", "Measures the duration of inbound HTTP requests."); + this.httpServerRequestDuration = meter.CreateHistogram(HttpServerRequestDurationMetricName, "s", "Measures the duration of inbound HTTP requests."); this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); @@ -148,7 +151,15 @@ public override void OnEventWritten(string name, object payload) // We are relying here on ASP.NET Core to set duration before writing the stop event. // https://github.com/dotnet/aspnetcore/blob/d6fa351048617ae1c8b47493ba1abbe94c3a24cf/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L449 // TODO: Follow up with .NET team if we can continue to rely on this behavior. - this.httpServerDuration.Record(Activity.Current.Duration.TotalSeconds, tags); + if (this.emitNewAttributes) + { + this.httpServerRequestDuration.Record(Activity.Current.Duration.TotalSeconds, tags); + } + + if (this.emitOldAttributes) + { + this.httpServerDuration.Record(Activity.Current.Duration.TotalMilliseconds, tags); + } } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs index 5e66aaec7ac..e78aa966534 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs @@ -14,11 +14,13 @@ // limitations under the License. // +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using OpenTelemetry.Instrumentation.AspNetCore; using OpenTelemetry.Instrumentation.AspNetCore.Implementation; using OpenTelemetry.Internal; +using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Metrics; @@ -78,9 +80,17 @@ public static MeterProviderBuilder AddAspNetCoreInstrumentation( builder.AddMeter(AspNetCoreMetrics.InstrumentationName); - builder.AddView( - instrumentName: HttpInMetricsListener.HttpServerDurationMetricName, - metricStreamConfiguration: new ExplicitBucketHistogramConfiguration { Boundaries = HttpInMetricsListener.HttpServerDurationMetricExplicitBounds }); + var configuration = new ConfigurationBuilder().AddEnvironmentVariables().Build(); + var httpSemanticConvention = GetSemanticConventionOptIn(configuration); + if (httpSemanticConvention.HasFlag(HttpSemanticConvention.New)) + { + // THIS IS A HACK. + // TODO: REMOVE VIEW AND USE HINT API WHEN AVAILABLE. + // https://github.com/open-telemetry/opentelemetry-dotnet/pull/4766#discussion_r1291903760 + builder.AddView( + instrumentName: HttpInMetricsListener.HttpServerRequestDurationMetricName, + metricStreamConfiguration: new ExplicitBucketHistogramConfiguration { Boundaries = HttpInMetricsListener.HttpServerDurationMetricExplicitBounds }); + } builder.AddInstrumentation(sp => { diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 05ae091b4b7..647424ddac6 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -30,6 +30,8 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Tests; public class MetricTests : IClassFixture>, IDisposable { + public const string SemanticConventionOptInKeyName = "OTEL_SEMCONV_STABILITY_OPT_IN"; + private const int StandardTagsCount = 6; private readonly WebApplicationFactory factory; @@ -47,47 +49,79 @@ public void AddAspNetCoreInstrumentation_BadArgs() Assert.Throws(() => builder.AddAspNetCoreInstrumentation()); } - [Fact] - public async Task RequestMetricIsCaptured() + [Theory] + [InlineData(null, true, false, 6)] // emits old metric & attributes + [InlineData("http", false, true, 6)] // emits new metric & attributes + [InlineData("http/dup", true, true, 11)] // emits both old & new + public async Task RequestMetricIsCaptured(string environmentVarValue, bool validateOldSemConv, bool validateNewSemConv, int expectedTagsCount) { - var metricItems = new List(); + try + { + Environment.SetEnvironmentVariable(SemanticConventionOptInKeyName, environmentVarValue); - this.meterProvider = Sdk.CreateMeterProviderBuilder() - .AddAspNetCoreInstrumentation() - .AddInMemoryExporter(metricItems) - .Build(); + var metricItems = new List(); - using (var client = this.factory - .WithWebHostBuilder(builder => + this.meterProvider = Sdk.CreateMeterProviderBuilder() + .AddAspNetCoreInstrumentation() + .AddInMemoryExporter(metricItems) + .Build(); + + using (var client = this.factory + .WithWebHostBuilder(builder => + { + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + }) + .CreateClient()) { - builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); - }) - .CreateClient()) - { - using var response1 = await client.GetAsync("/api/values").ConfigureAwait(false); - using var response2 = await client.GetAsync("/api/values/2").ConfigureAwait(false); + using var response1 = await client.GetAsync("/api/values").ConfigureAwait(false); + using var response2 = await client.GetAsync("/api/values/2").ConfigureAwait(false); - response1.EnsureSuccessStatusCode(); - response2.EnsureSuccessStatusCode(); - } + response1.EnsureSuccessStatusCode(); + response2.EnsureSuccessStatusCode(); + } - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false); + // We need to let End callback execute as it is executed AFTER response was returned. + // In unit tests environment there may be a lot of parallel unit tests executed, so + // giving some breezing room for the End callback to complete + await Task.Delay(TimeSpan.FromSeconds(1)).ConfigureAwait(false); - this.meterProvider.Dispose(); + this.meterProvider.Dispose(); - var requestMetrics = metricItems - .Where(item => item.Name == "http.server.duration") - .ToArray(); + if (validateOldSemConv) + { + var requestMetrics = metricItems + .Where(item => item.Name == "http.server.duration") + .ToArray(); - var metric = Assert.Single(requestMetrics); - var metricPoints = GetMetricPoints(metric); - Assert.Equal(2, metricPoints.Count); + var metric = Assert.Single(requestMetrics); + Assert.Equal("ms", metric.Unit); + var metricPoints = GetMetricPoints(metric); + Assert.Equal(2, metricPoints.Count); + + AssertMetricPoint(metricPoints[0], expectedRoute: "api/Values", expectedTagsCount: expectedTagsCount, validateOldSemConv: true); + AssertMetricPoint(metricPoints[1], expectedRoute: "api/Values/{id}", expectedTagsCount: expectedTagsCount, validateOldSemConv: true); + } + + if (validateNewSemConv) + { + var requestMetrics = metricItems + .Where(item => item.Name == "http.server.request.duration") + .ToArray(); + + var metric = Assert.Single(requestMetrics); + Assert.Equal("s", metric.Unit); + var metricPoints = GetMetricPoints(metric); + Assert.Equal(2, metricPoints.Count); + + AssertMetricPoint(metricPoints[0], expectedRoute: "api/Values", expectedTagsCount: expectedTagsCount, validateNewSemConv: true); + AssertMetricPoint(metricPoints[1], expectedRoute: "api/Values/{id}", expectedTagsCount: expectedTagsCount, validateNewSemConv: true); + } - AssertMetricPoint(metricPoints[0], expectedRoute: "api/Values"); - AssertMetricPoint(metricPoints[1], expectedRoute: "api/Values/{id}"); + } + finally + { + Environment.SetEnvironmentVariable(SemanticConventionOptInKeyName, null); + } } [Fact] @@ -133,7 +167,7 @@ void ConfigureTestServices(IServiceCollection services) // Assert single because we filtered out one route var metricPoint = Assert.Single(GetMetricPoints(metric)); - AssertMetricPoint(metricPoint); + AssertMetricPoint(metricPoint, validateOldSemConv: true); } [Fact] @@ -187,7 +221,7 @@ void ConfigureTestServices(IServiceCollection services) var metric = Assert.Single(requestMetrics); var metricPoint = Assert.Single(GetMetricPoints(metric)); - var tags = AssertMetricPoint(metricPoint, expectedTagsCount: StandardTagsCount + 2); + var tags = AssertMetricPoint(metricPoint, expectedTagsCount: StandardTagsCount + 2, validateOldSemConv: true); Assert.Contains(tagsToAdd[0], tags); Assert.Contains(tagsToAdd[1], tags); @@ -215,7 +249,9 @@ private static List GetMetricPoints(Metric metric) private static KeyValuePair[] AssertMetricPoint( MetricPoint metricPoint, string expectedRoute = "api/Values", - int expectedTagsCount = StandardTagsCount) + int expectedTagsCount = StandardTagsCount, + bool validateNewSemConv = false, + bool validateOldSemConv = false) { var count = metricPoint.GetHistogramCount(); var sum = metricPoint.GetHistogramSum(); @@ -230,20 +266,41 @@ private static KeyValuePair[] AssertMetricPoint( attributes[i++] = tag; } - var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, "GET"); - var scheme = new KeyValuePair(SemanticConventions.AttributeHttpScheme, "http"); - var statusCode = new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, 200); - var flavor = new KeyValuePair(SemanticConventions.AttributeHttpFlavor, "1.1"); - var host = new KeyValuePair(SemanticConventions.AttributeNetHostName, "localhost"); - var route = new KeyValuePair(SemanticConventions.AttributeHttpRoute, expectedRoute); - Assert.Contains(method, attributes); - Assert.Contains(scheme, attributes); - Assert.Contains(statusCode, attributes); - Assert.Contains(flavor, attributes); - Assert.Contains(host, attributes); - Assert.Contains(route, attributes); + // Inspect Attributes Assert.Equal(expectedTagsCount, attributes.Length); + if (validateNewSemConv) + { + var method = new KeyValuePair(SemanticConventions.AttributeHttpRequestMethod, "GET"); + var scheme = new KeyValuePair(SemanticConventions.AttributeUrlScheme, "http"); + var statusCode = new KeyValuePair(SemanticConventions.AttributeHttpResponseStatusCode, 200); + var flavor = new KeyValuePair(SemanticConventions.AttributeNetworkProtocolVersion, "1.1"); + var host = new KeyValuePair(SemanticConventions.AttributeServerAddress, "localhost"); + var route = new KeyValuePair(SemanticConventions.AttributeHttpRoute, expectedRoute); + Assert.Contains(method, attributes); + Assert.Contains(scheme, attributes); + Assert.Contains(statusCode, attributes); + Assert.Contains(flavor, attributes); + Assert.Contains(host, attributes); + Assert.Contains(route, attributes); + } + + if (validateOldSemConv) + { + var method = new KeyValuePair(SemanticConventions.AttributeHttpMethod, "GET"); + var scheme = new KeyValuePair(SemanticConventions.AttributeHttpScheme, "http"); + var statusCode = new KeyValuePair(SemanticConventions.AttributeHttpStatusCode, 200); + var flavor = new KeyValuePair(SemanticConventions.AttributeHttpFlavor, "1.1"); + var host = new KeyValuePair(SemanticConventions.AttributeNetHostName, "localhost"); + var route = new KeyValuePair(SemanticConventions.AttributeHttpRoute, expectedRoute); + Assert.Contains(method, attributes); + Assert.Contains(scheme, attributes); + Assert.Contains(statusCode, attributes); + Assert.Contains(flavor, attributes); + Assert.Contains(host, attributes); + Assert.Contains(route, attributes); + } + // Inspect Histogram Bounds var histogramBuckets = metricPoint.GetHistogramBuckets(); var histogramBounds = new List(); @@ -252,9 +309,19 @@ private static KeyValuePair[] AssertMetricPoint( histogramBounds.Add(t.ExplicitBound); } - Assert.Equal( - expected: new List { 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10, double.PositiveInfinity }, - actual: histogramBounds); + if (validateNewSemConv) + { + Assert.Equal( + expected: new List { 0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10, double.PositiveInfinity }, + actual: histogramBounds); + } + + if (validateOldSemConv) + { + Assert.Equal( + expected: new List { 0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000, double.PositiveInfinity }, + actual: histogramBounds); + } return attributes; } From 56676a9a56e033d4c527b18c6c83238d50029f71 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Lee Date: Tue, 22 Aug 2023 14:21:43 -0700 Subject: [PATCH 6/7] fix --- .../MetricTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 647424ddac6..957fcd8b1dc 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -116,7 +116,6 @@ public async Task RequestMetricIsCaptured(string environmentVarValue, bool valid AssertMetricPoint(metricPoints[0], expectedRoute: "api/Values", expectedTagsCount: expectedTagsCount, validateNewSemConv: true); AssertMetricPoint(metricPoints[1], expectedRoute: "api/Values/{id}", expectedTagsCount: expectedTagsCount, validateNewSemConv: true); } - } finally { From 598ca2f347e41db45992330fabd41d771bcc86c0 Mon Sep 17 00:00:00 2001 From: Timothy Mothra Lee Date: Tue, 22 Aug 2023 14:38:17 -0700 Subject: [PATCH 7/7] changelog --- src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 75370e7fbee..667624a692f 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -3,9 +3,10 @@ ## Unreleased * **Breaking change** The metric `http.server.duration` has been updated to the - latest spec for [metrics](https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md). - Unit has been changed from millisecond to second and the histogram bounds - have been updated accordingly. + latest spec for [metrics](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-metrics.md). + Implemented new histogram `http.server.request.duration` to replace + `http.server.duration`. New histogram records seconds and has updated histogram + bounds. ([#4766](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4766)) ## 1.5.1-beta.1