diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md index 51ae981b732..56946c1533e 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md @@ -9,6 +9,12 @@ [issue](https://github.com/open-telemetry/opentelemetry-dotnet/issues/5092) for details and workaround. ([#5077](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5077)) +* Removed support for the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable + which toggled the use of the new conventions for the + [server, client, and shared network attributes](https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/general/attributes.md#server-client-and-shared-network-attributes). + Now that this suite of attributes are stable, this instrumentation will only + emit the new attributes. + ([#5259](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5259)) ## 1.6.0-beta.3 diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentationOptions.cs index 4ed15104000..415bf8f0f71 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentationOptions.cs @@ -2,8 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; -using Microsoft.Extensions.Configuration; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.GrpcNetClient; @@ -12,23 +10,6 @@ namespace OpenTelemetry.Instrumentation.GrpcNetClient; /// public class GrpcClientInstrumentationOptions { - internal readonly HttpSemanticConvention HttpSemanticConvention; - - /// - /// Initializes a new instance of the class. - /// - public GrpcClientInstrumentationOptions() - : this(new ConfigurationBuilder().AddEnvironmentVariables().Build()) - { - } - - internal GrpcClientInstrumentationOptions(IConfiguration configuration) - { - Debug.Assert(configuration != null, "configuration was null"); - - this.HttpSemanticConvention = GetSemanticConventionOptIn(configuration); - } - /// /// Gets or sets a value indicating whether down stream instrumentation is suppressed (disabled). /// diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs index 2ce6add8ab6..9ac826921d8 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs @@ -9,7 +9,6 @@ using OpenTelemetry.Context.Propagation; using OpenTelemetry.Instrumentation.Http; using OpenTelemetry.Trace; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.GrpcNetClient.Implementation; @@ -27,17 +26,11 @@ internal sealed class GrpcClientDiagnosticListener : ListenerHandler private static readonly PropertyFetcher StopResponseFetcher = new("Response"); private readonly GrpcClientInstrumentationOptions options; - private readonly bool emitOldAttributes; - private readonly bool emitNewAttributes; public GrpcClientDiagnosticListener(GrpcClientInstrumentationOptions options) : base("Grpc.Net.Client") { this.options = options; - - this.emitOldAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.Old); - - this.emitNewAttributes = this.options.HttpSemanticConvention.HasFlag(HttpSemanticConvention.New); } public override void OnEventWritten(string name, object payload) @@ -133,35 +126,18 @@ public void OnStartActivity(Activity activity, object payload) } var uriHostNameType = Uri.CheckHostName(request.RequestUri.Host); - if (this.emitOldAttributes) - { - if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6) - { - activity.SetTag(SemanticConventions.AttributeNetPeerIp, request.RequestUri.Host); - } - else - { - activity.SetTag(SemanticConventions.AttributeNetPeerName, request.RequestUri.Host); - } - activity.SetTag(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port); + if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6) + { + activity.SetTag(SemanticConventions.AttributeServerSocketAddress, request.RequestUri.Host); } - - // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md - if (this.emitNewAttributes) + else { - if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6) - { - activity.SetTag(SemanticConventions.AttributeServerSocketAddress, request.RequestUri.Host); - } - else - { - activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); - } - - activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port); + activity.SetTag(SemanticConventions.AttributeServerAddress, request.RequestUri.Host); } + activity.SetTag(SemanticConventions.AttributeServerPort, request.RequestUri.Port); + try { this.options.EnrichWithHttpRequestMessage?.Invoke(activity, request); diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj b/src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj index 2881d4e4ef0..b85c018d7ab 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj @@ -13,8 +13,6 @@ - - diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.GrpcNetClient/TracerProviderBuilderExtensions.cs index 3df0b394e2c..567bdb72c14 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/TracerProviderBuilderExtensions.cs @@ -50,15 +50,10 @@ public static TracerProviderBuilder AddGrpcClientInstrumentation( name ??= Options.DefaultName; - builder.ConfigureServices(services => + if (configure != null) { - if (configure != null) - { - services.Configure(name, configure); - } - - services.RegisterOptionsFactory(configuration => new GrpcClientInstrumentationOptions(configuration)); - }); + builder.ConfigureServices(services => services.Configure(name, configure)); + } builder.AddSource(GrpcClientDiagnosticListener.ActivitySourceName); builder.AddLegacySource("Grpc.Net.Client.GrpcOut"); diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs index 2d7227f3482..35fd982a7bd 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs @@ -8,7 +8,6 @@ using Grpc.Core; #endif using Grpc.Net.Client; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; #if !NETFRAMEWORK using OpenTelemetry.Context.Propagation; @@ -91,213 +90,16 @@ public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool sho if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6) { - Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeNetPeerIp)); - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeNetPeerName)); - } - else - { - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeNetPeerIp)); - Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeNetPeerName)); - } - - Assert.Equal(uri.Port, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); - Assert.Equal(Status.Unset, activity.GetStatus()); - - // Tags added by the library then removed from the instrumentation - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); - Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); - - if (shouldEnrich) - { - Assert.True(enrichWithHttpRequestMessageCalled); - Assert.True(enrichWithHttpResponseMessageCalled); - } - } - - [Theory] - [InlineData("http://localhost")] - [InlineData("http://localhost", false)] - [InlineData("http://127.0.0.1")] - [InlineData("http://127.0.0.1", false)] - [InlineData("http://[::1]")] - [InlineData("http://[::1]", false)] - public void GrpcClientCallsAreCollectedSuccessfully_New(string baseAddress, bool shouldEnrich = true) - { - var config = new KeyValuePair[] { new("OTEL_SEMCONV_STABILITY_OPT_IN", "http") }; - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(config) - .Build(); - - bool enrichWithHttpRequestMessageCalled = false; - bool enrichWithHttpResponseMessageCalled = false; - - var uri = new Uri($"{baseAddress}:1234"); - var uriHostNameType = Uri.CheckHostName(uri.Host); - - using var httpClient = ClientTestHelpers.CreateTestClient(async request => - { - var streamContent = await ClientTestHelpers.CreateResponseContent(new HelloReply()); - var response = ResponseUtils.CreateResponse(HttpStatusCode.OK, streamContent, grpcStatusCode: global::Grpc.Core.StatusCode.OK); - response.TrailingHeaders().Add("grpc-message", "value"); - return response; - }); - - var exportedItems = new List(); - - using var parent = new Activity("parent") - .SetIdFormat(ActivityIdFormat.W3C) - .Start(); - - using (Sdk.CreateTracerProviderBuilder() - .SetSampler(new AlwaysOnSampler()) - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddGrpcClientInstrumentation(options => - { - if (shouldEnrich) - { - options.EnrichWithHttpRequestMessage = (activity, httpRequestMessage) => { enrichWithHttpRequestMessageCalled = true; }; - options.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => { enrichWithHttpResponseMessageCalled = true; }; - } - }) - .AddInMemoryExporter(exportedItems) - .Build()) - { - var channel = GrpcChannel.ForAddress(uri, new GrpcChannelOptions - { - HttpClient = httpClient, - }); - var client = new Greeter.GreeterClient(channel); - var rs = client.SayHello(new HelloRequest()); - } - - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - ValidateGrpcActivity(activity); - Assert.Equal(parent.TraceId, activity.Context.TraceId); - Assert.Equal(parent.SpanId, activity.ParentSpanId); - Assert.NotEqual(parent.SpanId, activity.Context.SpanId); - Assert.NotEqual(default, activity.Context.SpanId); - - Assert.Equal($"greet.Greeter/SayHello", activity.DisplayName); - Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem)); - Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService)); - Assert.Equal("SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); - - if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6) - { - Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeServerSocketAddress)); - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - } - else - { - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeServerSocketAddress)); - Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); - } - - Assert.Equal(uri.Port, activity.GetTagValue(SemanticConventions.AttributeServerPort)); - Assert.Equal(Status.Unset, activity.GetStatus()); - - // Tags added by the library then removed from the instrumentation - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); - Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); - Assert.Equal(0, activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); - - if (shouldEnrich) - { - Assert.True(enrichWithHttpRequestMessageCalled); - Assert.True(enrichWithHttpResponseMessageCalled); - } - } - - [Theory] - [InlineData("http://localhost")] - [InlineData("http://localhost", false)] - [InlineData("http://127.0.0.1")] - [InlineData("http://127.0.0.1", false)] - [InlineData("http://[::1]")] - [InlineData("http://[::1]", false)] - public void GrpcClientCallsAreCollectedSuccessfully_Dupe(string baseAddress, bool shouldEnrich = true) - { - var config = new KeyValuePair[] { new("OTEL_SEMCONV_STABILITY_OPT_IN", "http/dup") }; - var configuration = new ConfigurationBuilder() - .AddInMemoryCollection(config) - .Build(); - - bool enrichWithHttpRequestMessageCalled = false; - bool enrichWithHttpResponseMessageCalled = false; - - var uri = new Uri($"{baseAddress}:1234"); - var uriHostNameType = Uri.CheckHostName(uri.Host); - - using var httpClient = ClientTestHelpers.CreateTestClient(async request => - { - var streamContent = await ClientTestHelpers.CreateResponseContent(new HelloReply()); - var response = ResponseUtils.CreateResponse(HttpStatusCode.OK, streamContent, grpcStatusCode: global::Grpc.Core.StatusCode.OK); - response.TrailingHeaders().Add("grpc-message", "value"); - return response; - }); - - var exportedItems = new List(); - - using var parent = new Activity("parent") - .SetIdFormat(ActivityIdFormat.W3C) - .Start(); - - using (Sdk.CreateTracerProviderBuilder() - .SetSampler(new AlwaysOnSampler()) - .ConfigureServices(services => services.AddSingleton(configuration)) - .AddGrpcClientInstrumentation(options => - { - if (shouldEnrich) - { - options.EnrichWithHttpRequestMessage = (activity, httpRequestMessage) => { enrichWithHttpRequestMessageCalled = true; }; - options.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => { enrichWithHttpResponseMessageCalled = true; }; - } - }) - .AddInMemoryExporter(exportedItems) - .Build()) - { - var channel = GrpcChannel.ForAddress(uri, new GrpcChannelOptions - { - HttpClient = httpClient, - }); - var client = new Greeter.GreeterClient(channel); - var rs = client.SayHello(new HelloRequest()); - } - - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - ValidateGrpcActivity(activity); - Assert.Equal(parent.TraceId, activity.Context.TraceId); - Assert.Equal(parent.SpanId, activity.ParentSpanId); - Assert.NotEqual(parent.SpanId, activity.Context.SpanId); - Assert.NotEqual(default, activity.Context.SpanId); - - Assert.Equal($"greet.Greeter/SayHello", activity.DisplayName); - Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystem)); - Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService)); - Assert.Equal("SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); - - if (uriHostNameType == UriHostNameType.IPv4 || uriHostNameType == UriHostNameType.IPv6) - { - Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeNetPeerIp)); - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeNetPeerName)); Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeServerSocketAddress)); Assert.Null(activity.GetTagValue(SemanticConventions.AttributeServerAddress)); } else { - Assert.Null(activity.GetTagValue(SemanticConventions.AttributeNetPeerIp)); - Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeNetPeerName)); Assert.Null(activity.GetTagValue(SemanticConventions.AttributeServerSocketAddress)); Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); } Assert.Equal(uri.Port, activity.GetTagValue(SemanticConventions.AttributeServerPort)); - Assert.Equal(uri.Port, activity.GetTagValue(SemanticConventions.AttributeNetPeerPort)); Assert.Equal(Status.Unset, activity.GetStatus()); // Tags added by the library then removed from the instrumentation diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs index 9ef4eaf9e79..1c397a27abe 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.server.cs @@ -14,7 +14,6 @@ using OpenTelemetry.Instrumentation.GrpcNetClient; using OpenTelemetry.Trace; using Xunit; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; using Status = OpenTelemetry.Trace.Status; namespace OpenTelemetry.Instrumentation.Grpc.Tests; @@ -43,7 +42,6 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(string enableGrpc var configuration = new ConfigurationBuilder() .AddInMemoryCollection(new Dictionary { - [SemanticConventionOptInKeyName] = "http", ["OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION"] = enableGrpcAspNetCoreSupport, }) .Build(); @@ -116,7 +114,6 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributesWhenItCreatesNewAc var configuration = new ConfigurationBuilder() .AddInMemoryCollection(new Dictionary { - [SemanticConventionOptInKeyName] = "http", ["OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION"] = enableGrpcAspNetCoreSupport, }) .Build();