From 86952d6e166b0c98bdc23719440d5339eac19b6d Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Wed, 22 Nov 2023 13:24:43 -0800 Subject: [PATCH 1/9] [HttpClient] Remove Sdk dependency --- .../GrpcClientDiagnosticListener.cs | 33 +------ .../.publicApi/PublicAPI.Unshipped.txt | 2 + .../HttpClientInstrumentationOptions.cs | 24 +---- .../HttpHandlerDiagnosticListener.cs | 7 +- .../HttpHandlerMetricsDiagnosticListener.cs | 5 - .../OpenTelemetry.Instrumentation.Http.csproj | 10 +- .../TracerProviderBuilderExtensions.cs | 2 - .../GrpcTests.client.cs | 92 ++++++++++--------- .../HttpClientTests.Basic.cs | 74 +++++---------- 9 files changed, 98 insertions(+), 151 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs index 9dd29e86fd5..8543eff88fb 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs @@ -96,34 +96,11 @@ public void OnStartActivity(Activity activity, object payload) return; } - if (this.options.SuppressDownstreamInstrumentation) - { - SuppressInstrumentationScope.Enter(); - - // If we are suppressing downstream instrumentation then inject - // context here. Grpc.Net.Client uses HttpClient, so - // SuppressDownstreamInstrumentation means that the - // OpenTelemetry instrumentation for HttpClient will not be - // invoked. - - // Note that HttpClient natively generates its own activity and - // propagates W3C trace context headers regardless of whether - // OpenTelemetry HttpClient instrumentation is invoked. - // Therefore, injecting here preserves more intuitive span - // parenting - i.e., the entry point span of a downstream - // service would be parented to the span generated by - // Grpc.Net.Client rather than the span generated natively by - // HttpClient. Injecting here also ensures that baggage is - // propagated to downstream services. - // Injecting context here also ensures that the configured - // propagator is used, as HttpClient by itself will only - // do TraceContext propagation. - var textMapPropagator = Propagators.DefaultTextMapPropagator; - textMapPropagator.Inject( - new PropagationContext(activity.Context, Baggage.Current), - request, - HttpRequestMessageContextPropagation.HeaderValueSetter); - } + var textMapPropagator = Propagators.DefaultTextMapPropagator; + textMapPropagator.Inject( + new PropagationContext(activity.Context, Baggage.Current), + request, + HttpRequestMessageContextPropagation.HeaderValueSetter); if (activity.IsAllDataRequested) { diff --git a/src/OpenTelemetry.Instrumentation.Http/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.Http/.publicApi/PublicAPI.Unshipped.txt index 2dd55551b0c..90bd1d37134 100644 --- a/src/OpenTelemetry.Instrumentation.Http/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.Http/.publicApi/PublicAPI.Unshipped.txt @@ -16,6 +16,8 @@ OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.FilterHttpWe OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.HttpClientInstrumentationOptions() -> void OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordException.get -> bool OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordException.set -> void +OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.SuppressInstrumentationWhenGrpcIsPresent.get -> bool +OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.SuppressInstrumentationWhenGrpcIsPresent.set -> void OpenTelemetry.Metrics.MeterProviderBuilderExtensions OpenTelemetry.Trace.TracerProviderBuilderExtensions static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs index 30c2b41641d..6cc75d9f589 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs @@ -20,9 +20,7 @@ using System.Net.Http; #endif using System.Runtime.CompilerServices; -using Microsoft.Extensions.Configuration; using OpenTelemetry.Instrumentation.Http.Implementation; -using static OpenTelemetry.Internal.HttpSemanticConventionHelper; namespace OpenTelemetry.Instrumentation.Http; @@ -31,23 +29,6 @@ namespace OpenTelemetry.Instrumentation.Http; /// public class HttpClientInstrumentationOptions { - internal readonly HttpSemanticConvention HttpSemanticConvention; - - /// - /// Initializes a new instance of the class. - /// - public HttpClientInstrumentationOptions() - : this(new ConfigurationBuilder().AddEnvironmentVariables().Build()) - { - } - - internal HttpClientInstrumentationOptions(IConfiguration configuration) - { - Debug.Assert(configuration != null, "configuration was null"); - - this.HttpSemanticConvention = GetSemanticConventionOptIn(configuration); - } - /// /// Gets or sets a filter function that determines whether or not to /// collect telemetry on a per request basis. @@ -157,6 +138,11 @@ internal HttpClientInstrumentationOptions(IConfiguration configuration) /// public bool RecordException { get; set; } + /// + /// Gets or sets a value indicating whether suppresses http instrumentation when the http call is part of grpc client call. + /// + public bool SuppressInstrumentationWhenGrpcIsPresent { get; set; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool EventFilterHttpRequestMessage(string activityName, object arg1) { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index 68e08949228..c99a418b92a 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -104,8 +104,13 @@ public void OnStartActivity(Activity activity, object payload) // By this time, samplers have already run and // activity.IsAllDataRequested populated accordingly. - if (Sdk.SuppressInstrumentation) + // In case when the http call is part of the grpc call and SuppressInstrumentationWhenGrpcIsPresent is set to true + // we skip injecting headers and set activity traceflags to false so that the activity is not exported. + // Grpc instrumentation will do the correct context propagation in this case. + if (this.options.SuppressInstrumentationWhenGrpcIsPresent && activity.Parent != null && activity.Parent.OperationName == "Grpc.Net.Client.GrpcOut") { + activity.IsAllDataRequested = false; + activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; return; } diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs index b3d45af15e8..15f6b6c1023 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerMetricsDiagnosticListener.cs @@ -66,11 +66,6 @@ public override void OnEventWritten(string name, object payload) public void OnStopEventWritten(Activity activity, object payload) { - if (Sdk.SuppressInstrumentation) - { - return; - } - if (TryFetchRequest(payload, out HttpRequestMessage request)) { // see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-metrics.md diff --git a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj index b9295759edf..292d8926e98 100644 --- a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj +++ b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj @@ -12,24 +12,24 @@ - - - - + + - + + + diff --git a/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs index f8bb13a615c..cf0863091f0 100644 --- a/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/TracerProviderBuilderExtensions.cs @@ -72,8 +72,6 @@ public static TracerProviderBuilder AddHttpClientInstrumentation( { services.Configure(name, configureHttpClientInstrumentationOptions); } - - services.RegisterOptionsFactory(configuration => new HttpClientInstrumentationOptions(configuration)); }); #if NETFRAMEWORK diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs index a9a6ff901ed..4a0ada256a6 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs @@ -16,6 +16,7 @@ using System.Diagnostics; using System.Net; +using System.Runtime.InteropServices; using Greet; #if !NETFRAMEWORK using Grpc.Core; @@ -394,16 +395,13 @@ public void GrpcAndHttpClientInstrumentationIsInvoked(bool shouldEnrich) public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() { var uri = new Uri($"http://localhost:{this.server.Port}"); - var processor = new Mock>(); - - using var parent = new Activity("parent") - .Start(); + var exporterItems = new List(); using (Sdk.CreateTracerProviderBuilder() .SetSampler(new AlwaysOnSampler()) - .AddGrpcClientInstrumentation(o => o.SuppressDownstreamInstrumentation = true) - .AddHttpClientInstrumentation() - .AddProcessor(processor.Object) + .AddGrpcClientInstrumentation() + .AddHttpClientInstrumentation(o => o.SuppressInstrumentationWhenGrpcIsPresent = true) + .AddInMemoryExporter(exporterItems) .Build()) { Parallel.ForEach( @@ -420,11 +418,11 @@ public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() }); } - Assert.Equal(11, processor.Invocations.Count); // SetParentProvider + OnStart/OnEnd (gRPC) * 4 + OnShutdown/Dispose called. - var grpcSpan1 = (Activity)processor.Invocations[2].Arguments[0]; - var grpcSpan2 = (Activity)processor.Invocations[4].Arguments[0]; - var grpcSpan3 = (Activity)processor.Invocations[6].Arguments[0]; - var grpcSpan4 = (Activity)processor.Invocations[8].Arguments[0]; + Assert.Equal(4, exporterItems.Count); // SetParentProvider + OnStart/OnEnd (gRPC) * 4 + OnShutdown/Dispose called. + var grpcSpan1 = exporterItems[0]; + var grpcSpan2 = exporterItems[1]; + var grpcSpan3 = exporterItems[2]; + var grpcSpan4 = exporterItems[3]; ValidateGrpcActivity(grpcSpan1); Assert.Equal($"greet.Greeter/SayHello", grpcSpan1.DisplayName); @@ -443,13 +441,15 @@ public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() Assert.Equal(0, grpcSpan4.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); } - [Fact] - public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue(bool suppressHttpInstrumentation) { try { var uri = new Uri($"http://localhost:{this.server.Port}"); - var processor = new Mock>(); + var exporterItems = new List(); using var source = new ActivitySource("test-source"); @@ -468,11 +468,8 @@ public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() using (Sdk.CreateTracerProviderBuilder() .AddSource("test-source") - .AddGrpcClientInstrumentation(o => - { - o.SuppressDownstreamInstrumentation = true; - }) - .AddHttpClientInstrumentation() + .AddGrpcClientInstrumentation() + .AddHttpClientInstrumentation(o => o.SuppressInstrumentationWhenGrpcIsPresent = suppressHttpInstrumentation) .AddAspNetCoreInstrumentation(options => { options.EnrichWithHttpRequest = (activity, request) => @@ -480,7 +477,7 @@ public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() activity.SetCustomProperty("customField", request.Headers["customField"].ToString()); }; }) // Instrumenting the server side as well - .AddProcessor(processor.Object) + .AddInMemoryExporter(exporterItems) .Build()) { using (var activity = source.StartActivity("parent")) @@ -490,31 +487,42 @@ public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() var client = new Greeter.GreeterClient(channel); var rs = client.SayHello(new HelloRequest()); } + } + + if (suppressHttpInstrumentation) + { + Assert.Equal(3, exporterItems.Count); // parent, grpc client and grpc server activity. + + var serverActivity = exporterItems[0]; + var clientActivity = exporterItems[1]; - WaitForProcessorInvocations(processor, 7); + Assert.Equal($"greet.Greeter/SayHello", clientActivity.DisplayName); + Assert.Equal($"greet.Greeter/SayHello", serverActivity.DisplayName); + Assert.Equal(clientActivity.TraceId, serverActivity.TraceId); + Assert.Equal(clientActivity.SpanId, serverActivity.ParentSpanId); + Assert.Equal(0, clientActivity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); + Assert.Equal("customValue", serverActivity.GetCustomProperty("customField") as string); } + else + { + Assert.Equal(4, exporterItems.Count); - Assert.Equal(9, processor.Invocations.Count); // SetParentProvider + (OnStart + OnEnd) * 3 (parent, gRPC client, and server) + Shutdown + Dispose called. + var server = exporterItems + .Where(item => item.OperationName == OperationNameHttpRequestIn).ToArray(); - Assert.Single(processor.Invocations, invo => invo.Method.Name == "SetParentProvider"); - Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnStart), "parent")); - Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnStart), OperationNameGrpcOut)); - Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnStart), OperationNameHttpRequestIn)); - Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnEnd), OperationNameHttpRequestIn)); - Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnEnd), OperationNameGrpcOut)); - Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnEnd), "parent")); - Assert.Single(processor.Invocations, invo => invo.Method.Name == "OnShutdown"); - Assert.Single(processor.Invocations, invo => invo.Method.Name == nameof(processor.Object.Dispose)); + var httpClient = exporterItems + .Where(item => item.OperationName == OperationNameHttpOut).ToArray(); - var serverActivity = GetActivityFromProcessorInvocation(processor, nameof(processor.Object.OnEnd), OperationNameHttpRequestIn); - var clientActivity = GetActivityFromProcessorInvocation(processor, nameof(processor.Object.OnEnd), OperationNameGrpcOut); + var grpcClient = exporterItems + .Where(item => item.OperationName == OperationNameGrpcOut).ToArray(); - Assert.Equal($"greet.Greeter/SayHello", clientActivity.DisplayName); - Assert.Equal($"greet.Greeter/SayHello", serverActivity.DisplayName); - Assert.Equal(clientActivity.TraceId, serverActivity.TraceId); - Assert.Equal(clientActivity.SpanId, serverActivity.ParentSpanId); - Assert.Equal(0, clientActivity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); - Assert.Equal("customValue", serverActivity.GetCustomProperty("customField") as string); + Assert.Single(server); + Assert.Single(httpClient); + Assert.Single(grpcClient); + + Assert.Equal(server[0].ParentId, httpClient[0].Id); + Assert.Equal(httpClient[0].ParentId, grpcClient[0].Id); + } } finally { @@ -526,7 +534,7 @@ public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() } } - [Fact] + [Fact(Skip = "TODO:Remove this test, this will not be needed")] public void GrpcDoesNotPropagateContextWithSuppressInstrumentationOptionSetToFalse() { try @@ -586,7 +594,7 @@ public void GrpcDoesNotPropagateContextWithSuppressInstrumentationOptionSetToFal } } - [Fact] + [Fact(Skip = "TODO:Remove this test, this will not be needed")] public void GrpcClientInstrumentationRespectsSdkSuppressInstrumentation() { try diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index 5fd03c1ddb2..cf316c7f5e0 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -278,61 +278,37 @@ public async Task InjectsHeadersAsync_CustomFormat() })); } - [Fact] - public async Task RespectsSuppress() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task SuppressIsNotAffectedInAbsenceOfGrpcCall(bool suppressInstrumentationWhenGrpcIsPresent) { - try - { - var propagator = new Mock(); - propagator.Setup(m => m.Inject(It.IsAny(), It.IsAny(), It.IsAny>())) - .Callback>((context, message, action) => - { - action(message, "custom_traceparent", $"00/{context.ActivityContext.TraceId}/{context.ActivityContext.SpanId}/01"); - action(message, "custom_tracestate", Activity.Current.TraceStateString); - }); - - var processor = new Mock>(); - - using var request = new HttpRequestMessage - { - RequestUri = new Uri(this.url), - Method = new HttpMethod("GET"), - }; - - using var parent = new Activity("parent") - .SetIdFormat(ActivityIdFormat.W3C) - .Start(); - parent.TraceStateString = "k1=v1,k2=v2"; - parent.ActivityTraceFlags = ActivityTraceFlags.Recorded; + var exportedItems = new List(); - Sdk.SetDefaultTextMapPropagator(propagator.Object); + using var request = new HttpRequestMessage + { + RequestUri = new Uri(this.url), + Method = new HttpMethod("GET"), + }; - using (Sdk.CreateTracerProviderBuilder() - .AddHttpClientInstrumentation() - .AddProcessor(processor.Object) - .Build()) - { - using var c = new HttpClient(); - using (SuppressInstrumentationScope.Begin()) - { - await c.SendAsync(request); - } - } + // Set non-grpc parent activity + using var parent = new Activity("parent") + .SetIdFormat(ActivityIdFormat.W3C) + .Start(); + parent.TraceStateString = "k1=v1,k2=v2"; + parent.ActivityTraceFlags = ActivityTraceFlags.Recorded; - // If suppressed, activity is not emitted and - // propagation is also not performed. - Assert.Equal(3, processor.Invocations.Count); // SetParentProvider/OnShutdown/Dispose called. - Assert.False(request.Headers.Contains("custom_traceparent")); - Assert.False(request.Headers.Contains("custom_tracestate")); - } - finally + using (Sdk.CreateTracerProviderBuilder() + .AddHttpClientInstrumentation(o => o.SuppressInstrumentationWhenGrpcIsPresent = suppressInstrumentationWhenGrpcIsPresent) + .AddInMemoryExporter(exportedItems) + .Build()) { - Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[] - { - new TraceContextPropagator(), - new BaggagePropagator(), - })); + using var c = new HttpClient(); + + await c.SendAsync(request); } + + Assert.Single(exportedItems); } [Fact] From d8b9336cae13e4c1e22da00f27071fcdda9f1900 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Wed, 22 Nov 2023 13:29:12 -0800 Subject: [PATCH 2/9] refactor test --- .../GrpcTests.client.cs | 121 +++++++----------- 1 file changed, 48 insertions(+), 73 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs index 4a0ada256a6..bbe9be24026 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs @@ -446,91 +446,66 @@ public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() [InlineData(false)] public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue(bool suppressHttpInstrumentation) { - try - { - var uri = new Uri($"http://localhost:{this.server.Port}"); - var exporterItems = new List(); - - using var source = new ActivitySource("test-source"); - - var propagator = new Mock(); - propagator.Setup(m => m.Inject(It.IsAny(), It.IsAny(), It.IsAny>())) - .Callback>((context, message, action) => - { - action(message, "customField", "customValue"); - }); + var uri = new Uri($"http://localhost:{this.server.Port}"); + var exporterItems = new List(); - Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[] - { - new TraceContextPropagator(), - propagator.Object, - })); + using var source = new ActivitySource("test-source"); - using (Sdk.CreateTracerProviderBuilder() - .AddSource("test-source") - .AddGrpcClientInstrumentation() - .AddHttpClientInstrumentation(o => o.SuppressInstrumentationWhenGrpcIsPresent = suppressHttpInstrumentation) - .AddAspNetCoreInstrumentation(options => - { - options.EnrichWithHttpRequest = (activity, request) => - { - activity.SetCustomProperty("customField", request.Headers["customField"].ToString()); - }; - }) // Instrumenting the server side as well - .AddInMemoryExporter(exporterItems) - .Build()) + using (Sdk.CreateTracerProviderBuilder() + .AddSource("test-source") + .AddGrpcClientInstrumentation() + .AddHttpClientInstrumentation(o => o.SuppressInstrumentationWhenGrpcIsPresent = suppressHttpInstrumentation) + .AddAspNetCoreInstrumentation() + .AddInMemoryExporter(exporterItems) + .Build()) + { + using (var activity = source.StartActivity("parent")) { - using (var activity = source.StartActivity("parent")) - { - Assert.NotNull(activity); - var channel = GrpcChannel.ForAddress(uri); - var client = new Greeter.GreeterClient(channel); - var rs = client.SayHello(new HelloRequest()); - } + Assert.NotNull(activity); + var channel = GrpcChannel.ForAddress(uri); + var client = new Greeter.GreeterClient(channel); + var rs = client.SayHello(new HelloRequest()); } + } - if (suppressHttpInstrumentation) - { - Assert.Equal(3, exporterItems.Count); // parent, grpc client and grpc server activity. - - var serverActivity = exporterItems[0]; - var clientActivity = exporterItems[1]; - - Assert.Equal($"greet.Greeter/SayHello", clientActivity.DisplayName); - Assert.Equal($"greet.Greeter/SayHello", serverActivity.DisplayName); - Assert.Equal(clientActivity.TraceId, serverActivity.TraceId); - Assert.Equal(clientActivity.SpanId, serverActivity.ParentSpanId); - Assert.Equal(0, clientActivity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); - Assert.Equal("customValue", serverActivity.GetCustomProperty("customField") as string); - } - else - { - Assert.Equal(4, exporterItems.Count); + if (suppressHttpInstrumentation) + { + Assert.Equal(3, exporterItems.Count); // parent, grpc client and grpc server activity. - var server = exporterItems - .Where(item => item.OperationName == OperationNameHttpRequestIn).ToArray(); + var server = exporterItems + .Where(item => item.OperationName == OperationNameHttpRequestIn).ToArray(); - var httpClient = exporterItems - .Where(item => item.OperationName == OperationNameHttpOut).ToArray(); + var httpClient = exporterItems + .Where(item => item.OperationName == OperationNameHttpOut).ToArray(); - var grpcClient = exporterItems - .Where(item => item.OperationName == OperationNameGrpcOut).ToArray(); + var grpcClient = exporterItems + .Where(item => item.OperationName == OperationNameGrpcOut).ToArray(); - Assert.Single(server); - Assert.Single(httpClient); - Assert.Single(grpcClient); + Assert.Single(server); + Assert.Empty(httpClient); + Assert.Single(grpcClient); - Assert.Equal(server[0].ParentId, httpClient[0].Id); - Assert.Equal(httpClient[0].ParentId, grpcClient[0].Id); - } + Assert.Equal(server[0].ParentId, grpcClient[0].Id); } - finally + else { - Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[] - { - new TraceContextPropagator(), - new BaggagePropagator(), - })); + Assert.Equal(4, exporterItems.Count); // parent, grpc client, httpclient and grpc server activity. + + var server = exporterItems + .Where(item => item.OperationName == OperationNameHttpRequestIn).ToArray(); + + var httpClient = exporterItems + .Where(item => item.OperationName == OperationNameHttpOut).ToArray(); + + var grpcClient = exporterItems + .Where(item => item.OperationName == OperationNameGrpcOut).ToArray(); + + Assert.Single(server); + Assert.Single(httpClient); + Assert.Single(grpcClient); + + Assert.Equal(server[0].ParentId, httpClient[0].Id); + Assert.Equal(httpClient[0].ParentId, grpcClient[0].Id); } } From 61cf01937d2d36ac041c90af8ffff7fc18661ff3 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Wed, 22 Nov 2023 13:45:43 -0800 Subject: [PATCH 3/9] rmv using --- .../OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs index bbe9be24026..afa0353ed4b 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs @@ -16,7 +16,6 @@ using System.Diagnostics; using System.Net; -using System.Runtime.InteropServices; using Greet; #if !NETFRAMEWORK using Grpc.Core; From b015b0283555ec16d3d68011bf1cf8928f35e159 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 28 Nov 2023 12:11:56 -0800 Subject: [PATCH 4/9] remove new option --- .../GrpcClientDiagnosticListener.cs | 33 +++- .../.publicApi/PublicAPI.Unshipped.txt | 2 - .../HttpClientInstrumentationOptions.cs | 5 - .../HttpHandlerDiagnosticListener.cs | 10 -- .../GrpcTests.client.cs | 148 ++++++++++-------- .../HttpClientTests.Basic.cs | 74 ++++++--- 6 files changed, 160 insertions(+), 112 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs index 8543eff88fb..9dd29e86fd5 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs @@ -96,11 +96,34 @@ public void OnStartActivity(Activity activity, object payload) return; } - var textMapPropagator = Propagators.DefaultTextMapPropagator; - textMapPropagator.Inject( - new PropagationContext(activity.Context, Baggage.Current), - request, - HttpRequestMessageContextPropagation.HeaderValueSetter); + if (this.options.SuppressDownstreamInstrumentation) + { + SuppressInstrumentationScope.Enter(); + + // If we are suppressing downstream instrumentation then inject + // context here. Grpc.Net.Client uses HttpClient, so + // SuppressDownstreamInstrumentation means that the + // OpenTelemetry instrumentation for HttpClient will not be + // invoked. + + // Note that HttpClient natively generates its own activity and + // propagates W3C trace context headers regardless of whether + // OpenTelemetry HttpClient instrumentation is invoked. + // Therefore, injecting here preserves more intuitive span + // parenting - i.e., the entry point span of a downstream + // service would be parented to the span generated by + // Grpc.Net.Client rather than the span generated natively by + // HttpClient. Injecting here also ensures that baggage is + // propagated to downstream services. + // Injecting context here also ensures that the configured + // propagator is used, as HttpClient by itself will only + // do TraceContext propagation. + var textMapPropagator = Propagators.DefaultTextMapPropagator; + textMapPropagator.Inject( + new PropagationContext(activity.Context, Baggage.Current), + request, + HttpRequestMessageContextPropagation.HeaderValueSetter); + } if (activity.IsAllDataRequested) { diff --git a/src/OpenTelemetry.Instrumentation.Http/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.Http/.publicApi/PublicAPI.Unshipped.txt index 90bd1d37134..2dd55551b0c 100644 --- a/src/OpenTelemetry.Instrumentation.Http/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.Http/.publicApi/PublicAPI.Unshipped.txt @@ -16,8 +16,6 @@ OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.FilterHttpWe OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.HttpClientInstrumentationOptions() -> void OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordException.get -> bool OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.RecordException.set -> void -OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.SuppressInstrumentationWhenGrpcIsPresent.get -> bool -OpenTelemetry.Instrumentation.Http.HttpClientInstrumentationOptions.SuppressInstrumentationWhenGrpcIsPresent.set -> void OpenTelemetry.Metrics.MeterProviderBuilderExtensions OpenTelemetry.Trace.TracerProviderBuilderExtensions static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddHttpClientInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder diff --git a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs index 6cc75d9f589..f0cd19691e2 100644 --- a/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs @@ -138,11 +138,6 @@ public class HttpClientInstrumentationOptions /// public bool RecordException { get; set; } - /// - /// Gets or sets a value indicating whether suppresses http instrumentation when the http call is part of grpc client call. - /// - public bool SuppressInstrumentationWhenGrpcIsPresent { get; set; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool EventFilterHttpRequestMessage(string activityName, object arg1) { diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index c99a418b92a..82743153487 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -104,16 +104,6 @@ public void OnStartActivity(Activity activity, object payload) // By this time, samplers have already run and // activity.IsAllDataRequested populated accordingly. - // In case when the http call is part of the grpc call and SuppressInstrumentationWhenGrpcIsPresent is set to true - // we skip injecting headers and set activity traceflags to false so that the activity is not exported. - // Grpc instrumentation will do the correct context propagation in this case. - if (this.options.SuppressInstrumentationWhenGrpcIsPresent && activity.Parent != null && activity.Parent.OperationName == "Grpc.Net.Client.GrpcOut") - { - activity.IsAllDataRequested = false; - activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; - return; - } - if (!TryFetchRequest(payload, out HttpRequestMessage request)) { HttpInstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerDiagnosticListener), nameof(this.OnStartActivity)); diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs index afa0353ed4b..59f6c917e55 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs @@ -390,17 +390,20 @@ public void GrpcAndHttpClientInstrumentationIsInvoked(bool shouldEnrich) Assert.Equal(shouldEnrich ? "yes" : "no", grpcSpan.Tags.Where(tag => tag.Key == "enrichedWithHttpResponseMessage").FirstOrDefault().Value); } - [Fact] + [Fact(Skip = "Removed for stable release of http instrumentation")] public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() { var uri = new Uri($"http://localhost:{this.server.Port}"); - var exporterItems = new List(); + var processor = new Mock>(); + + using var parent = new Activity("parent") + .Start(); using (Sdk.CreateTracerProviderBuilder() .SetSampler(new AlwaysOnSampler()) - .AddGrpcClientInstrumentation() - .AddHttpClientInstrumentation(o => o.SuppressInstrumentationWhenGrpcIsPresent = true) - .AddInMemoryExporter(exporterItems) + .AddGrpcClientInstrumentation(o => o.SuppressDownstreamInstrumentation = true) + .AddHttpClientInstrumentation() + .AddProcessor(processor.Object) .Build()) { Parallel.ForEach( @@ -417,11 +420,11 @@ public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() }); } - Assert.Equal(4, exporterItems.Count); // SetParentProvider + OnStart/OnEnd (gRPC) * 4 + OnShutdown/Dispose called. - var grpcSpan1 = exporterItems[0]; - var grpcSpan2 = exporterItems[1]; - var grpcSpan3 = exporterItems[2]; - var grpcSpan4 = exporterItems[3]; + Assert.Equal(11, processor.Invocations.Count); // SetParentProvider + OnStart/OnEnd (gRPC) * 4 + OnShutdown/Dispose called. + var grpcSpan1 = (Activity)processor.Invocations[2].Arguments[0]; + var grpcSpan2 = (Activity)processor.Invocations[4].Arguments[0]; + var grpcSpan3 = (Activity)processor.Invocations[6].Arguments[0]; + var grpcSpan4 = (Activity)processor.Invocations[8].Arguments[0]; ValidateGrpcActivity(grpcSpan1); Assert.Equal($"greet.Greeter/SayHello", grpcSpan1.DisplayName); @@ -440,75 +443,90 @@ public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() Assert.Equal(0, grpcSpan4.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue(bool suppressHttpInstrumentation) + [Fact(Skip = "Removed for stable release of http instrumentation")] + public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() { - var uri = new Uri($"http://localhost:{this.server.Port}"); - var exporterItems = new List(); + try + { + var uri = new Uri($"http://localhost:{this.server.Port}"); + var processor = new Mock>(); - using var source = new ActivitySource("test-source"); + using var source = new ActivitySource("test-source"); - using (Sdk.CreateTracerProviderBuilder() - .AddSource("test-source") - .AddGrpcClientInstrumentation() - .AddHttpClientInstrumentation(o => o.SuppressInstrumentationWhenGrpcIsPresent = suppressHttpInstrumentation) - .AddAspNetCoreInstrumentation() - .AddInMemoryExporter(exporterItems) - .Build()) - { - using (var activity = source.StartActivity("parent")) + var propagator = new Mock(); + propagator.Setup(m => m.Inject(It.IsAny(), It.IsAny(), It.IsAny>())) + .Callback>((context, message, action) => + { + action(message, "customField", "customValue"); + }); + + Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[] { - Assert.NotNull(activity); - var channel = GrpcChannel.ForAddress(uri); - var client = new Greeter.GreeterClient(channel); - var rs = client.SayHello(new HelloRequest()); - } - } + new TraceContextPropagator(), + propagator.Object, + })); - if (suppressHttpInstrumentation) - { - Assert.Equal(3, exporterItems.Count); // parent, grpc client and grpc server activity. + using (Sdk.CreateTracerProviderBuilder() + .AddSource("test-source") + .AddGrpcClientInstrumentation(o => + { + o.SuppressDownstreamInstrumentation = true; + }) + .AddHttpClientInstrumentation() + .AddAspNetCoreInstrumentation(options => + { + options.EnrichWithHttpRequest = (activity, request) => + { + activity.SetCustomProperty("customField", request.Headers["customField"].ToString()); + }; + }) // Instrumenting the server side as well + .AddProcessor(processor.Object) + .Build()) + { + using (var activity = source.StartActivity("parent")) + { + Assert.NotNull(activity); + var channel = GrpcChannel.ForAddress(uri); + var client = new Greeter.GreeterClient(channel); + var rs = client.SayHello(new HelloRequest()); + } - var server = exporterItems - .Where(item => item.OperationName == OperationNameHttpRequestIn).ToArray(); + WaitForProcessorInvocations(processor, 7); + } - var httpClient = exporterItems - .Where(item => item.OperationName == OperationNameHttpOut).ToArray(); + Assert.Equal(9, processor.Invocations.Count); // SetParentProvider + (OnStart + OnEnd) * 3 (parent, gRPC client, and server) + Shutdown + Dispose called. - var grpcClient = exporterItems - .Where(item => item.OperationName == OperationNameGrpcOut).ToArray(); + Assert.Single(processor.Invocations, invo => invo.Method.Name == "SetParentProvider"); + Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnStart), "parent")); + Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnStart), OperationNameGrpcOut)); + Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnStart), OperationNameHttpRequestIn)); + Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnEnd), OperationNameHttpRequestIn)); + Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnEnd), OperationNameGrpcOut)); + Assert.Single(processor.Invocations, GeneratePredicateForMoqProcessorActivity(nameof(processor.Object.OnEnd), "parent")); + Assert.Single(processor.Invocations, invo => invo.Method.Name == "OnShutdown"); + Assert.Single(processor.Invocations, invo => invo.Method.Name == nameof(processor.Object.Dispose)); - Assert.Single(server); - Assert.Empty(httpClient); - Assert.Single(grpcClient); + var serverActivity = GetActivityFromProcessorInvocation(processor, nameof(processor.Object.OnEnd), OperationNameHttpRequestIn); + var clientActivity = GetActivityFromProcessorInvocation(processor, nameof(processor.Object.OnEnd), OperationNameGrpcOut); - Assert.Equal(server[0].ParentId, grpcClient[0].Id); + Assert.Equal($"greet.Greeter/SayHello", clientActivity.DisplayName); + Assert.Equal($"greet.Greeter/SayHello", serverActivity.DisplayName); + Assert.Equal(clientActivity.TraceId, serverActivity.TraceId); + Assert.Equal(clientActivity.SpanId, serverActivity.ParentSpanId); + Assert.Equal(0, clientActivity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); + Assert.Equal("customValue", serverActivity.GetCustomProperty("customField") as string); } - else + finally { - Assert.Equal(4, exporterItems.Count); // parent, grpc client, httpclient and grpc server activity. - - var server = exporterItems - .Where(item => item.OperationName == OperationNameHttpRequestIn).ToArray(); - - var httpClient = exporterItems - .Where(item => item.OperationName == OperationNameHttpOut).ToArray(); - - var grpcClient = exporterItems - .Where(item => item.OperationName == OperationNameGrpcOut).ToArray(); - - Assert.Single(server); - Assert.Single(httpClient); - Assert.Single(grpcClient); - - Assert.Equal(server[0].ParentId, httpClient[0].Id); - Assert.Equal(httpClient[0].ParentId, grpcClient[0].Id); + Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[] + { + new TraceContextPropagator(), + new BaggagePropagator(), + })); } } - [Fact(Skip = "TODO:Remove this test, this will not be needed")] + [Fact] public void GrpcDoesNotPropagateContextWithSuppressInstrumentationOptionSetToFalse() { try @@ -568,7 +586,7 @@ public void GrpcDoesNotPropagateContextWithSuppressInstrumentationOptionSetToFal } } - [Fact(Skip = "TODO:Remove this test, this will not be needed")] + [Fact(Skip = "Removed for stable release of http instrumentation")] public void GrpcClientInstrumentationRespectsSdkSuppressInstrumentation() { try diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index cf316c7f5e0..ef78701eb44 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -278,37 +278,61 @@ public async Task InjectsHeadersAsync_CustomFormat() })); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public async Task SuppressIsNotAffectedInAbsenceOfGrpcCall(bool suppressInstrumentationWhenGrpcIsPresent) + [Fact(Skip = "Removed for stable release of http instrumentation")] + public async Task RespectsSuppress() { - var exportedItems = new List(); - - using var request = new HttpRequestMessage + try { - RequestUri = new Uri(this.url), - Method = new HttpMethod("GET"), - }; + var propagator = new Mock(); + propagator.Setup(m => m.Inject(It.IsAny(), It.IsAny(), It.IsAny>())) + .Callback>((context, message, action) => + { + action(message, "custom_traceparent", $"00/{context.ActivityContext.TraceId}/{context.ActivityContext.SpanId}/01"); + action(message, "custom_tracestate", Activity.Current.TraceStateString); + }); - // Set non-grpc parent activity - using var parent = new Activity("parent") - .SetIdFormat(ActivityIdFormat.W3C) - .Start(); - parent.TraceStateString = "k1=v1,k2=v2"; - parent.ActivityTraceFlags = ActivityTraceFlags.Recorded; + var processor = new Mock>(); - using (Sdk.CreateTracerProviderBuilder() - .AddHttpClientInstrumentation(o => o.SuppressInstrumentationWhenGrpcIsPresent = suppressInstrumentationWhenGrpcIsPresent) - .AddInMemoryExporter(exportedItems) - .Build()) - { - using var c = new HttpClient(); + using var request = new HttpRequestMessage + { + RequestUri = new Uri(this.url), + Method = new HttpMethod("GET"), + }; - await c.SendAsync(request); - } + using var parent = new Activity("parent") + .SetIdFormat(ActivityIdFormat.W3C) + .Start(); + parent.TraceStateString = "k1=v1,k2=v2"; + parent.ActivityTraceFlags = ActivityTraceFlags.Recorded; - Assert.Single(exportedItems); + Sdk.SetDefaultTextMapPropagator(propagator.Object); + + using (Sdk.CreateTracerProviderBuilder() + .AddHttpClientInstrumentation() + .AddProcessor(processor.Object) + .Build()) + { + using var c = new HttpClient(); + using (SuppressInstrumentationScope.Begin()) + { + await c.SendAsync(request); + } + } + + // If suppressed, activity is not emitted and + // propagation is also not performed. + Assert.Equal(3, processor.Invocations.Count); // SetParentProvider/OnShutdown/Dispose called. + Assert.False(request.Headers.Contains("custom_traceparent")); + Assert.False(request.Headers.Contains("custom_tracestate")); + } + finally + { + Sdk.SetDefaultTextMapPropagator(new CompositeTextMapPropagator(new TextMapPropagator[] + { + new TraceContextPropagator(), + new BaggagePropagator(), + })); + } } [Fact] From 63e2dd6b9b3e6bf818c120e4dbb097e625653418 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 28 Nov 2023 13:01:28 -0800 Subject: [PATCH 5/9] rmv api ref --- .../OpenTelemetry.Instrumentation.Http.csproj | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj index 292d8926e98..9201d0e06d2 100644 --- a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj +++ b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj @@ -17,13 +17,11 @@ - - From bfb1760d594c5292299b57aec96288e9aae8b778 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 28 Nov 2023 13:17:02 -0800 Subject: [PATCH 6/9] nit --- .../OpenTelemetry.Instrumentation.Http.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj index 9201d0e06d2..32f80e31c21 100644 --- a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj +++ b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj @@ -27,7 +27,7 @@ - + From d292a26ee749eea22b4446c87f0e1b06dca75f45 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 28 Nov 2023 18:52:41 -0800 Subject: [PATCH 7/9] changelog --- src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 2d5b686bfcc..5ef8ca9eb84 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -11,6 +11,14 @@ * Update activity DisplayName as per the specification. ([#5078](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5078)) +* Removed reference to `OpenTelemetry` package. This is a **breaking change** + for users relying on + [SuppressDownstreamInstrumentation](https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Instrumentation.GrpcNetClient#suppressdownstreaminstrumentation) + option in `OpenTelemetry.Instrumentation.GrpcNetClient`. For details, check + out this + [issue](https://github.com/open-telemetry/opentelemetry-dotnet/issues/5092). + ([#5077](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5077)) + ## 1.6.0-beta.3 Released 2023-Nov-17 From 323b381aea9113c717120b3b058bec7391f577d5 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 28 Nov 2023 19:14:58 -0800 Subject: [PATCH 8/9] upsate test --- .../GrpcTests.client.cs | 6 +++--- .../HttpClientTests.Basic.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs index 93e3ef7725e..53328008bfb 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs @@ -390,7 +390,7 @@ public void GrpcAndHttpClientInstrumentationIsInvoked(bool shouldEnrich) Assert.Equal(shouldEnrich ? "yes" : "no", grpcSpan.Tags.Where(tag => tag.Key == "enrichedWithHttpResponseMessage").FirstOrDefault().Value); } - [Fact(Skip = "Removed for stable release of http instrumentation")] + [Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/issues/5092")] public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() { var uri = new Uri($"http://localhost:{this.server.Port}"); @@ -443,7 +443,7 @@ public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() Assert.Equal(0, grpcSpan4.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); } - [Fact(Skip = "Removed for stable release of http instrumentation")] + [Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/issues/5092")] public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() { try @@ -586,7 +586,7 @@ public void GrpcDoesNotPropagateContextWithSuppressInstrumentationOptionSetToFal } } - [Fact(Skip = "Removed for stable release of http instrumentation")] + [Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/issues/5092")] public void GrpcClientInstrumentationRespectsSdkSuppressInstrumentation() { try diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs index 2205ba4c30b..97794e53d54 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.cs @@ -278,7 +278,7 @@ public async Task InjectsHeadersAsync_CustomFormat() })); } - [Fact(Skip = "Removed for stable release of http instrumentation")] + [Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/issues/5092")] public async Task RespectsSuppress() { try From a7f1350de7aa3d2cdbb2a8818964df5af4321f61 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Tue, 28 Nov 2023 19:25:41 -0800 Subject: [PATCH 9/9] grpc changelog --- .../CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md index 2299a1573a9..51ae981b732 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +* **Breaking Change** : + [SuppressDownstreamInstrumentation](https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Instrumentation.GrpcNetClient#suppressdownstreaminstrumentation) + option will no longer be supported when used with certain versions of + `OpenTelemetry.Instrumentation.Http` package. Check out this + [issue](https://github.com/open-telemetry/opentelemetry-dotnet/issues/5092) + for details and workaround. + ([#5077](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5077)) + ## 1.6.0-beta.3 Released 2023-Nov-17