Optimize gRPC client instrumentation for suppress instrumentation (#1904

) * Optimize gRPC client instrumentation for SuppressInstrumentation * Udated unit tests
open-telemetry · Mar 13, 2021 · 41a3371 · 41a3371
1 parent c404e19
commit 41a3371
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 6 deletions.
diff --git a/...penTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs b/...penTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs
@@ -43,6 +43,22 @@ public GrpcClientDiagnosticListener(GrpcClientInstrumentationOptions options)
 
         public override void OnStartActivity(Activity activity, object payload)
         {
+            // The overall flow of what GrpcClient library does is as below:
+            // Activity.Start()
+            // DiagnosticSource.WriteEvent("Start", payload)
+            // DiagnosticSource.WriteEvent("Stop", payload)
+            // Activity.Stop()
+
+            // This method is in the WriteEvent("Start", payload) path.
+            // By this time, samplers have already run and
+            // activity.IsAllDataRequested populated accordingly.
+
+            if (Sdk.SuppressInstrumentation)
+            {
+                return;
+            }
+
+            // Ensure context propagation irrespective of sampling decision
             if (!this.startRequestFetcher.TryFetch(payload, out HttpRequestMessage request) || request == null)
             {
                 GrpcInstrumentationEventSource.Log.NullPayload(nameof(GrpcClientDiagnosticListener), nameof(this.OnStartActivity));
@@ -78,15 +94,15 @@ public override void OnStartActivity(Activity activity, object payload)
                     HttpRequestMessageContextPropagation.HeaderValueSetter);
             }
 
-            var grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity);
+            if (activity.IsAllDataRequested)
+            {
+                ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
+                ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client);
 
-            activity.DisplayName = grpcMethod?.Trim('/');
+                var grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity);
 
-            ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource);
-            ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client);
+                activity.DisplayName = grpcMethod?.Trim('/');
 
-            if (activity.IsAllDataRequested)
-            {
                 activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc);
 
                 if (GrpcTagHelper.TryParseRpcServiceAndRpcMethod(grpcMethod, out var rpcService, out var rpcMethod))

diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs
@@ -23,6 +23,7 @@
 using Microsoft.AspNetCore.Http;
 using Moq;
 using OpenTelemetry.Instrumentation.GrpcNetClient;
+using OpenTelemetry.Instrumentation.GrpcNetClient.Implementation;
 using OpenTelemetry.Trace;
 using Xunit;
 
@@ -143,6 +144,7 @@ public void GrpcAndHttpClientInstrumentationIsInvoked(bool shouldEnrich)
 
             ValidateGrpcActivity(grpcSpan);
             Assert.Equal($"greet.Greeter/SayHello", grpcSpan.DisplayName);
+            Assert.Equal(0, grpcSpan.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode));
             Assert.Equal($"HTTP POST", httpSpan.DisplayName);
             Assert.Equal(grpcSpan.SpanId, httpSpan.ParentSpanId);
         }
@@ -274,6 +276,7 @@ public void GrpcPropagatesContextWithSuppressInstrumentation()
             Assert.Equal($"greet.Greeter/SayHello", serverActivity.DisplayName);
             Assert.Equal(clientActivity.TraceId, serverActivity.TraceId);
             Assert.Equal(clientActivity.SpanId, serverActivity.ParentSpanId);
+            Assert.Equal(0, clientActivity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode));
             Assert.Contains("item1=value1", serverActivity.GetCustomProperty("BaggageString") as string);
         }
 
@@ -286,11 +289,14 @@ public void Grpc_BadArgs()
 
         private static void ValidateGrpcActivity(Activity activityToValidate)
         {
+            Assert.Equal(GrpcClientDiagnosticListener.ActivitySourceName, activityToValidate.Source.Name);
+            Assert.Equal(GrpcClientDiagnosticListener.Version.ToString(), activityToValidate.Source.Version);
             Assert.Equal(ActivityKind.Client, activityToValidate.Kind);
         }
 
         private static void ActivityEnrichment(Activity activity, string method, object obj)
         {
+            Assert.True(activity.IsAllDataRequested);
             switch (method)
             {
                 case "OnStartActivity":