Update HttpInListener to add gRPC tags - By creating a new activity with the OperationName used by the framework

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

@@ -31,7 +31,6 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
     internal class HttpInListener : ListenerHandler
     {
         internal const string ActivityOperationName = "Microsoft.AspNetCore.Hosting.HttpRequestIn";
-        internal const string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener";
         internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName();
         internal static readonly string ActivitySourceName = AssemblyName.Name;
         internal static readonly Version Version = AssemblyName.Version;
@@ -92,13 +91,15 @@ public override 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.
-                    Activity newOne = new Activity(ActivityNameByHttpInListener);
+                    Activity newOne = new Activity(ActivityOperationName);
                     newOne.SetParentId(ctx.ActivityContext.TraceId, ctx.ActivityContext.SpanId, ctx.ActivityContext.TraceFlags);
                     newOne.TraceStateString = ctx.ActivityContext.TraceState;
 
                     // Starting the new activity make it the Activity.Current one.
                     newOne.Start();
 
+                    newOne.SetCustomProperty("IsCreatedByInstrumentation", true);
+
                     // Set IsAllDataRequested to false for the activity created by the framework to only export the sibling activity and not the framework activity
                     activity.IsAllDataRequested = false;
                     activity = newOne;
@@ -194,7 +195,7 @@ public override void OnStopActivity(Activity activity, object payload)
                 }
             }
 
-            if (activity.OperationName.Equals(ActivityNameByHttpInListener, StringComparison.Ordinal))
+            if (activity.GetCustomProperty("IsCreatedByInstrumentation") is bool isCreatedByInstrumentation && isCreatedByInstrumentation)
             {
                 // If instrumentation started a new Activity, it must
                 // be stopped here.

src/OpenTelemetry.Instrumentation.AspNetCore/TracerProviderBuilderExtensions.cs

@@ -44,8 +44,7 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation(
             configureAspNetCoreInstrumentationOptions?.Invoke(aspnetCoreOptions);
             builder.AddInstrumentation(() => new AspNetCoreInstrumentation(aspnetCoreOptions));
             builder.AddSource(HttpInListener.ActivitySourceName);
-            builder.AddLegacyActivity(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore
-            builder.AddLegacyActivity(HttpInListener.ActivityNameByHttpInListener); // for the sibling activities created by the instrumentation library
+            builder.AddLegacyActivity(HttpInListener.ActivityOperationName); // for the activities created by AspNetCore framework and instrumentation
 
             return builder;
         }

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs

@@ -174,10 +174,10 @@ public async Task SuccessfulTemplateControllerCallUsesParentContext()
             // List of invocations
             // 1. SetParentProvider for TracerProviderSdk
             // 2. OnStart for the activity created by AspNetCore with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn
-            // 3. OnStart for the sibling activity created by the instrumentation library with the OperationName: ActivityCreatedByHttpInListener
-            // 4. OnEnd for the sibling activity created by the instrumentation library with the OperationName: ActivityCreatedByHttpInListener
+            // 3. OnStart for the sibling activity created by the instrumentation library with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn and the customProperty IsCreatedByInstrumentation set to true
+            // 4. OnEnd for the sibling activity created by the instrumentation library with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn and the customProperty IsCreatedByInstrumentation set to true
 
-            // we should only call Processor.OnEnd once for the sibling activity with the OperationName ActivityCreatedByHttpInListener
+            // we should only call Processor.OnEnd once for the sibling activity
             Assert.Single(activityProcessor.Invocations, invo => invo.Method.Name == "OnEnd");
             var activity = activityProcessor.Invocations.FirstOrDefault(invo => invo.Method.Name == "OnEnd").Arguments[0] as Activity;
 
@@ -188,7 +188,7 @@ public async Task SuccessfulTemplateControllerCallUsesParentContext()
 #else
             // ASP.NET Core before 3.x is not W3C aware and hence Activity created by it
             // is always ignored and new one is created by the Instrumentation
-            Assert.Equal("ActivityCreatedByHttpInListener", activity.OperationName);
+            Assert.True(activity.GetCustomProperty("IsCreatedByInstrumentation") is bool isCreatedByInstrumentation && isCreatedByInstrumentation);
 #endif
             Assert.Equal(ActivityKind.Server, activity.Kind);
             Assert.Equal("api/Values/{id}", activity.DisplayName);
@@ -237,8 +237,8 @@ public async Task CustomPropagator()
             // List of invocations on the processor
             // 1. SetParentProvider for TracerProviderSdk
             // 2. OnStart for the activity created by AspNetCore with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn
-            // 3. OnStart for the sibling activity created by the instrumentation library with the OperationName: ActivityCreatedByHttpInListener
-            // 4. OnEnd for the sibling activity created by the instrumentation library with the OperationName: ActivityCreatedByHttpInListener
+            // 3. OnStart for the sibling activity created by the instrumentation library with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn and the customProperty IsCreatedByInstrumentation set to true
+            // 4. OnEnd for the sibling activity created by the instrumentation library with the OperationName: Microsoft.AspNetCore.Hosting.HttpRequestIn and the customProperty IsCreatedByInstrumentation set to true
             Assert.Equal(4, activityProcessor.Invocations.Count);
 
             var startedActivities = activityProcessor.Invocations.Where(invo => invo.Method.Name == "OnStart");
@@ -247,23 +247,23 @@ public async Task CustomPropagator()
             Assert.Single(stoppedActivities);
 
             // The activity created by the framework and the sibling activity are both sent to Processor.OnStart
-            Assert.Contains(startedActivities, item =>
+            Assert.Equal(2, startedActivities.Count(item =>
             {
                 var startedActivity = item.Arguments[0] as Activity;
                 return startedActivity.OperationName == HttpInListener.ActivityOperationName;
-            });
+            }));
 
-            Assert.Contains(startedActivities, item =>
+            Assert.Single(startedActivities, item =>
             {
                 var startedActivity = item.Arguments[0] as Activity;
-                return startedActivity.OperationName == HttpInListener.ActivityNameByHttpInListener;
+                return startedActivity.GetCustomProperty("IsCreatedByInstrumentation") is bool isCreatedByInstrumentation && isCreatedByInstrumentation;
             });
 
             // Only the sibling activity is sent to Processor.OnEnd
-            Assert.Contains(stoppedActivities, item =>
+            Assert.Single(stoppedActivities, item =>
             {
                 var stoppedActivity = item.Arguments[0] as Activity;
-                return stoppedActivity.OperationName == HttpInListener.ActivityNameByHttpInListener;
+                return stoppedActivity.GetCustomProperty("IsCreatedByInstrumentation") is bool isCreatedByInstrumentation && isCreatedByInstrumentation;
             });
 
             var activity = activityProcessor.Invocations.FirstOrDefault(invo => invo.Method.Name == "OnEnd").Arguments[0] as Activity;

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

@@ -20,12 +20,15 @@
 using System.Net;
 using System.Threading;
 using Greet;
+using Grpc.Core;
 using Grpc.Net.Client;
 using Moq;
+using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Instrumentation.Grpc.Tests.Services;
 using OpenTelemetry.Instrumentation.GrpcNetClient;
 using OpenTelemetry.Trace;
 using Xunit;
+using Status = OpenTelemetry.Trace.Status;
 
 namespace OpenTelemetry.Instrumentation.Grpc.Tests
 {
@@ -108,6 +111,86 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(bool? enableGrpcA
             Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string);
         }
 
+        [Theory]
+        [InlineData(null)]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void GrpcAspNetCoreInstrumentationAddsCorrectAttributesWhenItCreatesNewActivity(bool? enableGrpcAspNetCoreSupport)
+        {
+            // B3Propagator along with the headers passed to the client.SayHello ensure that the instrumentation creates a sibling activity
+            Sdk.SetDefaultTextMapPropagator(new B3Propagator());
+            var processor = new Mock<BaseProcessor<Activity>>();
+            var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder();
+
+            if (enableGrpcAspNetCoreSupport.HasValue)
+            {
+                tracerProviderBuilder.AddAspNetCoreInstrumentation(options =>
+                {
+                    options.EnableGrpcAspNetCoreSupport = enableGrpcAspNetCoreSupport.Value;
+                });
+            }
+            else
+            {
+                tracerProviderBuilder.AddAspNetCoreInstrumentation();
+            }
+
+            using var tracerProvider = tracerProviderBuilder
+                .AddProcessor(processor.Object)
+                .Build();
+
+            var clientLoopbackAddresses = new[] { IPAddress.Loopback.ToString(), IPAddress.IPv6Loopback.ToString() };
+            var uri = new Uri($"http://localhost:{this.server.Port}");
+
+            using var channel = GrpcChannel.ForAddress(uri);
+            var client = new Greeter.GreeterClient(channel);
+            var headers = new Metadata();
+            headers.Add("traceparent", "00-120dc44db5b736468afb112197b0dbd3-5dfbdf27ec544544-01");
+            headers.Add("x-b3-traceid", "120dc44db5b736468afb112197b0dbd3");
+            headers.Add("x-b3-spanid", "b0966f651b9e0126");
+            headers.Add("x-b3-sampled", "1");
+            client.SayHello(new HelloRequest(), headers);
+
+            WaitForProcessorInvocations(processor, 4);
+
+            Assert.Equal(4, processor.Invocations.Count); // SetParentProvider, OnStart (framework activity), OnStart (instrumentation activity), OnStop (instrumentation activity)
+            var activity = GetActivityFromProcessorInvocation(processor, nameof(processor.Object.OnEnd), OperationNameHttpRequestIn);
+
+            Assert.Equal(ActivityKind.Server, activity.Kind);
+
+            if (!enableGrpcAspNetCoreSupport.HasValue || enableGrpcAspNetCoreSupport.Value)
+            {
+                Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem));
+                Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService));
+                Assert.Equal("SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod));
+                Assert.Contains(activity.GetTagValue(SemanticConventions.AttributeNetPeerIp), clientLoopbackAddresses);
+                Assert.NotEqual(0, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort));
+                Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName));
+                Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName));
+                Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode));
+            }
+            else
+            {
+                Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName));
+                Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName));
+            }
+
+            Assert.Equal(Status.Unset, activity.GetStatus());
+
+            // The following are http.* attributes that are also included on the span for the gRPC invocation.
+            Assert.Equal($"localhost:{this.server.Port}", activity.GetTagValue(SemanticConventions.AttributeHttpHost));
+            Assert.Equal("POST", activity.GetTagValue(SemanticConventions.AttributeHttpMethod));
+            Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SpanAttributeConstants.HttpPathKey));
+            Assert.Equal($"http://localhost:{this.server.Port}/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpUrl));
+            Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string);
+
+            // Set the SDK to use the default propagator for other unit tests
+            Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[]
+            {
+                new TraceContextPropagator(),
+                new BaggagePropagator(),
+            }));
+        }
+
         public void Dispose()
         {
             this.server.Dispose();