[Instrumentation.GrpcCore] Fix analysis warnings (#963)

Fix/suppress code analysis warnings in OpenTelemetry.Instrumentation.GrpcCore.
Contributes to #950.

examples/grpc.core/Examples.GrpcCore.AspNetCore/Startup.cs

@@ -110,7 +110,7 @@ protected override Task ExecuteAsync(CancellationToken stoppingToken)
                     tcs.SetResult(true);
                 });
 
-            return tcs.Task.ContinueWith(antecedent => tokenRegistration.Dispose());
+            return tcs.Task.ContinueWith(antecedent => tokenRegistration.Dispose(), stoppingToken);
         }
     }
 }

src/OpenTelemetry.Instrumentation.GrpcCore/ClientTracingInterceptor.cs

@@ -17,6 +17,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Threading.Tasks;
 using Grpc.Core;
 using Grpc.Core.Interceptors;
 using OpenTelemetry.Context.Propagation;
@@ -52,6 +53,9 @@ public override TResponse BlockingUnaryCall<TRequest, TResponse>(
         ClientInterceptorContext<TRequest, TResponse> context,
         BlockingUnaryCallContinuation<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         ClientRpcScope<TRequest, TResponse> rpcScope = null;
 
         try
@@ -81,11 +85,16 @@ public override AsyncUnaryCall<TResponse> AsyncUnaryCall<TRequest, TResponse>(
         ClientInterceptorContext<TRequest, TResponse> context,
         AsyncUnaryCallContinuation<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         ClientRpcScope<TRequest, TResponse> rpcScope = null;
 
         try
         {
+#pragma warning  disable CA2000
             rpcScope = new ClientRpcScope<TRequest, TResponse>(context, this.options);
+#pragma warning restore CA2000
             rpcScope.RecordRequest(request);
             var responseContinuation = continuation(request, rpcScope.Context);
             var responseAsync = responseContinuation.ResponseAsync.ContinueWith(
@@ -103,7 +112,8 @@ public override AsyncUnaryCall<TResponse> AsyncUnaryCall<TRequest, TResponse>(
                         rpcScope.CompleteWithException(ex.InnerException);
                         throw ex.InnerException;
                     }
-                });
+                },
+                TaskScheduler.Current);
 
             return new AsyncUnaryCall<TResponse>(
                 responseAsync,
@@ -128,11 +138,16 @@ public override AsyncClientStreamingCall<TRequest, TResponse> AsyncClientStreami
         ClientInterceptorContext<TRequest, TResponse> context,
         AsyncClientStreamingCallContinuation<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         ClientRpcScope<TRequest, TResponse> rpcScope = null;
 
         try
         {
+#pragma warning disable CA2000
             rpcScope = new ClientRpcScope<TRequest, TResponse>(context, this.options);
+#pragma warning restore CA2000
             var responseContinuation = continuation(rpcScope.Context);
             var clientRequestStreamProxy = new ClientStreamWriterProxy<TRequest>(
                 responseContinuation.RequestStream,
@@ -154,7 +169,8 @@ public override AsyncClientStreamingCall<TRequest, TResponse> AsyncClientStreami
                         rpcScope.CompleteWithException(ex.InnerException);
                         throw ex.InnerException;
                     }
-                });
+                },
+                TaskScheduler.Current);
 
             return new AsyncClientStreamingCall<TRequest, TResponse>(
                 clientRequestStreamProxy,
@@ -181,11 +197,16 @@ public override AsyncServerStreamingCall<TResponse> AsyncServerStreamingCall<TRe
         ClientInterceptorContext<TRequest, TResponse> context,
         AsyncServerStreamingCallContinuation<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         ClientRpcScope<TRequest, TResponse> rpcScope = null;
 
         try
         {
+#pragma warning disable CA2000
             rpcScope = new ClientRpcScope<TRequest, TResponse>(context, this.options);
+#pragma warning restore CA2000
             rpcScope.RecordRequest(request);
             var responseContinuation = continuation(request, rpcScope.Context);
 
@@ -218,11 +239,16 @@ public override AsyncDuplexStreamingCall<TRequest, TResponse> AsyncDuplexStreami
         ClientInterceptorContext<TRequest, TResponse> context,
         AsyncDuplexStreamingCallContinuation<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         ClientRpcScope<TRequest, TResponse> rpcScope = null;
 
         try
         {
+#pragma warning disable CA2000
             rpcScope = new ClientRpcScope<TRequest, TResponse>(context, this.options);
+#pragma warning restore CA2000
             var responseContinuation = continuation(rpcScope.Context);
 
             var requestStreamProxy = new ClientStreamWriterProxy<TRequest>(

src/OpenTelemetry.Instrumentation.GrpcCore/ClientTracingInterceptorOptions.cs

@@ -27,7 +27,7 @@ public class ClientTracingInterceptorOptions
     /// <summary>
     /// Gets or sets a value indicating whether or not to record individual message events.
     /// </summary>
-    public bool RecordMessageEvents { get; set; } = false;
+    public bool RecordMessageEvents { get; set; }
 
     /// <summary>
     /// Gets the propagator.

src/OpenTelemetry.Instrumentation.GrpcCore/RpcScope.cs

@@ -46,7 +46,7 @@ internal abstract class RpcScope<TRequest, TResponse> : IDisposable
     /// <summary>
     /// The complete flag.
     /// </summary>
-    private long complete = 0;
+    private long complete;
 
     /// <summary>
     /// The request message counter.

src/OpenTelemetry.Instrumentation.GrpcCore/ServerTracingInterceptor.cs

@@ -54,6 +54,9 @@ public override async Task<TResponse> UnaryServerHandler<TRequest, TResponse>(
         ServerCallContext context,
         UnaryServerMethod<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         using var rpcScope = new ServerRpcScope<TRequest, TResponse>(context, this.options);
 
         try
@@ -77,6 +80,9 @@ public override async Task<TResponse> ClientStreamingServerHandler<TRequest, TRe
         ServerCallContext context,
         ClientStreamingServerMethod<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         using var rpcScope = new ServerRpcScope<TRequest, TResponse>(context, this.options);
 
         try
@@ -104,6 +110,9 @@ public override async Task ServerStreamingServerHandler<TRequest, TResponse>(
         ServerCallContext context,
         ServerStreamingServerMethod<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         using var rpcScope = new ServerRpcScope<TRequest, TResponse>(context, this.options);
 
         try
@@ -127,6 +136,9 @@ public override async Task ServerStreamingServerHandler<TRequest, TResponse>(
     /// <inheritdoc/>
     public override async Task DuplexStreamingServerHandler<TRequest, TResponse>(IAsyncStreamReader<TRequest> requestStream, IServerStreamWriter<TResponse> responseStream, ServerCallContext context, DuplexStreamingServerMethod<TRequest, TResponse> continuation)
     {
+        Guard.ThrowIfNull(context);
+        Guard.ThrowIfNull(continuation);
+
         using var rpcScope = new ServerRpcScope<TRequest, TResponse>(context, this.options);
 
         try

src/OpenTelemetry.Instrumentation.GrpcCore/ServerTracingInterceptorOptions.cs

@@ -27,7 +27,7 @@ public class ServerTracingInterceptorOptions
     /// <summary>
     /// Gets or sets a value indicating whether or not to record individual message events.
     /// </summary>
-    public bool RecordMessageEvents { get; set; } = false;
+    public bool RecordMessageEvents { get; set; }
 
     /// <summary>
     /// Gets the propagator.

test/OpenTelemetry.Instrumentation.GrpcCore.Tests/FoobarService.cs

@@ -33,7 +33,17 @@ internal class FoobarService : Foobar.FoobarBase
     /// <summary>
     /// Default traceparent header value with the sampling bit on.
     /// </summary>
-    internal static readonly string DefaultTraceparentWithSampling = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01";
+    internal const string DefaultTraceparentWithSampling = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01";
+
+    /// <summary>
+    /// The request header fail with status code.
+    /// </summary>
+    internal const string RequestHeaderFailWithStatusCode = "failurestatuscode";
+
+    /// <summary>
+    /// The request header error description.
+    /// </summary>
+    internal const string RequestHeaderErrorDescription = "failuredescription";
 
     /// <summary>
     /// The default parent from a traceparent header.
@@ -60,16 +70,6 @@ internal class FoobarService : Foobar.FoobarBase
     /// </summary>
     internal static readonly int DefaultResponseMessageSize = ((IMessage)DefaultResponseMessage).CalculateSize();
 
-    /// <summary>
-    /// The request header fail with status code.
-    /// </summary>
-    internal static readonly string RequestHeaderFailWithStatusCode = "failurestatuscode";
-
-    /// <summary>
-    /// The request header error description.
-    /// </summary>
-    internal static readonly string RequestHeaderErrorDescription = "failuredescription";
-
     /// <summary>
     /// Starts the specified service.
     /// </summary>

test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs

@@ -35,7 +35,7 @@ public class GrpcCoreClientInterceptorTests
     /// <summary>
     /// A bogus server uri.
     /// </summary>
-    private static readonly string BogusServerUri = "dns:i.dont.exist:77923";
+    private const string BogusServerUri = "dns:i.dont.exist:77923";
 
     /// <summary>
     /// The default metadata func.
@@ -49,7 +49,7 @@ public class GrpcCoreClientInterceptorTests
     [Fact]
     public async Task AsyncUnarySuccess()
     {
-        await this.TestHandlerSuccess(FoobarService.MakeUnaryAsyncRequest, DefaultMetadataFunc()).ConfigureAwait(false);
+        await TestHandlerSuccess(FoobarService.MakeUnaryAsyncRequest, DefaultMetadataFunc()).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -59,7 +59,7 @@ public async Task AsyncUnarySuccess()
     [Fact]
     public async Task AsyncUnaryUnavailable()
     {
-        await this.TestHandlerFailure(
+        await TestHandlerFailure(
             FoobarService.MakeUnaryAsyncRequest,
             StatusCode.Unavailable,
             validateErrorDescription: false,
@@ -73,7 +73,7 @@ await this.TestHandlerFailure(
     [Fact]
     public async Task AsyncUnaryFail()
     {
-        await this.TestHandlerFailure(FoobarService.MakeUnaryAsyncRequest).ConfigureAwait(false);
+        await TestHandlerFailure(FoobarService.MakeUnaryAsyncRequest).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -97,7 +97,7 @@ static void MakeRequest(Foobar.FoobarClient client)
     [Fact]
     public async Task ClientStreamingSuccess()
     {
-        await this.TestHandlerSuccess(FoobarService.MakeClientStreamingRequest, DefaultMetadataFunc()).ConfigureAwait(false);
+        await TestHandlerSuccess(FoobarService.MakeClientStreamingRequest, DefaultMetadataFunc()).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -107,7 +107,7 @@ public async Task ClientStreamingSuccess()
     [Fact]
     public async Task ClientStreamingUnavailable()
     {
-        await this.TestHandlerFailure(
+        await TestHandlerFailure(
             FoobarService.MakeClientStreamingRequest,
             StatusCode.Unavailable,
             validateErrorDescription: false,
@@ -121,7 +121,7 @@ await this.TestHandlerFailure(
     [Fact]
     public async Task ClientStreamingFail()
     {
-        await this.TestHandlerFailure(FoobarService.MakeClientStreamingRequest).ConfigureAwait(false);
+        await TestHandlerFailure(FoobarService.MakeClientStreamingRequest).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -145,7 +145,7 @@ static void MakeRequest(Foobar.FoobarClient client)
     [Fact]
     public async Task ServerStreamingSuccess()
     {
-        await this.TestHandlerSuccess(FoobarService.MakeServerStreamingRequest, DefaultMetadataFunc()).ConfigureAwait(false);
+        await TestHandlerSuccess(FoobarService.MakeServerStreamingRequest, DefaultMetadataFunc()).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -155,7 +155,7 @@ public async Task ServerStreamingSuccess()
     [Fact]
     public async Task ServerStreamingFail()
     {
-        await this.TestHandlerFailure(FoobarService.MakeServerStreamingRequest).ConfigureAwait(false);
+        await TestHandlerFailure(FoobarService.MakeServerStreamingRequest).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -179,7 +179,7 @@ static void MakeRequest(Foobar.FoobarClient client)
     [Fact]
     public async Task DuplexStreamingSuccess()
     {
-        await this.TestHandlerSuccess(FoobarService.MakeDuplexStreamingRequest, DefaultMetadataFunc()).ConfigureAwait(false);
+        await TestHandlerSuccess(FoobarService.MakeDuplexStreamingRequest, DefaultMetadataFunc()).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -189,7 +189,7 @@ public async Task DuplexStreamingSuccess()
     [Fact]
     public async Task DuplexStreamingUnavailable()
     {
-        await this.TestHandlerFailure(
+        await TestHandlerFailure(
             FoobarService.MakeDuplexStreamingRequest,
             StatusCode.Unavailable,
             validateErrorDescription: false,
@@ -203,7 +203,7 @@ await this.TestHandlerFailure(
     [Fact]
     public async Task DuplexStreamingFail()
     {
-        await this.TestHandlerFailure(FoobarService.MakeDuplexStreamingRequest).ConfigureAwait(false);
+        await TestHandlerFailure(FoobarService.MakeDuplexStreamingRequest).ConfigureAwait(false);
     }
 
     /// <summary>
@@ -376,7 +376,7 @@ static void ValidateCommonEventAttributes(ActivityEvent activityEvent)
     /// <param name="clientRequestFunc">The client request function.</param>
     /// <param name="additionalMetadata">The additional metadata, if any.</param>
     /// <returns>A Task.</returns>
-    private async Task TestHandlerSuccess(Func<Foobar.FoobarClient, Metadata, Task> clientRequestFunc, Metadata additionalMetadata)
+    private static async Task TestHandlerSuccess(Func<Foobar.FoobarClient, Metadata, Task> clientRequestFunc, Metadata additionalMetadata)
     {
         var mockPropagator = new Mock<TextMapPropagator>();
         PropagationContext capturedPropagationContext = default;
@@ -482,7 +482,7 @@ private async Task TestHandlerSuccess(Func<Foobar.FoobarClient, Metadata, Task>
     /// <returns>
     /// A Task.
     /// </returns>
-    private async Task TestHandlerFailure(
+    private static async Task TestHandlerFailure(
         Func<Foobar.FoobarClient, Metadata, Task> clientRequestFunc,
         StatusCode statusCode = StatusCode.ResourceExhausted,
         bool validateErrorDescription = true,