Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update Semantic Conventions: grpc attributes in aspnetcore #4660

Merged
merged 9 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
variable `OTEL_SEMCONV_STABILITY_OPT_IN`.
([#4537](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4537))
([#4606](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4606))
([#4660](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4660))

* Fixed an issue affecting NET 7.0+. If custom propagation is being used
and tags are added to an Activity during sampling then that Activity would be dropped.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,20 +490,27 @@ private void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext
activity.DisplayName = grpcMethod.TrimStart('/');

activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc);
if (context.Connection.RemoteIpAddress != null)
{
// TODO: This attribute was changed in v1.13.0 https://github.com/open-telemetry/opentelemetry-specification/pull/2614
activity.SetTag(SemanticConventions.AttributeNetPeerIp, context.Connection.RemoteIpAddress.ToString());
}

if (this.emitOldAttributes)
{
if (context.Connection.RemoteIpAddress != null)
{
// TODO: This attribute was changed in v1.13.0 https://github.com/open-telemetry/opentelemetry-specification/pull/2614
activity.SetTag(SemanticConventions.AttributeNetPeerIp, context.Connection.RemoteIpAddress.ToString());
}

activity.SetTag(SemanticConventions.AttributeNetPeerPort, context.Connection.RemotePort);
}

// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/rpc/rpc-spans.md
if (this.emitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeServerPort, context.Connection.RemotePort);
if (context.Connection.RemoteIpAddress != null)
{
activity.SetTag(SemanticConventions.AttributeClientAddress, context.Connection.RemoteIpAddress.ToString());
}

activity.SetTag(SemanticConventions.AttributeClientPort, context.Connection.RemotePort);
}

bool validConversion = GrpcTagHelper.TryGetGrpcStatusCodeFromActivity(activity, out int status);
Expand Down
5 changes: 4 additions & 1 deletion src/Shared/SemanticConventions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,12 @@ internal static class SemanticConventions
public const string AttributeExceptionMessage = "exception.message";
public const string AttributeExceptionStacktrace = "exception.stacktrace";

// v1.21.0 (unreleased as of this commit)
// v1.21.0
// https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md
// https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/database/database-spans.md
// https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/rpc/rpc-spans.md
public const string AttributeClientAddress = "client.address";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add a comment saying this replaces net.peer.ip in specific cases.

public const string AttributeClientPort = "client.port";
public const string AttributeHttpRequestMethod = "http.request.method"; // replaces: "http.method" (AttributeHttpMethod)
public const string AttributeHttpResponseStatusCode = "http.response.status_code"; // replaces: "http.status_code" (AttributeHttpStatusCode)
public const string AttributeNetworkProtocolVersion = "network.protocol.version"; // replaces: "http.flavor" (AttributeHttpFlavor)
Expand Down
186 changes: 186 additions & 0 deletions test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@
using Greet;
using Grpc.Core;
using Grpc.Net.Client;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Moq;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Instrumentation.Grpc.Services.Tests;
using OpenTelemetry.Instrumentation.GrpcNetClient;
using OpenTelemetry.Trace;
using Xunit;
using static OpenTelemetry.Internal.HttpSemanticConventionHelper;
using Status = OpenTelemetry.Trace.Status;

namespace OpenTelemetry.Instrumentation.Grpc.Tests
Expand Down Expand Up @@ -114,6 +117,189 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(bool? enableGrpcA
Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string);
}

// Tests for v1.21.0 Semantic Conventions for database client calls.
// see the spec https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md
// This test emits the new attributes.
// This test method can replace the other (old) test method when this library is GA.
[Theory]
[InlineData(null)]
utpilla marked this conversation as resolved.
Show resolved Hide resolved
[InlineData(true)]
[InlineData(false)]
public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes_New(bool? enableGrpcAspNetCoreSupport)
{
var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string> { [SemanticConventionOptInKeyName] = "http" })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need not be done in this PR but the tests shouldn't be using the const string variable SemanticConventionOptInKeyName directly from src file. Our tests. would not complain about it when someone accidentally changes the value for it in the src file.

.Build();

var exportedItems = new List<Activity>();
var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder()
.ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration));

if (enableGrpcAspNetCoreSupport.HasValue)
{
tracerProviderBuilder.AddAspNetCoreInstrumentation(options =>
{
options.EnableGrpcAspNetCoreSupport = enableGrpcAspNetCoreSupport.Value;
});
}
else
{
tracerProviderBuilder.AddAspNetCoreInstrumentation();
}

using var tracerProvider = tracerProviderBuilder
.AddInMemoryExporter(exportedItems)
.Build();

var clientLoopbackAddresses = new[] { IPAddress.Loopback.ToString(), IPAddress.IPv6Loopback.ToString() };
var uri = new Uri($"http://localhost:{this.server.Port}");

using var channel = GrpcChannel.ForAddress(uri);
var client = new Greeter.GreeterClient(channel);
var returnMsg = client.SayHello(new HelloRequest()).Message;
Assert.False(string.IsNullOrEmpty(returnMsg));

WaitForExporterToReceiveItems(exportedItems, 1);
Assert.Single(exportedItems);
var activity = exportedItems[0];

Assert.Equal(ActivityKind.Server, activity.Kind);

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.AttributeClientAddress), clientLoopbackAddresses);
Assert.NotEqual(0, activity.GetTagValue(SemanticConventions.AttributeClientPort));
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName));
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName));
Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode));
}
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.
Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeServerAddress));
Assert.Equal(this.server.Port, activity.GetTagValue(SemanticConventions.AttributeServerPort));
Assert.Equal("POST", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod));
Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeUrlScheme));
Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeUrlPath));
Assert.Equal("2.0", activity.GetTagValue(SemanticConventions.AttributeNetworkProtocolVersion));
Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeUserAgentOriginal) as string);
}

// Tests for v1.21.0 Semantic Conventions for database client calls.
// see the spec https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md
// This test emits both the new and older attributes.
// This test method can be deleted when this library is GA.
[Theory]
[InlineData(null)]
[InlineData(true)]
[InlineData(false)]
public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes_Dupe(bool? enableGrpcAspNetCoreSupport)
{
var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string> { [SemanticConventionOptInKeyName] = "http/dup" })
.Build();

var exportedItems = new List<Activity>();
var tracerProviderBuilder = Sdk.CreateTracerProviderBuilder()
.ConfigureServices(services => services.AddSingleton<IConfiguration>(configuration));

if (enableGrpcAspNetCoreSupport.HasValue)
{
tracerProviderBuilder.AddAspNetCoreInstrumentation(options =>
{
options.EnableGrpcAspNetCoreSupport = enableGrpcAspNetCoreSupport.Value;
});
}
else
{
tracerProviderBuilder.AddAspNetCoreInstrumentation();
}

using var tracerProvider = tracerProviderBuilder
.AddInMemoryExporter(exportedItems)
.Build();

var clientLoopbackAddresses = new[] { IPAddress.Loopback.ToString(), IPAddress.IPv6Loopback.ToString() };
var uri = new Uri($"http://localhost:{this.server.Port}");

using var channel = GrpcChannel.ForAddress(uri);
var client = new Greeter.GreeterClient(channel);
var returnMsg = client.SayHello(new HelloRequest()).Message;
Assert.False(string.IsNullOrEmpty(returnMsg));

WaitForExporterToReceiveItems(exportedItems, 1);
Assert.Single(exportedItems);
var activity = exportedItems[0];

Assert.Equal(ActivityKind.Server, activity.Kind);

// OLD
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));
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName));
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName));
Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode));
}
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.
Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeNetHostName));
Assert.Equal(this.server.Port, activity.GetTagValue(SemanticConventions.AttributeNetHostPort));
Assert.Equal("POST", activity.GetTagValue(SemanticConventions.AttributeHttpMethod));
Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpTarget));
Assert.Equal($"http://localhost:{this.server.Port}/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeHttpUrl));
Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeHttpUserAgent) as string);

// NEW
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.AttributeClientAddress), clientLoopbackAddresses);
Assert.NotEqual(0, activity.GetTagValue(SemanticConventions.AttributeClientPort));
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName));
Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName));
Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode));
}
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.
Assert.Equal("localhost", activity.GetTagValue(SemanticConventions.AttributeServerAddress));
Assert.Equal(this.server.Port, activity.GetTagValue(SemanticConventions.AttributeServerPort));
Assert.Equal("POST", activity.GetTagValue(SemanticConventions.AttributeHttpRequestMethod));
Assert.Equal("http", activity.GetTagValue(SemanticConventions.AttributeUrlScheme));
Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeUrlPath));
Assert.Equal("2.0", activity.GetTagValue(SemanticConventions.AttributeNetworkProtocolVersion));
Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeUserAgentOriginal) as string);
}

#if NET6_0_OR_GREATER
[Theory(Skip = "Skipping for .NET 6 and higher due to bug #3023")]
#endif
Expand Down