Remove tags added by gRPC library in favor of OpenTelemetry tags (#1260)

* Remove grpc.method and grpc.status_code tags

* Remove unused usings

* Remove TODO that is probably a WONTDO

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
This commit is contained in:
Alan West 2020-09-14 18:01:40 -07:00 committed by GitHub
parent 768eaa3c96
commit a5a2eb0a8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 33 additions and 10 deletions

View File

@ -2,6 +2,12 @@
## Unreleased
* For gRPC invocations, the `grpc.method` and `grpc.status_code` attributes
added by the library are removed from the span. The information from these
attributes is contained in other attributes that follow the conventions of
OpenTelemetry.
([#1260](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1260)).
## 0.5.0-beta.2
Released 2020-08-28

View File

@ -253,21 +253,23 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
private static void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context)
{
// TODO: Should the leading slash be trimmed? Spec seems to suggest no leading slash: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/rpc.md#span-name
// Client instrumentation is trimming the leading slash. Whatever we decide here, should we apply the same to the client side?
// activity.DisplayName = grpcMethod?.Trim('/');
activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc);
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);
}
activity.SetTag(SemanticConventions.AttributeNetPeerIp, context.Connection.RemoteIpAddress.ToString());
activity.SetTag(SemanticConventions.AttributeNetPeerPort, context.Connection.RemotePort);
activity.SetStatus(GrpcTagHelper.GetGrpcStatusCodeFromActivity(activity));
// Remove the grpc.method tag added by the gRPC .NET library
activity.SetTag(GrpcTagHelper.GrpcStatusCodeTagName, null);
}
}
}

View File

@ -2,6 +2,12 @@
## Unreleased
* The `grpc.method` and `grpc.status_code` attributes
added by the library are removed from the span. The information from these
attributes is contained in other attributes that follow the conventions of
OpenTelemetry.
([#1260](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1260)).
## 0.5.0-beta.2
Released 2020-08-28

View File

@ -15,7 +15,6 @@
// </copyright>
using System;
using System.Diagnostics;
using System.Globalization;
using System.Net.Http;
using OpenTelemetry.Trace;
@ -69,6 +68,9 @@ namespace OpenTelemetry.Instrumentation.GrpcNetClient.Implementation
{
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);
@ -90,6 +92,9 @@ namespace OpenTelemetry.Instrumentation.GrpcNetClient.Implementation
if (activity.IsAllDataRequested)
{
activity.SetStatus(GrpcTagHelper.GetGrpcStatusCodeFromActivity(activity));
// Remove the grpc.status_code tag added by the gRPC .NET library
activity.SetTag(GrpcTagHelper.GrpcStatusCodeTagName, null);
}
this.activitySource.Stop(activity);

View File

@ -15,13 +15,13 @@
// </copyright>
using System;
using System.Diagnostics;
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using Greet;
using Grpc.Net.Client;
using Moq;
using OpenTelemetry.Instrumentation.Grpc.Tests.Services;
using OpenTelemetry.Instrumentation.GrpcNetClient;
using OpenTelemetry.Instrumentation.GrpcNetClient.Implementation;
using OpenTelemetry.Trace;
using Xunit;
@ -85,6 +85,10 @@ namespace OpenTelemetry.Instrumentation.Grpc.Tests
Assert.Equal(uri.Port, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort));
Assert.Equal(Status.Ok, activity.GetStatus());
Assert.Equal(expectedResource, activity.GetResource());
// Tags added by the library then removed from the instrumentation
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName));
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName));
}
[Fact]

View File

@ -15,13 +15,13 @@
// </copyright>
using System;
using System.Diagnostics;
using System.Linq;
using System.Net;
using System.Threading;
using Greet;
using Grpc.Net.Client;
using Moq;
using OpenTelemetry.Instrumentation.Grpc.Tests.Services;
using OpenTelemetry.Instrumentation.GrpcNetClient;
using OpenTelemetry.Trace;
using Xunit;
@ -67,9 +67,9 @@ namespace OpenTelemetry.Instrumentation.Grpc.Tests
Assert.Equal($"http://localhost:{this.fixture.Port}/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpUrl));
Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string);
// This attribute is added by the gRPC for .NET library. There is a discussion of having the OpenTelemetry instrumentation remove it.
// See: https://github.com/open-telemetry/opentelemetry-dotnet/issues/482#issuecomment-655753756
Assert.Equal($"0", activity.GetTagValue("grpc.status_code"));
// Tags added by the library then removed from the instrumentation
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName));
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName));
}
private static void WaitForProcessorInvocations(Mock<ActivityProcessor> spanProcessor, int invocationCount)