diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs index 38e8f552a34..d38eb04ce42 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentationOptions.cs @@ -61,5 +61,15 @@ public class AspNetCoreInstrumentationOptions /// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/exceptions.md. /// public bool RecordException { get; set; } + +#if NETSTANDARD2_1 + /// + /// Gets or sets a value indicating whether RPC attributes are added to an Activity when using Grpc.AspNetCore. Default is true. + /// + /// + /// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/rpc.md. + /// + public bool EnableGrpcAspNetCoreSupport { get; set; } = true; +#endif } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index b82e386b2df..563d362f36e 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -5,6 +5,11 @@ * Record `Exception` in AspNetCore instrumentation based on `RecordException` in `AspNetCoreInstrumentationOptions` ([#1408](https://github.com/open-telemetry/opentelemetry-dotnet/issues/1408)) +* Added configuration option `EnableGrpcAspNetCoreSupport` to enable or disable + support for adding OpenTelemetry RPC attributes when using + [Grpc.AspNetCore](https://www.nuget.org/packages/Grpc.AspNetCore/). + This option is enabled by default. + ([#1423](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1423)) ## 0.7.0-beta.1 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index ed695fa405c..e1b44a1dc5b 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -18,6 +18,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Runtime.CompilerServices; using System.Text; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -165,15 +166,18 @@ public override void OnStopActivity(Activity activity, object payload) activity.SetTag(SemanticConventions.AttributeHttpStatusCode, response.StatusCode); - if (TryGetGrpcMethod(activity, out var grpcMethod)) +#if NETSTANDARD2_1 + if (this.options.EnableGrpcAspNetCoreSupport && TryGetGrpcMethod(activity, out var grpcMethod)) { AddGrpcAttributes(activity, grpcMethod, context); } else { - Status status = SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode); - activity.SetStatus(status); + SetStatusFromHttpStatusCode(activity, response.StatusCode); } +#else + SetStatusFromHttpStatusCode(activity, response.StatusCode); +#endif } if (activity.OperationName.Equals(ActivityNameByHttpInListener, StringComparison.Ordinal)) @@ -289,12 +293,22 @@ private static string GetUri(HttpRequest request) return builder.ToString(); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void SetStatusFromHttpStatusCode(Activity activity, int statusCode) + { + var status = SpanHelper.ResolveSpanStatusForHttpStatusCode(statusCode); + activity.SetStatus(status); + } + +#if NETSTANDARD2_1 + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static bool TryGetGrpcMethod(Activity activity, out string grpcMethod) { grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity); return !string.IsNullOrEmpty(grpcMethod); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context) { activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc); @@ -306,11 +320,19 @@ private static void AddGrpcAttributes(Activity activity, string grpcMethod, Http // Remove the grpc.method tag added by the gRPC .NET library activity.SetTag(GrpcTagHelper.GrpcMethodTagName, null); + + // TODO: The grpc.status_code attribute added by the library is not currently + // removed because the tracing spec for span status is no longer based on + // gRPC status codes. Ultimately, the grpc.status_code tag should be replaced + // by whatever semantic convention is settled on in the RPC spec. + // See: https://github.com/open-telemetry/opentelemetry-dotnet/issues/1345 + // activity.SetTag(GrpcTagHelper.GrpcStatusCodeTagName, null); } activity.SetTag(SemanticConventions.AttributeNetPeerIp, context.Connection.RemoteIpAddress.ToString()); activity.SetTag(SemanticConventions.AttributeNetPeerPort, context.Connection.RemotePort); activity.SetStatus(GrpcTagHelper.GetGrpcStatusCodeFromActivity(activity)); } +#endif } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj b/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj index 61c8d73f6d2..698607674c7 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj @@ -1,6 +1,6 @@  - netstandard2.0 + netstandard2.0;netstandard2.1 ASP.NET Core instrumentation for OpenTelemetry .NET $(PackageTags);distributed-tracing;AspNetCore diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs index cf840f2706a..dc3bd1d8284 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs @@ -38,13 +38,29 @@ public GrpcTests() this.server = new GrpcServer(); } - [Fact] - public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes() + [Theory] + [InlineData(null)] + [InlineData(true)] + [InlineData(false)] + public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(bool? enableGrpcAspNetCoreSupport) { var processor = new Mock>(); - using var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddAspNetCoreInstrumentation() + 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(); @@ -62,11 +78,30 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes() invo.Method.Name == "OnEnd" && (invo.Arguments[0] as Activity).OperationName == "Microsoft.AspNetCore.Hosting.HttpRequestIn").Arguments[0]; Assert.Equal(ActivityKind.Server, activity.Kind); - 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)); + + 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)); + + // Tags added by the library then removed from the instrumentation + // TODO: The grpc.status_code attribute added by the library is not currently + // removed because the tracing spec for span status is no longer based on + // gRPC status codes. Ultimately, the grpc.status_code tag should be replaced + // by whatever semantic convention is settled on in the RPC spec. + // See: https://github.com/open-telemetry/opentelemetry-dotnet/issues/1345 + Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); + Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); + } + 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. @@ -75,10 +110,6 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes() 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); - - // Tags added by the library then removed from the instrumentation - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); } public void Dispose() diff --git a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/Dockerfile b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/Dockerfile index 11e5a5da502..bce6ebb0d44 100644 --- a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/Dockerfile +++ b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/Dockerfile @@ -9,7 +9,7 @@ WORKDIR /w3c RUN apt-get update && apt-get install -y git RUN git clone https://github.com/w3c/trace-context.git -FROM mcr.microsoft.com/dotnet/core/sdk:${SDK_VERSION} AS build +FROM mcr.microsoft.com/dotnet/core/sdk:3.1 AS build ARG PUBLISH_CONFIGURATION=Release ARG PUBLISH_FRAMEWORK=netcoreapp3.1 WORKDIR /repo