diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index 82eecf60816..db708294cc8 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -1,6 +1,8 @@ OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions -OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.Enrich.get -> System.Action -OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.Enrich.set -> void +OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.EnrichWithHttpRequestMessage.get -> System.Action +OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.EnrichWithHttpRequestMessage.set -> void +OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.EnrichWithHttpResponseMessage.get -> System.Action +OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.EnrichWithHttpResponseMessage.set -> void OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.GrpcClientInstrumentationOptions() -> void OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.SuppressDownstreamInstrumentation.get -> bool OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.SuppressDownstreamInstrumentation.set -> void diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt index 82eecf60816..db708294cc8 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/.publicApi/netstandard2.1/PublicAPI.Unshipped.txt @@ -1,6 +1,8 @@ OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions -OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.Enrich.get -> System.Action -OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.Enrich.set -> void +OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.EnrichWithHttpRequestMessage.get -> System.Action +OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.EnrichWithHttpRequestMessage.set -> void +OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.EnrichWithHttpResponseMessage.get -> System.Action +OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.EnrichWithHttpResponseMessage.set -> void OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.GrpcClientInstrumentationOptions() -> void OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.SuppressDownstreamInstrumentation.get -> bool OpenTelemetry.Instrumentation.GrpcNetClient.GrpcClientInstrumentationOptions.SuppressDownstreamInstrumentation.set -> void diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md index 8506f0d6163..08545533222 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md @@ -2,6 +2,17 @@ ## Unreleased + **Breaking change** The `Enrich` callback option has been removed. For better + usability, it has been replaced by two separate options: + `EnrichWithHttpRequestMessage`and `EnrichWithHttpResponseMessage`. Previously, + the single `Enrich` callback required the consumer to detect which event + triggered the callback to be invoked (e.g., request start or response end) and + then cast the object received to the appropriate type: `HttpRequestMessage` + and `HttpResponseMessage`. The separate callbacks make it clear what event + triggers them and there is no longer the need to cast the argument to the + expected type. + ([#3804](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3804)) + ## 1.0.0-rc9.8 Released 2022-Oct-17 diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentationOptions.cs index 91735e3a864..8a055128b03 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/GrpcClientInstrumentationOptions.cs @@ -16,6 +16,7 @@ using System; using System.Diagnostics; +using System.Net.Http; namespace OpenTelemetry.Instrumentation.GrpcNetClient { @@ -30,14 +31,21 @@ public class GrpcClientInstrumentationOptions public bool SuppressDownstreamInstrumentation { get; set; } /// - /// Gets or sets an action to enrich an Activity. + /// Gets or sets an action to enrich the Activity with . /// /// /// : the activity being enriched. - /// string: the name of the event. - /// object: the raw object from which additional information can be extracted to enrich the activity. - /// The type of this object depends on the event, which is given by the above parameter. + /// object from which additional information can be extracted to enrich the activity. /// - public Action Enrich { get; set; } + public Action EnrichWithHttpRequestMessage { get; set; } + + /// + /// Gets or sets an action to enrich an Activity with . + /// + /// + /// : the activity being enriched. + /// object from which additional information can be extracted to enrich the activity. + /// + public Action EnrichWithHttpResponseMessage { get; set; } } } diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs index 299d9ff9f8e..179831d2a67 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs @@ -150,7 +150,7 @@ public void OnStartActivity(Activity activity, object payload) try { - this.options.Enrich?.Invoke(activity, "OnStartActivity", request); + this.options.EnrichWithHttpRequestMessage?.Invoke(activity, request); } catch (Exception ex) { @@ -182,7 +182,7 @@ public void OnStopActivity(Activity activity, object payload) { try { - this.options.Enrich?.Invoke(activity, "OnStopActivity", response); + this.options.EnrichWithHttpResponseMessage?.Invoke(activity, response); } catch (Exception ex) { diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/README.md b/src/OpenTelemetry.Instrumentation.GrpcNetClient/README.md index 7dcb2d2c98c..462f5439321 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/README.md +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/README.md @@ -99,31 +99,29 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder() ### Enrich -This option allows one to enrich the activity with additional information -from the raw `HttpRequestMessage` object. The `Enrich` action is called only -when `activity.IsAllDataRequested` is `true`. It contains the activity itself -(which can be enriched), the name of the event, and the actual raw object. -For event name "OnStartActivity", the actual object will be -`HttpRequestMessage`. - -The following code snippet shows how to add additional tags using `Enrich`. +This instrumentation library provides `EnrichWithHttpRequestMessage` and +`EnrichWithHttpResponseMessage` options that can be used to enrich the activity +with additional information from the raw `HttpRequestMessage` and +`HttpResponseMessage` objects respectively. These actions are called only when +`activity.IsAllDataRequested` is `true`. It contains the activity itself (which +can be enriched), the name of the event, and the actual raw object. The +following code snippet shows how to add additional tags using these options. ```csharp services.AddOpenTelemetryTracing((builder) => { builder - .AddGrpcClientInstrumentation(opt => opt.Enrich - = (activity, eventName, rawObject) => + .AddGrpcClientInstrumentation((options) => { - if (eventName.Equals("OnStartActivity")) + options.EnrichWithHttpRequestMessage = (activity, httpRequestMessage) => + { + activity.SetTag("requestVersion", httpRequestMessage.Version); + }; + options.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => { - if (rawObject is HttpRequestMessage request) - { - activity.SetTag("requestVersion", request.Version); - } - } + activity.SetTag("responseVersion", httpResponseMessage.Version); + }; }) -}); ``` [Processor](../../docs/trace/extending-the-sdk/README.md#processor), diff --git a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs index 044f9739f4c..89525480fbb 100644 --- a/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs @@ -24,9 +24,6 @@ using Grpc.Core; using Grpc.Net.Client; using Microsoft.Extensions.DependencyInjection; -#if NET6_0_OR_GREATER -using Microsoft.AspNetCore.Http; -#endif using Moq; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Instrumentation.Grpc.Tests.GrpcTestHelpers; @@ -49,6 +46,9 @@ public partial class GrpcTests [InlineData("http://[::1]", false)] public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool shouldEnrich = true) { + bool enrichWithHttpRequestMessageCalled = false; + bool enrichWithHttpResponseMessageCalled = false; + var uri = new Uri($"{baseAddress}:1234"); var uriHostNameType = Uri.CheckHostName(uri.Host); @@ -72,7 +72,8 @@ public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool sho { if (shouldEnrich) { - options.Enrich = ActivityEnrichment; + options.EnrichWithHttpRequestMessage = (activity, httpRequestMessage) => { enrichWithHttpRequestMessageCalled = true; }; + options.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => { enrichWithHttpResponseMessageCalled = true; }; } }) .AddProcessor(processor.Object) @@ -118,6 +119,12 @@ public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool sho 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); + } } #if NET6_0_OR_GREATER @@ -128,7 +135,12 @@ public void GrpcAndHttpClientInstrumentationIsInvoked(bool shouldEnrich) { var uri = new Uri($"http://localhost:{this.server.Port}"); var processor = new Mock>(); - processor.Setup(x => x.OnStart(It.IsAny())).Callback(c => c.SetTag("enriched", "no")); + processor.Setup(x => x.OnStart(It.IsAny())).Callback(c => + { + c.SetTag("enrichedWithHttpRequestMessage", "no"); + c.SetTag("enrichedWithHttpResponseMessage", "no"); + }); + var parent = new Activity("parent") .Start(); @@ -138,7 +150,15 @@ public void GrpcAndHttpClientInstrumentationIsInvoked(bool shouldEnrich) { if (shouldEnrich) { - options.Enrich = ActivityEnrichment; + options.EnrichWithHttpRequestMessage = (activity, httpRequestMessage) => + { + activity.SetTag("enrichedWithHttpRequestMessage", "yes"); + }; + + options.EnrichWithHttpResponseMessage = (activity, httpResponseMessage) => + { + activity.SetTag("enrichedWithHttpResponseMessage", "yes"); + }; } }) .AddHttpClientInstrumentation() @@ -166,14 +186,14 @@ public void GrpcAndHttpClientInstrumentationIsInvoked(bool shouldEnrich) Assert.Equal($"HTTP POST", httpSpan.DisplayName); Assert.Equal(grpcSpan.SpanId, httpSpan.ParentSpanId); - Assert.NotEmpty(grpcSpan.Tags.Where(tag => tag.Key == "enriched")); - Assert.Equal(shouldEnrich ? "yes" : "no", grpcSpan.Tags.Where(tag => tag.Key == "enriched").FirstOrDefault().Value); + Assert.NotEmpty(grpcSpan.Tags.Where(tag => tag.Key == "enrichedWithHttpRequestMessage")); + Assert.NotEmpty(grpcSpan.Tags.Where(tag => tag.Key == "enrichedWithHttpResponseMessage")); + Assert.Equal(shouldEnrich ? "yes" : "no", grpcSpan.Tags.Where(tag => tag.Key == "enrichedWithHttpRequestMessage").FirstOrDefault().Value); + Assert.Equal(shouldEnrich ? "yes" : "no", grpcSpan.Tags.Where(tag => tag.Key == "enrichedWithHttpResponseMessage").FirstOrDefault().Value); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation(bool shouldEnrich) + [Fact] + public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() { var uri = new Uri($"http://localhost:{this.server.Port}"); var processor = new Mock>(); @@ -183,14 +203,7 @@ public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation(bool sho using (Sdk.CreateTracerProviderBuilder() .SetSampler(new AlwaysOnSampler()) - .AddGrpcClientInstrumentation(o => - { - o.SuppressDownstreamInstrumentation = true; - if (shouldEnrich) - { - o.Enrich = ActivityEnrichment; - } - }) + .AddGrpcClientInstrumentation(o => o.SuppressDownstreamInstrumentation = true) .AddHttpClientInstrumentation() .AddProcessor(processor.Object) .Build()) @@ -470,26 +483,6 @@ private static void ValidateGrpcActivity(Activity activityToValidate) Assert.Equal(ActivityKind.Client, activityToValidate.Kind); } - private static void ActivityEnrichment(Activity activity, string method, object obj) - { - Assert.True(activity.IsAllDataRequested); - switch (method) - { - case "OnStartActivity": - Assert.True(obj is HttpRequestMessage); - break; - - case "OnStopActivity": - Assert.True(obj is HttpResponseMessage); - break; - - default: - break; - } - - activity.SetTag("enriched", "yes"); - } - private static Predicate GeneratePredicateForMoqProcessorActivity(string methodName, string activityOperationName) { return invo => invo.Method.Name == methodName && (invo.Arguments[0] as Activity)?.OperationName == activityOperationName;