Merge branch 'main' into cijothomas/activity-exception-doc-fix

src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreTraceInstrumentationOptions.cs

@@ -3,6 +3,8 @@
 
 using System.Diagnostics;
 using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Configuration;
+using OpenTelemetry.Internal;
 
 namespace OpenTelemetry.Instrumentation.AspNetCore;
 
@@ -11,6 +13,24 @@ namespace OpenTelemetry.Instrumentation.AspNetCore;
 /// </summary>
 public class AspNetCoreTraceInstrumentationOptions
 {
+    /// <summary>
+    /// Initializes a new instance of the <see cref="AspNetCoreTraceInstrumentationOptions"/> class.
+    /// </summary>
+    public AspNetCoreTraceInstrumentationOptions()
+        : this(new ConfigurationBuilder().AddEnvironmentVariables().Build())
+    {
+    }
+
+    internal AspNetCoreTraceInstrumentationOptions(IConfiguration configuration)
+    {
+        Debug.Assert(configuration != null, "configuration was null");
+
+        if (configuration.TryGetBoolValue("OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION", out var enableGrpcInstrumentation))
+        {
+            this.EnableGrpcAspNetCoreSupport = enableGrpcInstrumentation;
+        }
+    }
+
     /// <summary>
     /// Gets or sets a filter function that determines whether or not to
     /// collect telemetry on a per request basis.
@@ -64,17 +84,11 @@ public class AspNetCoreTraceInstrumentationOptions
     /// </remarks>
     public bool RecordException { get; set; }
 
-    /*
-     * Removing for stable release of http instrumentation.
-     * grpc semantic conventions are not yet stable so this option will not be part of stable package.
-    #if NET6_0_OR_GREATER
-        /// <summary>
-        /// Gets or sets a value indicating whether RPC attributes are added to an Activity when using Grpc.AspNetCore. Default is true.
-        /// </summary>
-        /// <remarks>
-        /// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md.
-        /// </remarks>
-        public bool EnableGrpcAspNetCoreSupport { get; set; } = true;
-    #endif
-    */
+    /// <summary>
+    /// Gets or sets a value indicating whether RPC attributes are added to an Activity when using Grpc.AspNetCore.
+    /// </summary>
+    /// <remarks>
+    /// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md.
+    /// </remarks>
+    internal bool EnableGrpcAspNetCoreSupport { get; set; }
 }

src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md

@@ -2,6 +2,20 @@
 
 ## Unreleased
 
+* Re-introduced support for gRPC instrumentation as an opt-in experimental
+  feature. From now onwards, gRPC can be enabled by setting
+  `OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION` flag to
+  `True`. `OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION` can
+  be set as an environment variable or via IConfiguration. The change is
+  introduced in order to support stable release of `http` instrumentation.
+  Semantic conventions for RPC is still
+  [experimental](https://github.com/open-telemetry/semantic-conventions/tree/main/docs/rpc)
+  and hence the package will only support it as an opt-in experimental feature.
+  Note that the support was removed in `1.6.0-rc.1` version of the package and
+  versions released before `1.6.0-rc.1` had gRPC instrumentation enabled by
+  default.
+  ([#5130](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5130))
+
 ## 1.6.0-rc.1
 
 Released 2023-Dec-01

src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs

@@ -6,17 +6,13 @@
 using System.Diagnostics.CodeAnalysis;
 #endif
 using System.Reflection;
-#if !NETSTANDARD2_0
 using System.Runtime.CompilerServices;
-#endif
 using Microsoft.AspNetCore.Http;
 #if !NETSTANDARD
 using Microsoft.AspNetCore.Routing;
 #endif
 using OpenTelemetry.Context.Propagation;
-#if !NETSTANDARD2_0
 using OpenTelemetry.Instrumentation.GrpcNetClient;
-#endif
 using OpenTelemetry.Internal;
 using OpenTelemetry.Trace;
 
@@ -170,7 +166,7 @@ public void OnStartActivity(Activity activity, object payload)
 #endif
 
             var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/";
-            activity.DisplayName = this.GetDisplayName(request.Method);
+            activity.DisplayName = GetDisplayName(request.Method);
 
             // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md
 
@@ -233,20 +229,18 @@ public void OnStopActivity(Activity activity, object payload)
             var routePattern = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
             if (!string.IsNullOrEmpty(routePattern))
             {
-                activity.DisplayName = this.GetDisplayName(context.Request.Method, routePattern);
+                activity.DisplayName = GetDisplayName(context.Request.Method, routePattern);
                 activity.SetTag(SemanticConventions.AttributeHttpRoute, routePattern);
             }
 #endif
 
             activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
-            /*
-            #if !NETSTANDARD2_0
-
-                        if (this.options.EnableGrpcAspNetCoreSupport && TryGetGrpcMethod(activity, out var grpcMethod))
-                        {
-                            this.AddGrpcAttributes(activity, grpcMethod, context);
-                        }
-            */
+
+            if (this.options.EnableGrpcAspNetCoreSupport && TryGetGrpcMethod(activity, out var grpcMethod))
+            {
+                AddGrpcAttributes(activity, grpcMethod, context);
+            }
+
             if (activity.Status == ActivityStatusCode.Unset)
             {
                 activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, response.StatusCode));
@@ -327,27 +321,15 @@ static bool TryFetchException(object payload, out Exception exc)
             => ExceptionPropertyFetcher.TryFetch(payload, out exc) && exc != null;
     }
 
-#if !NETSTANDARD2_0
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private static bool TryGetGrpcMethod(Activity activity, out string grpcMethod)
     {
         grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity);
         return !string.IsNullOrEmpty(grpcMethod);
     }
-#endif
-
-    private string GetDisplayName(string httpMethod, string httpRoute = null)
-    {
-        var normalizedMethod = RequestMethodHelper.GetNormalizedHttpMethod(httpMethod);
-
-        return string.IsNullOrEmpty(httpRoute)
-            ? normalizedMethod
-            : $"{normalizedMethod} {httpRoute}";
-    }
 
-#if !NETSTANDARD2_0
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    private void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context)
+    private static void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context)
     {
         // The RPC semantic conventions indicate the span name
         // should not have a leading forward slash.
@@ -389,5 +371,13 @@ private void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext
             }
         }
     }
-#endif
+
+    private static string GetDisplayName(string httpMethod, string httpRoute = null)
+    {
+        var normalizedMethod = RequestMethodHelper.GetNormalizedHttpMethod(httpMethod);
+
+        return string.IsNullOrEmpty(httpRoute)
+            ? normalizedMethod
+            : $"{normalizedMethod} {httpRoute}";
+    }
 }

src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj

@@ -11,11 +11,13 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.GrpcNetClient\GrpcTagHelper.cs" Link="Includes\GrpcTagHelper.cs" />
     <Compile Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.GrpcNetClient\StatusCanonicalCode.cs" Link="Includes\StatusCanonicalCode.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\RequestMethodHelper.cs" Link="Includes\RequestMethodHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(RunningDotNetPack)' != 'true'">
@@ -28,10 +30,11 @@
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
-    <PackageReference Include="System.Text.Encodings.Web" />
     <PackageReference Include="Microsoft.AspNetCore.Http.Abstractions" />
     <PackageReference Include="Microsoft.AspNetCore.Http.Features" />
+    <PackageReference Include="Microsoft.Extensions.Configuration" />
     <PackageReference Include="Microsoft.Extensions.Options" />
+    <PackageReference Include="System.Text.Encodings.Web" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">

src/OpenTelemetry.Instrumentation.AspNetCore/TracerProviderBuilderExtensions.cs

@@ -62,6 +62,8 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation(
             {
                 services.Configure(name, configureAspNetCoreTraceInstrumentationOptions);
             }
+
+            services.RegisterOptionsFactory(configuration => new AspNetCoreTraceInstrumentationOptions(configuration));
         });
 
         if (builder is IDeferredTracerProviderBuilder deferredTracerProviderBuilder)

test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs

@@ -3,8 +3,19 @@
 
 #if NET6_0_OR_GREATER
 using System.Diagnostics;
+using System.Net;
+using Greet;
+using Grpc.Core;
+using Grpc.Net.Client;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
+using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Instrumentation.Grpc.Services.Tests;
+using OpenTelemetry.Instrumentation.GrpcNetClient;
+using OpenTelemetry.Trace;
 using Xunit;
+using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
+using Status = OpenTelemetry.Trace.Status;
 
 namespace OpenTelemetry.Instrumentation.Grpc.Tests;
 
@@ -21,34 +32,26 @@ public GrpcTests()
         this.server = new GrpcServer<GreeterService>();
     }
 
-    /*
     [Theory]
     [InlineData(null)]
-    [InlineData(true)]
-    [InlineData(false)]
-    public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(bool? enableGrpcAspNetCoreSupport)
+    [InlineData("true")]
+    [InlineData("false")]
+    [InlineData("True")]
+    [InlineData("False")]
+    public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(string enableGrpcAspNetCoreSupport)
     {
         var configuration = new ConfigurationBuilder()
-            .AddInMemoryCollection(new Dictionary<string, string> { [SemanticConventionOptInKeyName] = "http" })
+            .AddInMemoryCollection(new Dictionary<string, string>
+            {
+                [SemanticConventionOptInKeyName] = "http",
+                ["OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION"] = enableGrpcAspNetCoreSupport,
+            })
             .Build();
 
         var exportedItems = new List<Activity>();
-        var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder()
-            .ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration));
-
-        if (enableGrpcAspNetCoreSupport.HasValue)
-        {
-            tracerProviderBuilder.AddAspNetCoreInstrumentation(options =>
-            {
-                options.EnableGrpcAspNetCoreSupport = enableGrpcAspNetCoreSupport.Value;
-            });
-        }
-        else
-        {
-            tracerProviderBuilder.AddAspNetCoreInstrumentation();
-        }
-
-        using var tracerProvider = tracerProviderBuilder
+        using var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder()
+            .ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration))
+            .AddAspNetCoreInstrumentation()
             .AddInMemoryExporter(exportedItems)
             .Build();
 
@@ -66,7 +69,7 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(bool? enableGrpcA
 
         Assert.Equal(ActivityKind.Server, activity.Kind);
 
-        if (!enableGrpcAspNetCoreSupport.HasValue || enableGrpcAspNetCoreSupport.Value)
+        if (enableGrpcAspNetCoreSupport != null && enableGrpcAspNetCoreSupport.Equals("true", StringComparison.OrdinalIgnoreCase))
         {
             Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem));
             Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService));
@@ -99,30 +102,28 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(bool? enableGrpcA
     [Theory(Skip = "Skipping for .NET 6 and higher due to bug #3023")]
 #endif
     [InlineData(null)]
-    [InlineData(true)]
-    [InlineData(false)]
-    public void GrpcAspNetCoreInstrumentationAddsCorrectAttributesWhenItCreatesNewActivity(bool? enableGrpcAspNetCoreSupport)
+    [InlineData("true")]
+    [InlineData("false")]
+    [InlineData("True")]
+    [InlineData("False")]
+    public void GrpcAspNetCoreInstrumentationAddsCorrectAttributesWhenItCreatesNewActivity(string enableGrpcAspNetCoreSupport)
     {
         try
         {
             // B3Propagator along with the headers passed to the client.SayHello ensure that the instrumentation creates a sibling activity
             Sdk.SetDefaultTextMapPropagator(new Extensions.Propagators.B3Propagator());
             var exportedItems = new List<Activity>();
-            var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder();
-
-            if (enableGrpcAspNetCoreSupport.HasValue)
-            {
-                tracerProviderBuilder.AddAspNetCoreInstrumentation(options =>
-                {
-                    options.EnableGrpcAspNetCoreSupport = enableGrpcAspNetCoreSupport.Value;
-                });
-            }
-            else
-            {
-                tracerProviderBuilder.AddAspNetCoreInstrumentation();
-            }
-
-            using var tracerProvider = tracerProviderBuilder
+            var configuration = new ConfigurationBuilder()
+           .AddInMemoryCollection(new Dictionary<string, string>
+           {
+               [SemanticConventionOptInKeyName] = "http",
+               ["OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION"] = enableGrpcAspNetCoreSupport,
+           })
+           .Build();
+
+            using var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder()
+                .ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration))
+                .AddAspNetCoreInstrumentation()
                 .AddInMemoryExporter(exportedItems)
                 .Build();
 
@@ -146,7 +147,7 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributesWhenItCreatesNewAc
 
             Assert.Equal(ActivityKind.Server, activity.Kind);
 
-            if (!enableGrpcAspNetCoreSupport.HasValue || enableGrpcAspNetCoreSupport.Value)
+            if (enableGrpcAspNetCoreSupport != null && enableGrpcAspNetCoreSupport.Equals("true", StringComparison.OrdinalIgnoreCase))
             {
                 Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem));
                 Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService));
@@ -183,7 +184,7 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributesWhenItCreatesNewAc
             }));
         }
     }
-    */
+
     public void Dispose()
     {
         this.server.Dispose();