[Instrumentation.GrpcNetClient] Nullable (open-telemetry#1815)

Author: Kielek

src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/PublicAPI.Shipped.txt

@@ -0,0 +1 @@
+#nullable enable

src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/PublicAPI.Unshipped.txt

@@ -1,12 +1,12 @@
 OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions
-OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.EnrichWithHttpRequestMessage.get -> System.Action<System.Diagnostics.Activity, System.Net.Http.HttpRequestMessage>
+OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.EnrichWithHttpRequestMessage.get -> System.Action<System.Diagnostics.Activity!, System.Net.Http.HttpRequestMessage!>?
 OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.EnrichWithHttpRequestMessage.set -> void
-OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.EnrichWithHttpResponseMessage.get -> System.Action<System.Diagnostics.Activity, System.Net.Http.HttpResponseMessage>
+OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.EnrichWithHttpResponseMessage.get -> System.Action<System.Diagnostics.Activity!, System.Net.Http.HttpResponseMessage!>?
 OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.EnrichWithHttpResponseMessage.set -> void
 OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.GrpcClientTraceInstrumentationOptions() -> void
 OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.SuppressDownstreamInstrumentation.get -> bool
 OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions.SuppressDownstreamInstrumentation.set -> void
 OpenTelemetry.Trace.TracerProviderBuilderExtensions
-static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddGrpcClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder
-static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddGrpcClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action<OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
-static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddGrpcClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddGrpcClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder! builder) -> OpenTelemetry.Trace.TracerProviderBuilder!
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddGrpcClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder! builder, string? name, System.Action<OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions!>? configure) -> OpenTelemetry.Trace.TracerProviderBuilder!
+static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddGrpcClientInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Action<OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientTraceInstrumentationOptions!>? configure) -> OpenTelemetry.Trace.TracerProviderBuilder!

src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentation.cs

@@ -16,7 +16,7 @@ internal sealed class GrpcClientInstrumentation : IDisposable
     /// Initializes a new instance of the <see cref="GrpcClientInstrumentation"/> class.
     /// </summary>
     /// <param name="options">Configuration options for Grpc client instrumentation.</param>
-    public GrpcClientInstrumentation(GrpcClientTraceInstrumentationOptions options = null)
+    public GrpcClientInstrumentation(GrpcClientTraceInstrumentationOptions options)
     {
         this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new GrpcClientDiagnosticListener(options), isEnabledFilter: null, GrpcInstrumentationEventSource.Log.UnknownErrorProcessingEvent);
         this.diagnosticSourceSubscriber.Subscribe();

src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientTraceInstrumentationOptions.cs

@@ -22,7 +22,7 @@ public class GrpcClientTraceInstrumentationOptions
     /// <para><see cref="Activity"/>: the activity being enriched.</para>
     /// <para><see cref="HttpRequestMessage"/> object from which additional information can be extracted to enrich the activity.</para>
     /// </remarks>
-    public Action<Activity, HttpRequestMessage> EnrichWithHttpRequestMessage { get; set; }
+    public Action<Activity, HttpRequestMessage>? EnrichWithHttpRequestMessage { get; set; }
 
     /// <summary>
     /// Gets or sets an action to enrich an Activity with <see cref="HttpResponseMessage"/>.
@@ -31,5 +31,5 @@ public class GrpcClientTraceInstrumentationOptions
     /// <para><see cref="Activity"/>: the activity being enriched.</para>
     /// <para><see cref="HttpResponseMessage"/> object from which additional information can be extracted to enrich the activity.</para>
     /// </remarks>
-    public Action<Activity, HttpResponseMessage> EnrichWithHttpResponseMessage { get; set; }
+    public Action<Activity, HttpResponseMessage>? EnrichWithHttpResponseMessage { get; set; }
 }

src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs

@@ -2,9 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 using System.Diagnostics;
-#if NET6_0_OR_GREATER
 using System.Diagnostics.CodeAnalysis;
-#endif
 using System.Reflection;
 using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Internal;
@@ -16,7 +14,7 @@ internal sealed class GrpcClientDiagnosticListener : ListenerHandler
 {
     internal static readonly Assembly Assembly = typeof(GrpcClientDiagnosticListener).Assembly;
     internal static readonly AssemblyName AssemblyName = Assembly.GetName();
-    internal static readonly string ActivitySourceName = AssemblyName.Name;
+    internal static readonly string ActivitySourceName = AssemblyName.Name!;
     internal static readonly string Version = Assembly.GetPackageVersion();
     internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version);
 
@@ -34,26 +32,27 @@ public GrpcClientDiagnosticListener(GrpcClientTraceInstrumentationOptions option
         this.options = options;
     }
 
-    public override void OnEventWritten(string name, object payload)
+    public override void OnEventWritten(string name, object? payload)
     {
+        var activity = Activity.Current!;
         switch (name)
         {
             case OnStartEvent:
                 {
-                    this.OnStartActivity(Activity.Current, payload);
+                    this.OnStartActivity(activity, payload);
                 }
 
                 break;
             case OnStopEvent:
                 {
-                    this.OnStopActivity(Activity.Current, payload);
+                    this.OnStopActivity(activity, payload);
                 }
 
                 break;
         }
     }
 
-    public void OnStartActivity(Activity activity, object payload)
+    public void OnStartActivity(Activity activity, object? payload)
     {
         // The overall flow of what GrpcClient library does is as below:
         // Activity.Start()
@@ -71,7 +70,7 @@ public void OnStartActivity(Activity activity, object payload)
         }
 
         // Ensure context propagation irrespective of sampling decision
-        if (!TryFetchRequest(payload, out HttpRequestMessage request))
+        if (!TryFetchRequest(payload, out HttpRequestMessage? request))
         {
             GrpcInstrumentationEventSource.Log.NullPayload(nameof(GrpcClientDiagnosticListener), nameof(this.OnStartActivity));
             return;
@@ -113,31 +112,39 @@ public void OnStartActivity(Activity activity, object payload)
 
             var grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity);
 
-            activity.DisplayName = grpcMethod?.Trim('/');
-
-            activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc);
-
-            if (GrpcTagHelper.TryParseRpcServiceAndRpcMethod(grpcMethod, out var rpcService, out var rpcMethod))
+            if (grpcMethod != null)
             {
-                activity.SetTag(SemanticConventions.AttributeRpcService, rpcService);
-                activity.SetTag(SemanticConventions.AttributeRpcMethod, rpcMethod);
+                activity.DisplayName = grpcMethod.Trim('/');
 
-                // Remove the grpc.method tag added by the gRPC .NET library
-                activity.SetTag(GrpcTagHelper.GrpcMethodTagName, null);
+                if (GrpcTagHelper.TryParseRpcServiceAndRpcMethod(grpcMethod, out var rpcService, out var rpcMethod))
+                {
+                    activity.SetTag(SemanticConventions.AttributeRpcService, rpcService);
+                    activity.SetTag(SemanticConventions.AttributeRpcMethod, rpcMethod);
+
+                    // Remove the grpc.method tag added by the gRPC .NET library
+                    activity.SetTag(GrpcTagHelper.GrpcMethodTagName, null);
+                }
             }
 
-            var uriHostNameType = Uri.CheckHostName(request.RequestUri.Host);
+            activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc);
 
-            if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6)
-            {
-                activity.SetTag(SemanticConventions.AttributeServerSocketAddress, request.RequestUri.Host);
-            }
-            else
+            var requestUri = request.RequestUri;
+
+            if (requestUri != null)
             {
-                activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host);
-            }
+                var uriHostNameType = Uri.CheckHostName(requestUri.Host);
 
-            activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port);
+                if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6)
+                {
+                    activity.SetTag(SemanticConventions.AttributeServerSocketAddress, requestUri.Host);
+                }
+                else
+                {
+                    activity.SetTag(SemanticConventions.AttributeServerAddress, requestUri.Host);
+                }
+
+                activity.SetTag(SemanticConventions.AttributeServerPort, requestUri.Port);
+            }
 
             try
             {
@@ -154,11 +161,11 @@ public void OnStartActivity(Activity activity, object payload)
 #if NET6_0_OR_GREATER
         [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top level properties are preserved")]
 #endif
-        static bool TryFetchRequest(object payload, out HttpRequestMessage request)
+        static bool TryFetchRequest(object? payload, [NotNullWhen(true)] out HttpRequestMessage? request)
             => StartRequestFetcher.TryFetch(payload, out request) && request != null;
     }
 
-    public void OnStopActivity(Activity activity, object payload)
+    public void OnStopActivity(Activity activity, object? payload)
     {
         if (activity.IsAllDataRequested)
         {
@@ -177,7 +184,7 @@ public void OnStopActivity(Activity activity, object payload)
             // Remove the grpc.status_code tag added by the gRPC .NET library
             activity.SetTag(GrpcTagHelper.GrpcStatusCodeTagName, null);
 
-            if (TryFetchResponse(payload, out HttpResponseMessage response))
+            if (TryFetchResponse(payload, out HttpResponseMessage? response))
             {
                 try
                 {
@@ -195,7 +202,7 @@ public void OnStopActivity(Activity activity, object payload)
 #if NET6_0_OR_GREATER
         [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "The event source guarantees that top level properties are preserved")]
 #endif
-        static bool TryFetchResponse(object payload, out HttpResponseMessage response)
+        static bool TryFetchResponse(object? payload, [NotNullWhen(true)] out HttpResponseMessage? response)
             => StopResponseFetcher.TryFetch(payload, out response) && response != null;
     }
 }

src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj

@@ -7,8 +7,6 @@
     <MinVerTagPrefix>Instrumentation.GrpcNetClient-</MinVerTagPrefix>
 
     <ImplicitUsings>enable</ImplicitUsings>
-    <!-- this is temporary. will remove in future PR. -->
-    <Nullable>disable</Nullable>
   </PropertyGroup>
 
   <!--Do not run Package Baseline Validation as this package has never released a stable version.

src/OpenTelemetry.Instrumentation.GrpcNetClient/TracerProviderBuilderExtensions.cs

@@ -31,7 +31,7 @@ public static TracerProviderBuilder AddGrpcClientInstrumentation(this TracerProv
     /// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
     public static TracerProviderBuilder AddGrpcClientInstrumentation(
         this TracerProviderBuilder builder,
-        Action<GrpcClientTraceInstrumentationOptions> configure)
+        Action<GrpcClientTraceInstrumentationOptions>? configure)
         => AddGrpcClientInstrumentation(builder, name: null, configure);
 
     /// <summary>
@@ -43,8 +43,8 @@ public static TracerProviderBuilder AddGrpcClientInstrumentation(
     /// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
     public static TracerProviderBuilder AddGrpcClientInstrumentation(
         this TracerProviderBuilder builder,
-        string name,
-        Action<GrpcClientTraceInstrumentationOptions> configure)
+        string? name,
+        Action<GrpcClientTraceInstrumentationOptions>? configure)
     {
         Guard.ThrowIfNull(builder);
 

test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcServer.cs

@@ -15,7 +15,7 @@ public class GrpcServer<TService> : IDisposable
 {
     private static readonly Random GlobalRandom = new();
 
-    private readonly IHost host;
+    private readonly IHost? host;
 
     public GrpcServer()
     {
@@ -38,7 +38,7 @@ public GrpcServer()
             catch (IOException)
             {
                 retryCount--;
-                this.host.Dispose();
+                this.host?.Dispose();
             }
         }
     }
@@ -47,8 +47,8 @@ public GrpcServer()
 
     public void Dispose()
     {
-        this.host.StopAsync(TimeSpan.FromSeconds(5)).GetAwaiter().GetResult();
-        this.host.Dispose();
+        this.host?.StopAsync(TimeSpan.FromSeconds(5)).GetAwaiter().GetResult();
+        this.host?.Dispose();
         GC.SuppressFinalize(this);
     }
 

test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTestHelpers/ClientTestHelpers.cs

@@ -12,7 +12,7 @@ namespace OpenTelemetry.Instrumentation.Grpc.Tests.GrpcTestHelpers;
 
 internal static class ClientTestHelpers
 {
-    public static HttpClient CreateTestClient(Func<HttpRequestMessage, Task<HttpResponseMessage>> sendAsync, Uri baseAddress = null)
+    public static HttpClient CreateTestClient(Func<HttpRequestMessage, Task<HttpResponseMessage>> sendAsync, Uri? baseAddress = null)
     {
         var handler = TestHttpMessageHandler.Create(sendAsync);
         var httpClient = new HttpClient(handler);
@@ -21,13 +21,13 @@ public static HttpClient CreateTestClient(Func<HttpRequestMessage, Task<HttpResp
         return httpClient;
     }
 
-    public static Task<StreamContent> CreateResponseContent<TResponse>(TResponse response, ICompressionProvider compressionProvider = null)
+    public static Task<StreamContent> CreateResponseContent<TResponse>(TResponse response, ICompressionProvider? compressionProvider = null)
         where TResponse : IMessage<TResponse>
     {
         return CreateResponseContentCore(new[] { response }, compressionProvider);
     }
 
-    public static async Task WriteResponseAsync<TResponse>(Stream ms, TResponse response, ICompressionProvider compressionProvider)
+    public static async Task WriteResponseAsync<TResponse>(Stream ms, TResponse response, ICompressionProvider? compressionProvider)
         where TResponse : IMessage<TResponse>
     {
         var compress = false;
@@ -59,7 +59,7 @@ public static async Task WriteResponseAsync<TResponse>(Stream ms, TResponse resp
 #endif
     }
 
-    private static async Task<StreamContent> CreateResponseContentCore<TResponse>(TResponse[] responses, ICompressionProvider compressionProvider)
+    private static async Task<StreamContent> CreateResponseContentCore<TResponse>(TResponse[] responses, ICompressionProvider? compressionProvider)
         where TResponse : IMessage<TResponse>
     {
         var ms = new MemoryStream();

test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.client.cs

@@ -428,8 +428,8 @@ public void AddGrpcClientInstrumentationNamedOptionsSupported()
     [Fact]
     public void Grpc_BadArgs()
     {
-        TracerProviderBuilder builder = null;
-        Assert.Throws<ArgumentNullException>(() => builder.AddGrpcClientInstrumentation());
+        TracerProviderBuilder? builder = null;
+        Assert.Throws<ArgumentNullException>(() => builder!.AddGrpcClientInstrumentation());
     }
 
     private static void ValidateGrpcActivity(Activity activityToValidate)