From 715a7dead10d0aaee8dd0770e8e7a0051189b7d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Tue, 23 Jan 2024 06:47:32 +0100 Subject: [PATCH 1/7] [Instrumentation.AspNetCore] .NET6 library works on .NET7 and .NET8 --- ...mentationMeterProviderBuilderExtensions.cs | 5 +++ ...entationTracerProviderBuilderExtensions.cs | 32 +++++++++------- .../CHANGELOG.md | 8 +++- .../Implementation/HttpInListener.cs | 37 ++++++++++++------- src/Shared/ActivityHelperExtensions.cs | 29 --------------- .../Trace/ActivityExtensionsTest.cs | 23 ------------ 6 files changed, 55 insertions(+), 79 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs index 3da8c1de59d..fad1554b5ca 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs @@ -27,6 +27,11 @@ public static MeterProviderBuilder AddAspNetCoreInstrumentation( #if NET8_0_OR_GREATER return builder.ConfigureMeters(); #else + if (Environment.Version.Major >= 8) + { + return builder.ConfigureMeters(); + } + // Note: Warm-up the status code and method mapping. _ = TelemetryHelper.BoxedStatusCodes; _ = RequestMethodHelper.KnownMethods; diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs index d37d77ae3fa..d1d4b78bfb8 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs @@ -1,9 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#if NET7_0_OR_GREATER using System.Diagnostics; -#endif using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using OpenTelemetry.Instrumentation.AspNetCore; @@ -101,22 +99,30 @@ private static void AddAspNetCoreInstrumentationSources( // For .NET7.0 onwards activity will be created using activitySource. // https://github.com/dotnet/aspnetcore/blob/bf3352f2422bf16fa3ca49021f0e31961ce525eb/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L327 // For .NET6.0 and below, we will continue to use legacy way. -#if NET7_0_OR_GREATER - // TODO: Check with .NET team to see if this can be prevented - // as this allows user to override the ActivitySource. - var activitySourceService = serviceProvider?.GetService(); - if (activitySourceService != null) + +#if !NET7_0_OR_GREATER + if (HttpInListener.Net7OrGreater) { - builder.AddSource(activitySourceService.Name); +#endif + // TODO: Check with .NET team to see if this can be prevented + // as this allows user to override the ActivitySource. + var activitySourceService = serviceProvider?.GetService(); + if (activitySourceService != null) + { + builder.AddSource(activitySourceService.Name); + } + else + { + // For users not using hosting package? + builder.AddSource(HttpInListener.AspNetCoreActivitySourceName); + } +#if !NET7_0_OR_GREATER } else { - // For users not using hosting package? - builder.AddSource(HttpInListener.AspNetCoreActivitySourceName); + builder.AddSource(HttpInListener.ActivitySourceName); + builder.AddLegacySource(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore } -#else - builder.AddSource(HttpInListener.ActivitySourceName); - builder.AddLegacySource(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore #endif } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 03374b1cd94..c396ad52035 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -6,7 +6,13 @@ [#4466](https://github.com/open-telemetry/opentelemetry-dotnet/issues/4466) where the activity instance returned by `Activity.Current` was different than instance obtained from `IHttpActivityFeature.Activity`. - [#5136](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5136) + ([#5136](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5136)) + +* Fixed metrics instrumentation when library targeted for .NET6 or .NET 7 + was loaded by .NET8. + Fixed traces instrumentation when library targeted for .NET6 + was loaded by .NET7 or .NET8. + ([#5252](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5252)) ## 1.7.0 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 0d5013402d3..81205db6ae1 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -26,16 +26,18 @@ internal class HttpInListener : ListenerHandler internal const string OnUnhandledHostingExceptionEvent = "Microsoft.AspNetCore.Hosting.UnhandledException"; internal const string OnUnHandledDiagnosticsExceptionEvent = "Microsoft.AspNetCore.Diagnostics.UnhandledException"; -#if NET7_0_OR_GREATER // https://github.com/dotnet/aspnetcore/blob/8d6554e655b64da75b71e0e20d6db54a3ba8d2fb/src/Hosting/Hosting/src/GenericHost/GenericWebHostBuilder.cs#L85 internal static readonly string AspNetCoreActivitySourceName = "Microsoft.AspNetCore"; -#endif internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName(); internal static readonly string ActivitySourceName = AssemblyName.Name; internal static readonly Version Version = AssemblyName.Version; internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString()); +#if !NET7_0_OR_GREATER + internal static readonly bool Net7OrGreater = Environment.Version.Major >= 7; +#endif + private const string DiagnosticSourceName = "Microsoft.AspNetCore"; private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => @@ -120,14 +122,24 @@ public void OnStartActivity(Activity activity, object payload) // Create a new activity with its parent set from the extracted context. // This makes the new activity as a "sibling" of the activity created by // Asp.Net Core. -#if NET7_0_OR_GREATER + Activity newOne; +#if !NET7_0_OR_GREATER + if (Net7OrGreater) + { +#endif + // For NET7.0 onwards activity is created using ActivitySource so, // we will use the source of the activity to create the new one. - Activity newOne = activity.Source.CreateActivity(ActivityOperationName, ActivityKind.Server, ctx.ActivityContext); -#else - Activity newOne = new Activity(ActivityOperationName); - newOne.SetParentId(ctx.ActivityContext.TraceId, ctx.ActivityContext.SpanId, ctx.ActivityContext.TraceFlags); + newOne = activity.Source.CreateActivity(ActivityOperationName, ActivityKind.Server, ctx.ActivityContext); +#if !NET7_0_OR_GREATER + } + else + { + newOne = new Activity(ActivityOperationName); + newOne.SetParentId(ctx.ActivityContext.TraceId, ctx.ActivityContext.SpanId, ctx.ActivityContext.TraceFlags); + } #endif + newOne.TraceStateString = ctx.ActivityContext.TraceState; newOne.SetTag("IsCreatedByInstrumentation", bool.TrueString); @@ -166,8 +178,11 @@ public void OnStartActivity(Activity activity, object payload) } #if !NET7_0_OR_GREATER - ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource); - ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server); + if (!Net7OrGreater) + { + ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource); + ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server); + } #endif var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/"; @@ -261,12 +276,8 @@ public void OnStopActivity(Activity activity, object payload) } } -#if NET7_0_OR_GREATER var tagValue = activity.GetTagValue("IsCreatedByInstrumentation"); if (ReferenceEquals(tagValue, bool.TrueString)) -#else - if (activity.TryCheckFirstTag("IsCreatedByInstrumentation", out var tagValue) && ReferenceEquals(tagValue, bool.TrueString)) -#endif { // If instrumentation started a new Activity, it must // be stopped here. diff --git a/src/Shared/ActivityHelperExtensions.cs b/src/Shared/ActivityHelperExtensions.cs index b237ccd5457..a7510caa8fd 100644 --- a/src/Shared/ActivityHelperExtensions.cs +++ b/src/Shared/ActivityHelperExtensions.cs @@ -83,33 +83,4 @@ public static bool TryGetStatus(this Activity activity, out StatusCode statusCod return null; } - - /// - /// Checks if the user provided tag name is the first tag of the and retrieves the tag value. - /// - /// Activity instance. - /// Tag name. - /// Tag value. - /// if the first tag of the supplied Activity matches the user provide tag name. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryCheckFirstTag(this Activity activity, string tagName, out object? tagValue) - { - Debug.Assert(activity != null, "Activity should not be null"); - - var enumerator = activity!.EnumerateTagObjects(); - - if (enumerator.MoveNext()) - { - ref readonly var tag = ref enumerator.Current; - - if (tag.Key == tagName) - { - tagValue = tag.Value; - return true; - } - } - - tagValue = null; - return false; - } } diff --git a/test/OpenTelemetry.Api.Tests/Trace/ActivityExtensionsTest.cs b/test/OpenTelemetry.Api.Tests/Trace/ActivityExtensionsTest.cs index 31fbf806725..a48cebcffa2 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/ActivityExtensionsTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/ActivityExtensionsTest.cs @@ -198,27 +198,4 @@ public void GetTagValue() Assert.Null(activity.GetTagValue("tag1")); Assert.Null(activity.GetTagValue("Tag2")); } - - [Theory] - [InlineData("Key", "Value", true)] - [InlineData("CustomTag", null, false)] - public void TryCheckFirstTag(string tagName, object expectedTagValue, bool expectedResult) - { - using var activity = new Activity("Test"); - activity.SetTag("Key", "Value"); - - var result = activity.TryCheckFirstTag(tagName, out var tagValue); - Assert.Equal(expectedResult, result); - Assert.Equal(expectedTagValue, tagValue); - } - - [Fact] - public void TryCheckFirstTagReturnsFalseForActivityWithNoTags() - { - using var activity = new Activity("Test"); - - var result = activity.TryCheckFirstTag("Key", out var tagValue); - Assert.False(result); - Assert.Null(tagValue); - } } From b7e7fbbd63a9d3d1a17db96c697e3ece753e0ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 24 Jan 2024 17:03:15 +0100 Subject: [PATCH 2/7] dotnet format --- ...entationTracerProviderBuilderExtensions.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs index d1d4b78bfb8..5e9cdc23f83 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs @@ -104,18 +104,18 @@ private static void AddAspNetCoreInstrumentationSources( if (HttpInListener.Net7OrGreater) { #endif - // TODO: Check with .NET team to see if this can be prevented - // as this allows user to override the ActivitySource. - var activitySourceService = serviceProvider?.GetService(); - if (activitySourceService != null) - { - builder.AddSource(activitySourceService.Name); - } - else - { - // For users not using hosting package? - builder.AddSource(HttpInListener.AspNetCoreActivitySourceName); - } + // TODO: Check with .NET team to see if this can be prevented + // as this allows user to override the ActivitySource. + var activitySourceService = serviceProvider?.GetService(); + if (activitySourceService != null) + { + builder.AddSource(activitySourceService.Name); + } + else + { + // For users not using hosting package? + builder.AddSource(HttpInListener.AspNetCoreActivitySourceName); + } #if !NET7_0_OR_GREATER } else From bf886e0fde3a68e44b1955e08f7307890a0a8352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 24 Jan 2024 20:06:02 +0100 Subject: [PATCH 3/7] fix changelog Co-authored-by: Reiley Yang --- src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index c396ad52035..2151b7e9749 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -8,10 +8,10 @@ instance obtained from `IHttpActivityFeature.Activity`. ([#5136](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5136)) -* Fixed metrics instrumentation when library targeted for .NET6 or .NET 7 - was loaded by .NET8. - Fixed traces instrumentation when library targeted for .NET6 - was loaded by .NET7 or .NET8. +* Fixed metrics instrumentation when library targeting .NET 6 or .NET 7 + was loaded by .NET 8. + Fixed traces instrumentation when library targeting .NET 6 + was loaded by .NET 7 or .NET 8. ([#5252](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5252)) ## 1.7.0 From cbba3d6f38e2119518518f7460cdce7a9dcc6f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 31 Jan 2024 19:28:43 +0100 Subject: [PATCH 4/7] Keep workaround only in HttpInListener --- ...umentationMeterProviderBuilderExtensions.cs | 5 ----- ...mentationTracerProviderBuilderExtensions.cs | 18 ++++++------------ .../CHANGELOG.md | 6 ------ .../Implementation/HttpInListener.cs | 6 +++--- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs index fad1554b5ca..3da8c1de59d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationMeterProviderBuilderExtensions.cs @@ -27,11 +27,6 @@ public static MeterProviderBuilder AddAspNetCoreInstrumentation( #if NET8_0_OR_GREATER return builder.ConfigureMeters(); #else - if (Environment.Version.Major >= 8) - { - return builder.ConfigureMeters(); - } - // Note: Warm-up the status code and method mapping. _ = TelemetryHelper.BoxedStatusCodes; _ = RequestMethodHelper.KnownMethods; diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs index 5e9cdc23f83..d37d77ae3fa 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs @@ -1,7 +1,9 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#if NET7_0_OR_GREATER using System.Diagnostics; +#endif using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using OpenTelemetry.Instrumentation.AspNetCore; @@ -99,11 +101,7 @@ private static void AddAspNetCoreInstrumentationSources( // For .NET7.0 onwards activity will be created using activitySource. // https://github.com/dotnet/aspnetcore/blob/bf3352f2422bf16fa3ca49021f0e31961ce525eb/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L327 // For .NET6.0 and below, we will continue to use legacy way. - -#if !NET7_0_OR_GREATER - if (HttpInListener.Net7OrGreater) - { -#endif +#if NET7_0_OR_GREATER // TODO: Check with .NET team to see if this can be prevented // as this allows user to override the ActivitySource. var activitySourceService = serviceProvider?.GetService(); @@ -116,13 +114,9 @@ private static void AddAspNetCoreInstrumentationSources( // For users not using hosting package? builder.AddSource(HttpInListener.AspNetCoreActivitySourceName); } -#if !NET7_0_OR_GREATER - } - else - { - builder.AddSource(HttpInListener.ActivitySourceName); - builder.AddLegacySource(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore - } +#else + builder.AddSource(HttpInListener.ActivitySourceName); + builder.AddLegacySource(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore #endif } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index ea60b16a8ee..15564885cca 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -8,12 +8,6 @@ instance obtained from `IHttpActivityFeature.Activity`. ([#5136](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5136)) -* Fixed metrics instrumentation when library targeting .NET 6 or .NET 7 - was loaded by .NET 8. - Fixed traces instrumentation when library targeting .NET 6 - was loaded by .NET 7 or .NET 8. - ([#5252](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5252)) - * Fixed an issue where the `http.route` attribute was not set on either the `Activity` or `http.server.request.duration` metric generated from a request when an exception handling middleware is invoked. One caveat is that diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 9031a0bb61c..03e3fda3d21 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -35,12 +35,12 @@ internal class HttpInListener : ListenerHandler internal static readonly Version Version = AssemblyName.Version; internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString()); + private const string DiagnosticSourceName = "Microsoft.AspNetCore"; + #if !NET7_0_OR_GREATER - internal static readonly bool Net7OrGreater = Environment.Version.Major >= 7; + private static readonly bool Net7OrGreater = Environment.Version.Major >= 7; #endif - private const string DiagnosticSourceName = "Microsoft.AspNetCore"; - private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => { if (request.Headers.TryGetValue(name, out var value)) From 3054ec1ed2273e3acdeed59be598b464b9d3d2db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Thu, 1 Feb 2024 06:59:05 +0100 Subject: [PATCH 5/7] remove conditional compilation to simplify code --- .../Implementation/HttpInListener.cs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 03e3fda3d21..5c93a249fbf 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -37,9 +37,7 @@ internal class HttpInListener : ListenerHandler private const string DiagnosticSourceName = "Microsoft.AspNetCore"; -#if !NET7_0_OR_GREATER private static readonly bool Net7OrGreater = Environment.Version.Major >= 7; -#endif private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => { @@ -124,22 +122,17 @@ public void OnStartActivity(Activity activity, object payload) // This makes the new activity as a "sibling" of the activity created by // Asp.Net Core. Activity newOne; -#if !NET7_0_OR_GREATER if (Net7OrGreater) { -#endif - - // For NET7.0 onwards activity is created using ActivitySource so, - // we will use the source of the activity to create the new one. - newOne = activity.Source.CreateActivity(ActivityOperationName, ActivityKind.Server, ctx.ActivityContext); -#if !NET7_0_OR_GREATER + // For NET7.0 onwards activity is created using ActivitySource so, + // we will use the source of the activity to create the new one. + newOne = activity.Source.CreateActivity(ActivityOperationName, ActivityKind.Server, ctx.ActivityContext); } else { newOne = new Activity(ActivityOperationName); newOne.SetParentId(ctx.ActivityContext.TraceId, ctx.ActivityContext.SpanId, ctx.ActivityContext.TraceFlags); } -#endif newOne.TraceStateString = ctx.ActivityContext.TraceState; @@ -178,13 +171,11 @@ public void OnStartActivity(Activity activity, object payload) return; } -#if !NET7_0_OR_GREATER if (!Net7OrGreater) { ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource); ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Server); } -#endif var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/"; activity.DisplayName = GetDisplayName(request.Method); From 04f79631a278b636648199f88164b28784a7f1bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Mon, 5 Feb 2024 07:31:19 +0100 Subject: [PATCH 6/7] PR feedback - conditional check consistent in AspNetCoreInstrumentationTracerProviderBuilderExtensions with HttpInListener --- ...entationTracerProviderBuilderExtensions.cs | 29 ++++++++++--------- .../Implementation/HttpInListener.cs | 3 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs index d37d77ae3fa..8ec35718ccd 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationTracerProviderBuilderExtensions.cs @@ -1,9 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#if NET7_0_OR_GREATER using System.Diagnostics; -#endif using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using OpenTelemetry.Instrumentation.AspNetCore; @@ -101,22 +99,25 @@ private static void AddAspNetCoreInstrumentationSources( // For .NET7.0 onwards activity will be created using activitySource. // https://github.com/dotnet/aspnetcore/blob/bf3352f2422bf16fa3ca49021f0e31961ce525eb/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs#L327 // For .NET6.0 and below, we will continue to use legacy way. -#if NET7_0_OR_GREATER - // TODO: Check with .NET team to see if this can be prevented - // as this allows user to override the ActivitySource. - var activitySourceService = serviceProvider?.GetService(); - if (activitySourceService != null) + if (HttpInListener.Net7OrGreater) { - builder.AddSource(activitySourceService.Name); + // TODO: Check with .NET team to see if this can be prevented + // as this allows user to override the ActivitySource. + var activitySourceService = serviceProvider?.GetService(); + if (activitySourceService != null) + { + builder.AddSource(activitySourceService.Name); + } + else + { + // For users not using hosting package? + builder.AddSource(HttpInListener.AspNetCoreActivitySourceName); + } } else { - // For users not using hosting package? - builder.AddSource(HttpInListener.AspNetCoreActivitySourceName); + builder.AddSource(HttpInListener.ActivitySourceName); + builder.AddLegacySource(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore } -#else - builder.AddSource(HttpInListener.ActivitySourceName); - builder.AddLegacySource(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore -#endif } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 5c93a249fbf..3e9b93292bd 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -34,11 +34,10 @@ internal class HttpInListener : ListenerHandler internal static readonly string ActivitySourceName = AssemblyName.Name; internal static readonly Version Version = AssemblyName.Version; internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString()); + internal static readonly bool Net7OrGreater = Environment.Version.Major >= 7; private const string DiagnosticSourceName = "Microsoft.AspNetCore"; - private static readonly bool Net7OrGreater = Environment.Version.Major >= 7; - private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => { if (request.Headers.TryGetValue(name, out var value)) From 0c1af8ba03aefac8547bfdb4c6d0a8ad541d4bc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Mon, 5 Feb 2024 07:43:27 +0100 Subject: [PATCH 7/7] PR feedback - bring back perf code for .NET6 --- .../Implementation/HttpInListener.cs | 11 ++++++- src/Shared/ActivityHelperExtensions.cs | 29 +++++++++++++++++++ .../Trace/ActivityExtensionsTest.cs | 23 +++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 3e9b93292bd..97807d41512 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -268,7 +268,16 @@ public void OnStopActivity(Activity activity, object payload) } } - var tagValue = activity.GetTagValue("IsCreatedByInstrumentation"); + object tagValue; + if (Net7OrGreater) + { + tagValue = activity.GetTagValue("IsCreatedByInstrumentation"); + } + else + { + _ = activity.TryCheckFirstTag("IsCreatedByInstrumentation", out tagValue); + } + if (ReferenceEquals(tagValue, bool.TrueString)) { // If instrumentation started a new Activity, it must diff --git a/src/Shared/ActivityHelperExtensions.cs b/src/Shared/ActivityHelperExtensions.cs index a7510caa8fd..b237ccd5457 100644 --- a/src/Shared/ActivityHelperExtensions.cs +++ b/src/Shared/ActivityHelperExtensions.cs @@ -83,4 +83,33 @@ public static bool TryGetStatus(this Activity activity, out StatusCode statusCod return null; } + + /// + /// Checks if the user provided tag name is the first tag of the and retrieves the tag value. + /// + /// Activity instance. + /// Tag name. + /// Tag value. + /// if the first tag of the supplied Activity matches the user provide tag name. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool TryCheckFirstTag(this Activity activity, string tagName, out object? tagValue) + { + Debug.Assert(activity != null, "Activity should not be null"); + + var enumerator = activity!.EnumerateTagObjects(); + + if (enumerator.MoveNext()) + { + ref readonly var tag = ref enumerator.Current; + + if (tag.Key == tagName) + { + tagValue = tag.Value; + return true; + } + } + + tagValue = null; + return false; + } } diff --git a/test/OpenTelemetry.Api.Tests/Trace/ActivityExtensionsTest.cs b/test/OpenTelemetry.Api.Tests/Trace/ActivityExtensionsTest.cs index a48cebcffa2..31fbf806725 100644 --- a/test/OpenTelemetry.Api.Tests/Trace/ActivityExtensionsTest.cs +++ b/test/OpenTelemetry.Api.Tests/Trace/ActivityExtensionsTest.cs @@ -198,4 +198,27 @@ public void GetTagValue() Assert.Null(activity.GetTagValue("tag1")); Assert.Null(activity.GetTagValue("Tag2")); } + + [Theory] + [InlineData("Key", "Value", true)] + [InlineData("CustomTag", null, false)] + public void TryCheckFirstTag(string tagName, object expectedTagValue, bool expectedResult) + { + using var activity = new Activity("Test"); + activity.SetTag("Key", "Value"); + + var result = activity.TryCheckFirstTag(tagName, out var tagValue); + Assert.Equal(expectedResult, result); + Assert.Equal(expectedTagValue, tagValue); + } + + [Fact] + public void TryCheckFirstTagReturnsFalseForActivityWithNoTags() + { + using var activity = new Activity("Test"); + + var result = activity.TryCheckFirstTag("Key", out var tagValue); + Assert.False(result); + Assert.Null(tagValue); + } }