diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs index 5a518c8568..f26bf9225d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreInstrumentation.cs @@ -10,7 +10,7 @@ namespace OpenTelemetry.Instrumentation.AspNetCore; /// internal sealed class AspNetCoreInstrumentation : IDisposable { - internal static readonly Version SemanticConventionsVersion = new(1, 40, 0); + internal static readonly Version SemanticConventionsVersion = new(1, 42, 0); private static readonly HashSet DiagnosticSourceEvents = [ diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 4186d6c6f1..2c99263de3 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -21,6 +21,15 @@ of the Semantic Conventions for RPC/gRPC. ([#4370](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4370)) +* Update Semantic Conventions for RPC/gRPC to version + [1.42.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/README.md). + ([#4508](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4508)) + +* Fixed gRPC attributes being missing from the exported span when a sibling + activity is created because a non-default propagator (e.g. a custom or + composite propagator) is configured. + ([#4508](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4508)) + ## 1.15.2 Released 2026-Apr-21 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 5bda2c604b..7dbe40405d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -1,7 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System.Collections.Concurrent; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ -31,6 +30,19 @@ internal class HttpInListener : ListenerHandler private const string DiagnosticSourceName = "Microsoft.AspNetCore"; private const string CreatedByInstrumentationPropertyName = "OpenTelemetry.AspNetCore.CreatedByInstrumentation"; + private const string FrameworkActivityPropertyName = "OpenTelemetry.AspNetCore.FrameworkActivity"; + + // The gRPC .NET library adds these tags to the Activity created by ASP.NET Core + // (and not necessarily to Activity.Current). When the instrumentation creates a + // sibling Activity these tags must be copied from the original (framework) Activity. + // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1778 + private static readonly string[] GrpcSourceTagNames = + [ + GrpcTagHelper.GrpcMethodTagName, + GrpcTagHelper.GrpcStatusCodeTagName, + GrpcTagHelper.GrpcStatusTagName, + GrpcTagHelper.GrpcTargetTagName, + ]; private static readonly Func> HttpRequestHeaderValuesGetter = (request, name) => { @@ -46,10 +58,6 @@ internal class HttpInListener : ListenerHandler private static readonly PropertyFetcher ExceptionPropertyFetcher = new("Exception"); private static readonly object CreatedByInstrumentationMarker = new(); - // Caches the display name, rpc.service, and rpc.method derived from the raw gRPC method string. - // The set of distinct gRPC method strings is bounded by the number of gRPC endpoints in the app. - private static readonly GrpcMethodDetailsCache GrpcMethodCache = new(); - private readonly AspNetCoreTraceInstrumentationOptions options; private readonly bool nativeAspNetCoreOpenTelemetryEnabled; @@ -137,6 +145,12 @@ public void OnStartActivity(Activity activity, object? payload) newOne.SetCustomProperty(CreatedByInstrumentationPropertyName, CreatedByInstrumentationMarker); + // Keep a reference to the framework Activity. The gRPC .NET library may add + // its tags to that Activity rather than to Activity.Current, so they need to + // be copied onto the sibling Activity when it is stopped. + // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1778 + newOne.SetCustomProperty(FrameworkActivityPropertyName, activity); + // Starting the new activity make it the Activity.Current one. newOne.Start(); @@ -267,6 +281,13 @@ public void OnStopActivity(Activity activity, object? payload) activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode)); + // If the instrumentation created a sibling Activity, the gRPC .NET library may + // have added its grpc.* tags to the original (framework) Activity instead of to + // the sibling Activity that is exported. Copy them across so that gRPC requests + // are handled the same regardless of whether a sibling Activity was created. + // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1778 + CopyGrpcTagsFromFrameworkActivity(activity); + if (this.options.EnableGrpcAspNetCoreSupport && IsGrpcRequest(activity, out var grpcMethod)) { // Single pass over the tag collection to retrieve both gRPC tags, @@ -293,15 +314,12 @@ public void OnStopActivity(Activity activity, object? payload) } } - if (grpcMethod is { Length: > 0 }) - { - AddGrpcAttributes( - activity, - grpcMethod, - context, - grpcStatusCode, - hasGrpcStatusCode); - } + AddGrpcAttributes( + activity, + grpcMethod, + context, + grpcStatusCode, + hasGrpcStatusCode); } if (activity.Status == ActivityStatusCode.Unset) @@ -389,16 +407,15 @@ static bool TryFetchException(object? payload, [NotNullWhen(true)] out Exception [MethodImpl(MethodImplOptions.AggressiveInlining)] private static void AddGrpcAttributes( Activity activity, - string grpcMethod, + string? grpcMethod, HttpContext context, int grpcStatusCode, bool validStatusCode) { - var details = GrpcMethodCache.Get(grpcMethod); - // See the specs for semantic conventions. - // https://github.com/open-telemetry/semantic-conventions/blob/v1.41.0/docs/rpc/rpc-spans.md - activity.SetTag(SemanticConventions.AttributeRpcSystemName, GrpcTagHelper.RpcSystemGrpc); + // https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/rpc-spans.md + GrpcTagHelper.SetGrpcSystemName(activity); + GrpcTagHelper.SetGrpcMethodAndDisplayNameFromActivity(activity, grpcMethod); if (context.Connection.RemoteIpAddress != null) { @@ -414,30 +431,8 @@ private static void AddGrpcAttributes( activity.SetStatus(spanStatus); } - // https://github.com/open-telemetry/semantic-conventions/blob/v1.41.0/docs/rpc/grpc.md - if (details.IsParsed) - { - // The RPC semantic conventions indicate the span name should be rpc.method - // when it is available and not "_OTHER". - activity.DisplayName = details.DisplayName; - - // rpc.method is the fully-qualified logical method name, e.g. "package.Service/Method". - activity.SetTag(SemanticConventions.AttributeRpcMethod, details.DisplayName); - } - else - { - // The RPC semantic conventions indicate the span name should be rpc.system.name - // when rpc.method is "_OTHER". - activity.DisplayName = GrpcTagHelper.RpcSystemGrpc; - - // The method is not in the expected service/method form, so it is treated as unrecognized: - // rpc.method is set to "_OTHER" and the original value is preserved in rpc.method_original. - activity.SetTag(SemanticConventions.AttributeRpcMethod, GrpcTagHelper.RpcMethodOther); - activity.SetTag(SemanticConventions.AttributeRpcMethodOriginal, grpcMethod); - } - // The grpc.method tag has now been mapped to rpc.method, so the source tag can be removed. - // See https://github.com/open-telemetry/semantic-conventions/blob/v1.41.0/docs/non-normative/compatibility/grpc.md#attribute-mapping + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/non-normative/compatibility/grpc.md#attribute-mapping activity.SetTag(GrpcTagHelper.GrpcMethodTagName, null); activity.SetTag(GrpcTagHelper.GrpcTargetTagName, null); @@ -485,57 +480,30 @@ private static bool AspNetCoreHasNativeOpenTelemetryTags() return Net11OrGreater; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool IsGrpcRequest(Activity activity, [NotNullWhen(true)] out string? grpcMethod) - { - // gRPC-Web (https://learn.microsoft.com/aspnet/core/grpc/grpcweb) allows ASP.NET Core - // to support using gRPC from clients that do not support HTTP/2 or HTTP/3, so we - // can't just look at the HTTP protocol version to attempt to shortcut the test. - grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity); - return !string.IsNullOrEmpty(grpcMethod); - } - - private readonly struct GrpcMethodDetails + private static void CopyGrpcTagsFromFrameworkActivity(Activity activity) { - public GrpcMethodDetails(string displayName, string? rpcService, string? rpcMethod, bool isParsed) + // Only sibling activities created by the instrumentation have this property set. + if (activity.GetCustomProperty(FrameworkActivityPropertyName) is not Activity frameworkActivity) { - this.DisplayName = displayName; - this.RpcService = rpcService; - this.RpcMethod = rpcMethod; - this.IsParsed = isParsed; + return; } - public readonly string DisplayName { get; } - - public readonly string? RpcService { get; } - - public readonly string? RpcMethod { get; } - - public readonly bool IsParsed { get; } - } - - private sealed class GrpcMethodDetailsCache - { - private const int MaxCacheSize = 512; - private readonly ConcurrentDictionary cache = new(); - - public GrpcMethodDetails Get(string grpcMethod) + foreach (var tagName in GrpcSourceTagNames) { - if (this.cache.TryGetValue(grpcMethod, out var details)) + if (frameworkActivity.GetTagValue(tagName) is { } value) { - return details; + activity.SetTag(tagName, value); } - - // If the cache has reached its maximum size, just create a value without caching - return this.cache.Count >= MaxCacheSize ? Create(grpcMethod) : this.cache.GetOrAdd(grpcMethod, Create); } + } - private static GrpcMethodDetails Create(string method) - { - var displayName = method.Length > 0 && method[0] == '/' ? method.Substring(1) : method; - var isParsed = GrpcTagHelper.TryParseRpcServiceAndRpcMethod(method, out var serviceName, out var methodName); - - return new(displayName, isParsed ? serviceName : null, isParsed ? methodName : null, isParsed); - } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsGrpcRequest(Activity activity, [NotNullWhen(true)] out string? grpcMethod) + { + // gRPC-Web (https://learn.microsoft.com/aspnet/core/grpc/grpcweb) allows ASP.NET Core + // to support using gRPC from clients that do not support HTTP/2 or HTTP/3, so we + // can't just look at the HTTP protocol version to attempt to shortcut the test. + grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity); + return !string.IsNullOrEmpty(grpcMethod); } } diff --git a/src/OpenTelemetry.Instrumentation.GrpcCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.GrpcCore/CHANGELOG.md index 49e1f20e06..0076e05661 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.GrpcCore/CHANGELOG.md @@ -2,8 +2,9 @@ ## Unreleased -* **BREAKING**: Update to version 1.41.0 of the Semantic Conventions. - ([#4338](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4338)) +* **BREAKING**: Update to version 1.42.0 of the Semantic Conventions. + ([#4338](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4338), + [#4508](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4508)) * Add instrumentation scope version and schema URL to traces. ([#4338](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4338)) diff --git a/src/OpenTelemetry.Instrumentation.GrpcCore/GrpcCoreInstrumentation.cs b/src/OpenTelemetry.Instrumentation.GrpcCore/GrpcCoreInstrumentation.cs index 0a33e9368f..b0d71d8cc8 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcCore/GrpcCoreInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcCore/GrpcCoreInstrumentation.cs @@ -14,7 +14,7 @@ internal static class GrpcCoreInstrumentation /// /// Gets the version of the RPC Semantic Conventions used by the instrumentation. /// - internal static readonly Version SemanticConventionsVersion = new(1, 41, 0); + internal static readonly Version SemanticConventionsVersion = new(1, 42, 0); /// /// Gets the activity source for the instrumentation. diff --git a/src/OpenTelemetry.Instrumentation.GrpcCore/OpenTelemetry.Instrumentation.GrpcCore.csproj b/src/OpenTelemetry.Instrumentation.GrpcCore/OpenTelemetry.Instrumentation.GrpcCore.csproj index 41ce979cfb..7494564c73 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcCore/OpenTelemetry.Instrumentation.GrpcCore.csproj +++ b/src/OpenTelemetry.Instrumentation.GrpcCore/OpenTelemetry.Instrumentation.GrpcCore.csproj @@ -24,8 +24,11 @@ + + + diff --git a/src/OpenTelemetry.Instrumentation.GrpcCore/RpcScope.cs b/src/OpenTelemetry.Instrumentation.GrpcCore/RpcScope.cs index bf62eb5422..e35511074e 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcCore/RpcScope.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcCore/RpcScope.cs @@ -68,7 +68,7 @@ protected RpcScope( bool recordException) { this.host = host; - this.FullServiceName = fullServiceName?.TrimStart('/') ?? "unknownservice/unknownmethod"; + this.FullServiceName = fullServiceName?.Trim('/') ?? "unknownservice/unknownmethod"; this.recordMessageEvents = recordMessageEvents; this.recordException = recordException; } @@ -175,28 +175,13 @@ protected void SetActivity(Activity? activity) return; } - // Assign some reasonable defaults - var rpcService = this.FullServiceName; - var rpcMethod = this.FullServiceName; - - // Split the full service name by the slash - var parts = this.FullServiceName.Split('/'); - if (parts.Length == 2) - { - rpcService = parts[0]; - rpcMethod = parts[1]; - } - - this.activity.SetTag(SemanticConventions.AttributeRpcSystemName, "grpc"); - this.activity.SetTag(SemanticConventions.AttributeRpcService, rpcService); - this.activity.SetTag(SemanticConventions.AttributeRpcMethod, rpcMethod); + GrpcTagHelper.SetGrpcSystemName(this.activity); + GrpcTagHelper.SetGrpcMethodAndDisplayNameFromActivity(this.activity, this.FullServiceName); if (this.host is { Length: > 0 } host) { TrySetServerAttributes(this.activity, host); } - - this.activity.DisplayName = rpcMethod.Trim('/'); } private static void TrySetServerAttributes(Activity activity, string host) @@ -220,14 +205,34 @@ private static void TrySetServerAttributes(Activity activity, string host) /// /// The status code. /// If set to true [mark as completed]. - private void StopActivity(int statusCode, bool markAsCompleted = true) + /// The status description to set when the span is marked as failed, if any. + private void StopActivity(int statusCode, bool markAsCompleted = true, string? statusDescription = null) { if ((markAsCompleted && !this.TryMarkAsCompleted()) || this.activity is null) { return; } - this.activity.SetTag(SemanticConventions.AttributeRpcResponseStatusCode, statusCode); + var grpcStatusName = GrpcTagHelper.GetGrpcStatusCodeName(statusCode); + this.activity.SetTag(SemanticConventions.AttributeRpcResponseStatusCode, grpcStatusName); + + // Resolve the span status from the gRPC status code using the rules for the span kind: + // client spans treat every non-OK status as an error, whereas server spans only treat a + // subset of status codes as errors. + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/grpc.md + var spanStatus = this.activity.Kind == ActivityKind.Client + ? GrpcTagHelper.ResolveSpanStatusForGrpcStatusCodeOnClient(statusCode) + : GrpcTagHelper.ResolveSpanStatusForGrpcStatusCodeOnServer(statusCode); + + if (spanStatus == ActivityStatusCode.Error) + { + this.activity.SetStatus(spanStatus, statusDescription); + + // error.type is conditionally required when the operation failed; for gRPC it is set to + // the status code name. + this.activity.SetTag(SemanticConventions.AttributeErrorType, grpcStatusName); + } + this.activity.Stop(); } @@ -251,17 +256,15 @@ private void StopActivity(Exception exception) description = rpcException.Message; } - if (!string.IsNullOrEmpty(description)) - { - this.activity.SetStatus(ActivityStatusCode.Error, description); - } - if (this.activity.IsAllDataRequested && this.recordException) { this.activity.AddException(exception); } - this.StopActivity((int)grpcStatusCode, markAsCompleted: false); + // Defer to StopActivity to apply the span-kind specific status rules. The status description + // is only used when the resolved span status is an error so that, for example, server spans + // do not report an error status for status codes the conventions consider successful. + this.StopActivity((int)grpcStatusCode, markAsCompleted: false, statusDescription: description); } /// diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md index 99f60f6f14..8bf01a0131 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md @@ -2,8 +2,9 @@ ## Unreleased -* **BREAKING**: Update to version 1.41.0 of the Semantic Conventions. - ([#4338](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4338)) +* **BREAKING**: Update to version 1.42.0 of the Semantic Conventions. + ([#4338](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4338), + [#4508](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4508)) * Add instrumentation scope version and schema URL to traces. ([#4338](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4338)) @@ -11,6 +12,10 @@ * Updated OpenTelemetry core component version(s) to `1.16.0`. ([#4487](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4487)) +* Set the `error.type` attribute to the gRPC status code name on client spans when + the call fails. + ([#4508](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4508)) + ## 1.15.1-beta.1 Released 2026-Apr-21 diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs index 593a84e65a..5015be9b15 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs @@ -10,7 +10,7 @@ namespace OpenTelemetry.Instrumentation.GrpcNetClient.Implementation; internal sealed class GrpcClientDiagnosticListener : ListenerHandler { - internal static readonly Version SemanticConventionsVersion = new(1, 41, 0); + internal static readonly Version SemanticConventionsVersion = new(1, 42, 0); internal static readonly ActivitySource ActivitySource = ActivitySourceFactory.Create(SemanticConventionsVersion); private const string OnStartEvent = "Grpc.Net.Client.GrpcOut.Start"; @@ -107,23 +107,8 @@ public void OnStartActivity(Activity activity, object? payload) ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource); ActivityInstrumentationHelper.SetKindProperty(activity, ActivityKind.Client); - var grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity); - - activity.DisplayName = grpcMethod?.Trim('/') ?? GrpcTagHelper.RpcSystemGrpc; - - if (grpcMethod != null) - { - if (GrpcTagHelper.TryParseRpcServiceAndRpcMethod(grpcMethod, out var rpcService, out var rpcMethod)) - { - activity.SetTag(SemanticConventions.AttributeRpcService, rpcService); - activity.SetTag(SemanticConventions.AttributeRpcMethod, rpcMethod); - - // Remove the grpc.method tag added by the gRPC .NET library - activity.SetTag(GrpcTagHelper.GrpcMethodTagName, null); - } - } - - activity.SetTag(SemanticConventions.AttributeRpcSystemName, GrpcTagHelper.RpcSystemGrpc); + GrpcTagHelper.SetGrpcSystemName(activity); + GrpcTagHelper.SetGrpcMethodAndDisplayNameFromActivity(activity); var requestUri = request.RequestUri; @@ -175,12 +160,23 @@ public void OnStopActivity(Activity activity, object? payload) var validConversion = GrpcTagHelper.TryGetGrpcStatusCodeFromActivity(activity, out var status); if (validConversion) { + var spanStatus = GrpcTagHelper.ResolveSpanStatusForGrpcStatusCodeOnClient(status); if (activity.Status == ActivityStatusCode.Unset) { - activity.SetStatus(GrpcTagHelper.ResolveSpanStatusForGrpcStatusCodeOnClient(status)); + activity.SetStatus(spanStatus); } - activity.SetTag(SemanticConventions.AttributeRpcResponseStatusCode, status); + var grpcStatusName = GrpcTagHelper.GetGrpcStatusCodeName(status); + activity.SetTag(SemanticConventions.AttributeRpcResponseStatusCode, grpcStatusName); + + // error.type is conditionally required when the operation failed. For gRPC client + // spans all status codes other than OK are considered errors, and error.type is set + // to the status code name. + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/grpc.md + if (spanStatus == ActivityStatusCode.Error) + { + activity.SetTag(SemanticConventions.AttributeErrorType, grpcStatusName); + } } // Remove the grpc.status_code tag added by the gRPC .NET library diff --git a/src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj b/src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj index e915c8cb97..82cd80da12 100644 --- a/src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj +++ b/src/OpenTelemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj @@ -25,8 +25,8 @@ - + diff --git a/src/Shared/GrpcTagHelper.cs b/src/Shared/GrpcTagHelper.cs index ce38a05bc3..a1eae5f560 100644 --- a/src/Shared/GrpcTagHelper.cs +++ b/src/Shared/GrpcTagHelper.cs @@ -11,14 +11,19 @@ internal static class GrpcTagHelper { public const string RpcSystemGrpc = "grpc"; - // The value used for rpc.method when the gRPC method cannot be recognized as a - // fully-qualified service/method, in which case the original value is preserved in rpc.method_original. - // See https://github.com/open-telemetry/semantic-conventions/blob/v1.41.0/docs/rpc/grpc.md + // The value used for rpc.method when the gRPC method is not recognized, in which case + // the original value is preserved in rpc.method_original. + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/grpc.md public const string RpcMethodOther = "_OTHER"; + // The value used by the gRPC libraries for grpc.method when the method is not recognized. + // It maps to the "_OTHER" value used for rpc.method. + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/non-normative/compatibility/grpc.md#attribute-mapping + public const string GrpcMethodOther = "other"; + // The Grpc.Net.Client library adds its own tags to the activity. // These tags are used to source the tags added by the OpenTelemetry instrumentation. - // See https://github.com/open-telemetry/semantic-conventions/blob/v1.41.0/docs/non-normative/compatibility/grpc.md#attribute-mapping + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/non-normative/compatibility/grpc.md#attribute-mapping public const string GrpcMethodTagName = "grpc.method"; public const string GrpcStatusTagName = "grpc.status"; public const string GrpcStatusCodeTagName = "grpc.status_code"; @@ -27,35 +32,47 @@ internal static class GrpcTagHelper public static string? GetGrpcMethodFromActivity(Activity activity) => activity.GetTagValue(GrpcMethodTagName) as string; - public static bool TryGetGrpcStatusCodeFromActivity(Activity activity, out int statusCode) - { - statusCode = -1; - var grpcStatusCodeTag = activity.GetTagValue(GrpcStatusCodeTagName); - return grpcStatusCodeTag != null && - int.TryParse(grpcStatusCodeTag as string, NumberStyles.None, CultureInfo.InvariantCulture, out statusCode); - } + public static void SetGrpcSystemName(Activity activity) + => activity.SetTag(SemanticConventions.AttributeRpcSystemName, RpcSystemGrpc); - public static bool TryParseRpcServiceAndRpcMethod(string grpcMethod, out string rpcService, out string rpcMethod) + public static void SetGrpcMethodAndDisplayNameFromActivity(Activity activity, string? grpcMethod = null) { - var span = grpcMethod.AsSpan(); + grpcMethod ??= activity.GetTagValue(GrpcMethodTagName) as string; - if (!span.IsEmpty && span[0] is '/') + if (grpcMethod == null) { - span = span.Slice(1); + return; } - var lastSlash = span.LastIndexOf('/'); - if (lastSlash < 0) + var trimmedMethod = grpcMethod.Trim('/'); + + if (string.Equals(trimmedMethod, GrpcMethodOther, StringComparison.Ordinal)) + { + // The gRPC libraries use "other" when the method is not recognized. This maps to + // rpc.method "_OTHER" with the original value preserved in rpc.method_original, and the + // span is named after the RPC system as rpc.method is not a usable span name. + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/non-normative/compatibility/grpc.md#attribute-mapping + activity.DisplayName = RpcSystemGrpc; + activity.SetTag(SemanticConventions.AttributeRpcMethod, RpcMethodOther); + activity.SetTag(SemanticConventions.AttributeRpcMethodOriginal, trimmedMethod); + } + else { - rpcService = string.Empty; - rpcMethod = string.Empty; - return false; + // The RPC semantic conventions indicate the span name should be rpc.method when it is available. + activity.DisplayName = trimmedMethod; + activity.SetTag(SemanticConventions.AttributeRpcMethod, trimmedMethod); } - rpcService = span.Slice(0, lastSlash).ToString(); - rpcMethod = span.Slice(lastSlash + 1).ToString(); + // Remove the grpc.method tag added by the gRPC .NET library, if present. + activity.SetTag(GrpcMethodTagName, null); + } - return true; + public static bool TryGetGrpcStatusCodeFromActivity(Activity activity, out int statusCode) + { + statusCode = -1; + var grpcStatusCodeTag = activity.GetTagValue(GrpcStatusCodeTagName); + return grpcStatusCodeTag != null && + int.TryParse(grpcStatusCodeTag as string, NumberStyles.None, CultureInfo.InvariantCulture, out statusCode); } /// @@ -99,7 +116,7 @@ public static ActivityStatusCode ResolveSpanStatusForGrpcStatusCodeOnClient(int /// /// Helper method that populates span properties from RPC status code according - /// to https://github.com/open-telemetry/semantic-conventions/blob/v1.41.0/docs/rpc/grpc.md. + /// to https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/grpc.md. /// This method is for server spans where only specific status codes are considered errors: /// UNKNOWN, DEADLINE_EXCEEDED, UNIMPLEMENTED, INTERNAL, UNAVAILABLE, and DATA_LOSS. /// @@ -132,34 +149,31 @@ public static ActivityStatusCode ResolveSpanStatusForGrpcStatusCodeOnServer(int /// rpc.response.status_code and error.type attributes. /// /// - /// See https://github.com/grpc/grpc/blob/master/doc/statuscodes.md and - /// https://github.com/open-telemetry/semantic-conventions/blob/v1.41.0/docs/rpc/grpc.md. + /// See https://github.com/grpc/grpc/blob/v1.81.1/doc/statuscodes.md and + /// https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/grpc.md. /// /// The numeric gRPC status code. /// The canonical gRPC status code name (e.g. OK, DEADLINE_EXCEEDED), /// or the numeric value as a string if the code is not recognized. - public static string GetGrpcStatusCodeName(int statusCode) + public static string GetGrpcStatusCodeName(int statusCode) => statusCode switch { - return statusCode switch - { - (int)GrpcStatusCanonicalCode.Ok => "OK", - (int)GrpcStatusCanonicalCode.Cancelled => "CANCELLED", - (int)GrpcStatusCanonicalCode.Unknown => "UNKNOWN", - (int)GrpcStatusCanonicalCode.InvalidArgument => "INVALID_ARGUMENT", - (int)GrpcStatusCanonicalCode.DeadlineExceeded => "DEADLINE_EXCEEDED", - (int)GrpcStatusCanonicalCode.NotFound => "NOT_FOUND", - (int)GrpcStatusCanonicalCode.AlreadyExists => "ALREADY_EXISTS", - (int)GrpcStatusCanonicalCode.PermissionDenied => "PERMISSION_DENIED", - (int)GrpcStatusCanonicalCode.ResourceExhausted => "RESOURCE_EXHAUSTED", - (int)GrpcStatusCanonicalCode.FailedPrecondition => "FAILED_PRECONDITION", - (int)GrpcStatusCanonicalCode.Aborted => "ABORTED", - (int)GrpcStatusCanonicalCode.OutOfRange => "OUT_OF_RANGE", - (int)GrpcStatusCanonicalCode.Unimplemented => "UNIMPLEMENTED", - (int)GrpcStatusCanonicalCode.Internal => "INTERNAL", - (int)GrpcStatusCanonicalCode.Unavailable => "UNAVAILABLE", - (int)GrpcStatusCanonicalCode.DataLoss => "DATA_LOSS", - (int)GrpcStatusCanonicalCode.Unauthenticated => "UNAUTHENTICATED", - _ => statusCode.ToString(CultureInfo.InvariantCulture), - }; - } + (int)GrpcStatusCanonicalCode.Ok => "OK", + (int)GrpcStatusCanonicalCode.Cancelled => "CANCELLED", + (int)GrpcStatusCanonicalCode.Unknown => "UNKNOWN", + (int)GrpcStatusCanonicalCode.InvalidArgument => "INVALID_ARGUMENT", + (int)GrpcStatusCanonicalCode.DeadlineExceeded => "DEADLINE_EXCEEDED", + (int)GrpcStatusCanonicalCode.NotFound => "NOT_FOUND", + (int)GrpcStatusCanonicalCode.AlreadyExists => "ALREADY_EXISTS", + (int)GrpcStatusCanonicalCode.PermissionDenied => "PERMISSION_DENIED", + (int)GrpcStatusCanonicalCode.ResourceExhausted => "RESOURCE_EXHAUSTED", + (int)GrpcStatusCanonicalCode.FailedPrecondition => "FAILED_PRECONDITION", + (int)GrpcStatusCanonicalCode.Aborted => "ABORTED", + (int)GrpcStatusCanonicalCode.OutOfRange => "OUT_OF_RANGE", + (int)GrpcStatusCanonicalCode.Unimplemented => "UNIMPLEMENTED", + (int)GrpcStatusCanonicalCode.Internal => "INTERNAL", + (int)GrpcStatusCanonicalCode.Unavailable => "UNAVAILABLE", + (int)GrpcStatusCanonicalCode.DataLoss => "DATA_LOSS", + (int)GrpcStatusCanonicalCode.Unauthenticated => "UNAUTHENTICATED", + _ => statusCode.ToString(CultureInfo.InvariantCulture), + }; } diff --git a/test/OpenTelemetry.Contrib.Shared.Tests/GrpcTagHelperTests.cs b/test/OpenTelemetry.Contrib.Shared.Tests/GrpcTagHelperTests.cs index d262f11415..e044d8d0a1 100644 --- a/test/OpenTelemetry.Contrib.Shared.Tests/GrpcTagHelperTests.cs +++ b/test/OpenTelemetry.Contrib.Shared.Tests/GrpcTagHelperTests.cs @@ -21,18 +21,47 @@ public void GrpcTagHelper_GetGrpcMethodFromActivity() } [Theory] - [InlineData("Package.Service/Method", true, "Package.Service", "Method")] - [InlineData("/Package.Service/Method", true, "Package.Service", "Method")] - [InlineData("/ServiceWithNoPackage/Method", true, "ServiceWithNoPackage", "Method")] - [InlineData("/Some.Package.Service/Method", true, "Some.Package.Service", "Method")] - [InlineData("Invalid", false, "", "")] - public void GrpcTagHelper_TryParseRpcServiceAndRpcMethod(string grpcMethod, bool isSuccess, string expectedRpcService, string expectedRpcMethod) + [InlineData("/some.service/somemethod", "some.service/somemethod")] + [InlineData("some.service/somemethod", "some.service/somemethod")] + public void GrpcTagHelper_SetGrpcMethodAndDisplayNameFromActivity_RecognizedMethod(string grpcMethod, string expected) { - var success = GrpcTagHelper.TryParseRpcServiceAndRpcMethod(grpcMethod, out var rpcService, out var rpcMethod); + using var activity = new Activity("operationName"); + activity.SetTag(GrpcTagHelper.GrpcMethodTagName, grpcMethod); + + GrpcTagHelper.SetGrpcMethodAndDisplayNameFromActivity(activity); + + Assert.Equal(expected, activity.DisplayName); + Assert.Equal(expected, activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeRpcMethodOriginal)); + Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); + } + + [Theory] + [InlineData("other")] + [InlineData("/other")] + public void GrpcTagHelper_SetGrpcMethodAndDisplayNameFromActivity_UnrecognizedMethod(string grpcMethod) + { + using var activity = new Activity("operationName"); + activity.SetTag(GrpcTagHelper.GrpcMethodTagName, grpcMethod); + + GrpcTagHelper.SetGrpcMethodAndDisplayNameFromActivity(activity); - Assert.Equal(isSuccess, success); - Assert.Equal(expectedRpcService, rpcService); - Assert.Equal(expectedRpcMethod, rpcMethod); + Assert.Equal(GrpcTagHelper.RpcSystemGrpc, activity.DisplayName); + Assert.Equal(GrpcTagHelper.RpcMethodOther, activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); + Assert.Equal(GrpcTagHelper.GrpcMethodOther, activity.GetTagValue(SemanticConventions.AttributeRpcMethodOriginal)); + Assert.Null(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); + } + + [Fact] + public void GrpcTagHelper_SetGrpcMethodAndDisplayNameFromActivity_NoMethod() + { + using var activity = new Activity("operationName"); + + GrpcTagHelper.SetGrpcMethodAndDisplayNameFromActivity(activity); + + Assert.Equal("operationName", activity.DisplayName); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeRpcMethodOriginal)); } [Fact] @@ -112,5 +141,34 @@ public void GrpcTagHelper_GetGrpcStatusCodeFromEmptyActivity() Assert.False(validConversion); Assert.Equal(-1, status); Assert.Null(activity.GetTagValue(SemanticConventions.AttributeRpcGrpcStatusCode)); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); + } + + [Theory] + [InlineData(int.MinValue, "-2147483648")] + [InlineData(-1, "-1")] + [InlineData(0, "OK")] + [InlineData(1, "CANCELLED")] + [InlineData(2, "UNKNOWN")] + [InlineData(3, "INVALID_ARGUMENT")] + [InlineData(4, "DEADLINE_EXCEEDED")] + [InlineData(5, "NOT_FOUND")] + [InlineData(6, "ALREADY_EXISTS")] + [InlineData(7, "PERMISSION_DENIED")] + [InlineData(8, "RESOURCE_EXHAUSTED")] + [InlineData(9, "FAILED_PRECONDITION")] + [InlineData(10, "ABORTED")] + [InlineData(11, "OUT_OF_RANGE")] + [InlineData(12, "UNIMPLEMENTED")] + [InlineData(13, "INTERNAL")] + [InlineData(14, "UNAVAILABLE")] + [InlineData(15, "DATA_LOSS")] + [InlineData(16, "UNAUTHENTICATED")] + [InlineData(99, "99")] + [InlineData(int.MaxValue, "2147483647")] + public void GrpcTagHelper_ConvertStatusCodeToString(int statusCode, string expected) + { + var actual = GrpcTagHelper.GetGrpcStatusCodeName(statusCode); + Assert.Equal(expected, actual); } } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/GrpcTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/GrpcTests.cs index 6da2616c90..dd905db1dd 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/GrpcTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/GrpcTests.cs @@ -39,33 +39,6 @@ public void OnStopActivityAddsGrpcAttributesForParsedMethodAndValidStatusCode() AssertTag(activity, SemanticConventions.AttributeErrorType, "INTERNAL"); } - [Fact] - public void OnStopActivitySetsRpcMethodToOtherWhenMethodCannotBeParsed() - { - // Arrange - var listener = CreateListener(); - using var activity = CreateActivity("Invalid", "invalid"); - var context = CreateContext(remoteIpAddress: null, remotePort: 4317); - - // Act - listener.OnStopActivity(activity, context); - - // Assert - Assert.Equal("grpc", activity.DisplayName); - Assert.Equal(ActivityStatusCode.Unset, activity.Status); - - AssertTag(activity, GrpcTagHelper.GrpcMethodTagName, null); - AssertTag(activity, GrpcTagHelper.GrpcStatusCodeTagName, "invalid"); - AssertTag(activity, SemanticConventions.AttributeNetworkPeerAddress, null); - AssertTag(activity, SemanticConventions.AttributeNetworkPeerPort, 4317); - AssertTag(activity, SemanticConventions.AttributeRpcGrpcStatusCode, null); - AssertTag(activity, SemanticConventions.AttributeRpcMethod, "_OTHER"); - AssertTag(activity, SemanticConventions.AttributeRpcMethodOriginal, "Invalid"); - AssertTag(activity, SemanticConventions.AttributeRpcResponseStatusCode, null); - AssertTag(activity, SemanticConventions.AttributeRpcService, null); - AssertTag(activity, SemanticConventions.AttributeRpcSystemName, "grpc"); - } - [Fact] public void OnStopActivityIgnoresEmptyGrpcMethodTag() { diff --git a/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs b/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs index e11d146142..c642ba6f94 100644 --- a/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs +++ b/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreClientInterceptorTests.cs @@ -14,7 +14,7 @@ namespace OpenTelemetry.Instrumentation.GrpcCore.Tests; /// /// Grpc Core client interceptor tests. /// -public class GrpcCoreClientInterceptorTests +public class GrpcCoreClientInterceptorTests(WeaverFixture weaver, ITestOutputHelper outputHelper) : IClassFixture { /// /// A bogus server uri. @@ -32,7 +32,7 @@ public class GrpcCoreClientInterceptorTests /// A task. [Fact] public async Task AsyncUnarySuccess() => - await TestHandlerSuccess( + await this.TestHandlerSuccess( FoobarService.MakeUnaryAsyncRequest, FoobarService.UnaryMethod, DefaultMetadataFunc()); @@ -82,7 +82,7 @@ static void MakeRequest(Foobar.FoobarClient client) /// A task. [Fact] public async Task ClientStreamingSuccess() => - await TestHandlerSuccess(FoobarService.MakeClientStreamingRequest, FoobarService.ClientStreamingMethod, DefaultMetadataFunc()); + await this.TestHandlerSuccess(FoobarService.MakeClientStreamingRequest, FoobarService.ClientStreamingMethod, DefaultMetadataFunc()); /// /// Validates a failed ClientStreaming call when the service is unavailable. @@ -125,7 +125,7 @@ static void MakeRequest(Foobar.FoobarClient client) /// A task. [Fact] public async Task ServerStreamingSuccess() => - await TestHandlerSuccess(FoobarService.MakeServerStreamingRequest, FoobarService.ServerStreamingMethod, DefaultMetadataFunc()); + await this.TestHandlerSuccess(FoobarService.MakeServerStreamingRequest, FoobarService.ServerStreamingMethod, DefaultMetadataFunc()); /// /// Validates a failed ServerStreaming call. @@ -155,7 +155,7 @@ static void MakeRequest(Foobar.FoobarClient client) /// A task. [Fact] public async Task DuplexStreamingSuccess() => - await TestHandlerSuccess(FoobarService.MakeDuplexStreamingRequest, FoobarService.DuplexStreamingMethod, DefaultMetadataFunc()); + await this.TestHandlerSuccess(FoobarService.MakeDuplexStreamingRequest, FoobarService.DuplexStreamingMethod, DefaultMetadataFunc()); /// /// Validates a failed DuplexStreaming call when the service is unavailable. @@ -343,19 +343,35 @@ internal static void ValidateCommonActivityTags( Assert.True(activity.IsStopped, "The activity has not been stopped."); - Assert.Equal(expectedMethodName, activity.DisplayName); + var expectedRpcMethod = $"OpenTelemetry.Instrumentation.GrpcCore.Tests.Foobar/{expectedMethodName}"; + var expectedResponseStatusCode = GrpcTagHelper.GetGrpcStatusCodeName((int)expectedStatusCode); + + Assert.Equal(expectedRpcMethod, activity.DisplayName); // TagObjects contain non string values // Tags contains only string values Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeRpcSystemName && (string?)t.Value == "grpc"); - Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeRpcService && (string?)t.Value == "OpenTelemetry.Instrumentation.GrpcCore.Tests.Foobar"); - Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeRpcMethod && (string?)t.Value == expectedMethodName); - Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeRpcResponseStatusCode && (int?)t.Value == (int)expectedStatusCode); - - // Cancelled is not an error. - if (expectedStatusCode is not StatusCode.OK and not StatusCode.Cancelled) + Assert.DoesNotContain(activity.TagObjects, t => t.Key == SemanticConventions.AttributeRpcService); + Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeRpcMethod && (string?)t.Value == expectedRpcMethod); + Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeRpcResponseStatusCode && (string?)t.Value == expectedResponseStatusCode); + + // Client spans treat every non-OK status as an error, whereas server spans only treat a + // subset of status codes as errors. + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/grpc.md + var expectedStatus = activity.Kind == ActivityKind.Client + ? GrpcTagHelper.ResolveSpanStatusForGrpcStatusCodeOnClient((int)expectedStatusCode) + : GrpcTagHelper.ResolveSpanStatusForGrpcStatusCodeOnServer((int)expectedStatusCode); + + if (expectedStatus == ActivityStatusCode.Error) { + // A failed span has error.type set to the status code name. Assert.Equal(ActivityStatusCode.Error, activity.Status); + Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeErrorType && (string?)t.Value == expectedResponseStatusCode); + } + else + { + Assert.NotEqual(ActivityStatusCode.Error, activity.Status); + Assert.DoesNotContain(activity.TagObjects, t => t.Key == SemanticConventions.AttributeErrorType); } if (recordedMessages) @@ -397,6 +413,102 @@ static void ValidateCommonEventAttributes(ActivityEvent activityEvent) } } + /// + /// Tests basic handler failure. Instructs the server to fail with resources exhausted and validates the created Activity. + /// + /// The client request function. + /// The expected gRPC method name. + /// The status code to use for the failure. Defaults to ResourceExhausted. + /// if set to true [validate error description]. + /// An alternate server URI string. + /// + /// A Task. + /// + private static async Task TestHandlerFailure( + Func clientRequestFunc, + string expectedMethodName, + StatusCode statusCode = StatusCode.ResourceExhausted, + bool validateErrorDescription = true, + string? serverUriString = null) + { + var testTags = new TestActivityTags(); + var interceptorOptions = new ClientTracingInterceptorOptions + { + Propagator = new TraceContextPropagator(), + AdditionalTags = testTags.Tags, + RecordException = true, + }; + + using var activityListener = new InterceptorActivityListener(testTags); + + using (var server = FoobarService.Start()) + { + var client = FoobarService.ConstructRpcClient( + serverUriString ?? server.Target, + new ClientTracingInterceptor(interceptorOptions), + [ + new(FoobarService.RequestHeaderFailWithStatusCode, statusCode.ToString()), + new(FoobarService.RequestHeaderErrorDescription, "fubar") + ]); + + await Assert.ThrowsAsync(() => clientRequestFunc(client, null)); + } + + var activity = activityListener.Activity; + + ValidateCommonActivityTags( + activity, + expectedMethodName, + statusCode, + interceptorOptions.RecordMessageEvents, + interceptorOptions.RecordException); + + if (validateErrorDescription) + { + Assert.NotNull(activity); + Assert.Contains("fubar", activity.StatusDescription); + } + } + + /// + /// Tests for Activity cancellation when the handler is disposed before completing the RPC. + /// + /// The client request action. + /// The expected gRPC method name. + private void TestActivityIsCancelledWhenHandlerDisposed( + Action clientRequestAction, + string expectedMethodName) + { + var testTags = new TestActivityTags(); + using var activityListener = new InterceptorActivityListener(testTags); + + using (var server = FoobarService.Start()) + { + var clientInterceptorOptions = new ClientTracingInterceptorOptions + { + Propagator = new TraceContextPropagator(), + AdditionalTags = testTags.Tags, + }; + + var client = FoobarService.ConstructRpcClient(server.Target, new ClientTracingInterceptor(clientInterceptorOptions)); + clientRequestAction(client); + } + + // The activity is stopped asynchronously once the call is cancelled, so wait + // for it to be stopped before validating to avoid reading it prematurely. + Assert.True( + SpinWait.SpinUntil( + () => activityListener.Activity?.IsStopped == true, + TimeSpan.FromSeconds(5)), + "The activity was not stopped within the timeout."); + + ValidateCommonActivityTags( + activityListener.Activity, + expectedMethodName, + StatusCode.Cancelled, + false); + } + /// /// Tests basic handler success. /// @@ -404,7 +516,7 @@ static void ValidateCommonEventAttributes(ActivityEvent activityEvent) /// The expected gRPC method name. /// The additional metadata, if any. /// A Task. - private static async Task TestHandlerSuccess( + private async Task TestHandlerSuccess( Func clientRequestFunc, string expectedMethodName, Metadata additionalMetadata) @@ -523,102 +635,16 @@ private static async Task TestHandlerSuccess( Assert.Equal(parentActivity.Id, activity.ParentId); Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeServerAddress && (string?)t.Value == server.HostName); Assert.Contains(activity.TagObjects, t => t.Key == SemanticConventions.AttributeServerPort && (int?)t.Value == server.Port); - } - } - /// - /// Tests basic handler failure. Instructs the server to fail with resources exhausted and validates the created Activity. - /// - /// The client request function. - /// The expected gRPC method name. - /// The status code to use for the failure. Defaults to ResourceExhausted. - /// if set to true [validate error description]. - /// An alternate server URI string. - /// - /// A Task. - /// - private static async Task TestHandlerFailure( - Func clientRequestFunc, - string expectedMethodName, - StatusCode statusCode = StatusCode.ResourceExhausted, - bool validateErrorDescription = true, - string? serverUriString = null) - { - var testTags = new TestActivityTags(); - var interceptorOptions = new ClientTracingInterceptorOptions - { - Propagator = new TraceContextPropagator(), - AdditionalTags = testTags.Tags, - RecordException = true, - }; - - using var activityListener = new InterceptorActivityListener(testTags); - - using (var server = FoobarService.Start()) - { - var client = FoobarService.ConstructRpcClient( - serverUriString ?? server.Target, - new ClientTracingInterceptor(interceptorOptions), - [ - new(FoobarService.RequestHeaderFailWithStatusCode, statusCode.ToString()), - new(FoobarService.RequestHeaderErrorDescription, "fubar") - ]); - - await Assert.ThrowsAsync(() => clientRequestFunc(client, null)); - } - - var activity = activityListener.Activity; - - ValidateCommonActivityTags( - activity, - expectedMethodName, - statusCode, - interceptorOptions.RecordMessageEvents, - interceptorOptions.RecordException); - - if (validateErrorDescription) - { - Assert.NotNull(activity); - Assert.Contains("fubar", activity.StatusDescription); - } - } - - /// - /// Tests for Activity cancellation when the handler is disposed before completing the RPC. - /// - /// The client request action. - /// The expected gRPC method name. - private void TestActivityIsCancelledWhenHandlerDisposed( - Action clientRequestAction, - string expectedMethodName) - { - var testTags = new TestActivityTags(); - using var activityListener = new InterceptorActivityListener(testTags); - - using (var server = FoobarService.Start()) - { - var clientInterceptorOptions = new ClientTracingInterceptorOptions + if (DockerHelper.IsAvailable(DockerPlatform.Linux)) { - Propagator = new TraceContextPropagator(), - AdditionalTags = testTags.Tags, - }; - - var client = FoobarService.ConstructRpcClient(server.Target, new ClientTracingInterceptor(clientInterceptorOptions)); - clientRequestAction(client); + await WeaverTelemetryVerifier.VerifyAsync( + ([activity], []), + GrpcCoreInstrumentation.SemanticConventionsVersion, + weaver, + outputHelper, + [new("missing_attribute", "Attribute 'activityidentifier' does not exist in the registry.")]); + } } - - // The activity is stopped asynchronously once the call is cancelled, so wait - // for it to be stopped before validating to avoid reading it prematurely. - Assert.True( - SpinWait.SpinUntil( - () => activityListener.Activity?.IsStopped == true, - TimeSpan.FromSeconds(5)), - "The activity was not stopped within the timeout."); - - ValidateCommonActivityTags( - activityListener.Activity, - expectedMethodName, - StatusCode.Cancelled, - false); } } diff --git a/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreServerInterceptorTests.cs b/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreServerInterceptorTests.cs index d9644bdb3f..2bad136e2e 100644 --- a/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreServerInterceptorTests.cs +++ b/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/GrpcCoreServerInterceptorTests.cs @@ -4,13 +4,15 @@ using Grpc.Core; using Grpc.Core.Interceptors; using OpenTelemetry.Context.Propagation; +using OpenTelemetry.Tests; namespace OpenTelemetry.Instrumentation.GrpcCore.Tests; /// /// Grpc Core server interceptor tests. /// -public class GrpcCoreServerInterceptorTests +public class GrpcCoreServerInterceptorTests(WeaverFixture weaver, ITestOutputHelper outputHelper) + : IClassFixture { /// /// Validates a successful UnaryServerHandler call. @@ -18,7 +20,7 @@ public class GrpcCoreServerInterceptorTests /// A task. [Fact] public async Task UnaryServerHandlerSuccess() => - await TestHandlerSuccess(FoobarService.MakeUnaryAsyncRequest, FoobarService.UnaryMethod); + await this.TestHandlerSuccess(FoobarService.MakeUnaryAsyncRequest, FoobarService.UnaryMethod); /// /// Validates a failed UnaryServerHandler call. @@ -34,7 +36,7 @@ public async Task UnaryServerHandlerFail() => /// A task. [Fact] public async Task ClientStreamingServerHandlerSuccess() => - await TestHandlerSuccess(FoobarService.MakeClientStreamingRequest, FoobarService.ClientStreamingMethod); + await this.TestHandlerSuccess(FoobarService.MakeClientStreamingRequest, FoobarService.ClientStreamingMethod); /// /// Validates a failed ClientStreamingServerHandler call. @@ -50,7 +52,7 @@ public async Task ClientStreamingServerHandlerFail() => /// A task. [Fact] public async Task ServerStreamingServerHandlerSuccess() => - await TestHandlerSuccess(FoobarService.MakeServerStreamingRequest, FoobarService.ServerStreamingMethod); + await this.TestHandlerSuccess(FoobarService.MakeServerStreamingRequest, FoobarService.ServerStreamingMethod); /// /// Validates a failed ServerStreamingServerHandler call. @@ -66,7 +68,7 @@ public async Task ServerStreamingServerHandlerFail() => /// A task. [Fact] public async Task DuplexStreamingServerHandlerSuccess() => - await TestHandlerSuccess(FoobarService.MakeDuplexStreamingRequest, FoobarService.DuplexStreamingMethod); + await this.TestHandlerSuccess(FoobarService.MakeDuplexStreamingRequest, FoobarService.DuplexStreamingMethod); /// /// Validates a failed DuplexStreamingServerHandler call. @@ -144,6 +146,48 @@ static Task HandleUnaryCall(NonProtobufPayload request, Serv } } + /// + /// A common method to test server interceptor handler failure. + /// + /// The specific client request function. + /// The expected gRPC method name. + /// The additional metadata, if any. + /// A Task. + private static async Task TestHandlerFailure( + Func clientRequestFunc, + string expectedMethodName, + Metadata? additionalMetadata = null) + { + // starts the server with the server interceptor + var testTags = new TestActivityTags(); + var interceptorOptions = new ServerTracingInterceptorOptions { Propagator = new TraceContextPropagator(), AdditionalTags = testTags.Tags, RecordException = true }; + using var server = FoobarService.Start(new ServerTracingInterceptor(interceptorOptions)); + + using var activityListener = new InterceptorActivityListener(testTags); + var client = FoobarService.ConstructRpcClient( + server.Target, + additionalMetadata: + [ + new Metadata.Entry("traceparent", FoobarService.DefaultTraceparentWithSampling), + new Metadata.Entry(FoobarService.RequestHeaderFailWithStatusCode, StatusCode.ResourceExhausted.ToString()), + new Metadata.Entry(FoobarService.RequestHeaderErrorDescription, "fubar"), + ]); + + await Assert.ThrowsAsync(async () => await clientRequestFunc(client, additionalMetadata).ConfigureAwait(false)); + + var activity = activityListener.Activity; + + GrpcCoreClientInterceptorTests.ValidateCommonActivityTags( + activity, + expectedMethodName, + StatusCode.ResourceExhausted, + interceptorOptions.RecordMessageEvents, + interceptorOptions.RecordException); + + Assert.NotNull(activity); + Assert.Equal(FoobarService.DefaultParentFromTraceparentHeader.SpanId, activity.ParentSpanId); + } + /// /// A common method to test server interceptor handler success. /// @@ -151,7 +195,7 @@ static Task HandleUnaryCall(NonProtobufPayload request, Serv /// The expected gRPC method name. /// The additional metadata, if any. /// A Task. - private static async Task TestHandlerSuccess( + private async Task TestHandlerSuccess( Func clientRequestFunc, string expectedMethodName, Metadata? additionalMetadata = null) @@ -204,48 +248,16 @@ private static async Task TestHandlerSuccess( Assert.NotNull(activity); Assert.Equal(FoobarService.DefaultParentFromTraceparentHeader.SpanId, activity.ParentSpanId); - } - } - /// - /// A common method to test server interceptor handler failure. - /// - /// The specific client request function. - /// The expected gRPC method name. - /// The additional metadata, if any. - /// A Task. - private static async Task TestHandlerFailure( - Func clientRequestFunc, - string expectedMethodName, - Metadata? additionalMetadata = null) - { - // starts the server with the server interceptor - var testTags = new TestActivityTags(); - var interceptorOptions = new ServerTracingInterceptorOptions { Propagator = new TraceContextPropagator(), AdditionalTags = testTags.Tags, RecordException = true }; - using var server = FoobarService.Start(new ServerTracingInterceptor(interceptorOptions)); - - using var activityListener = new InterceptorActivityListener(testTags); - var client = FoobarService.ConstructRpcClient( - server.Target, - additionalMetadata: - [ - new Metadata.Entry("traceparent", FoobarService.DefaultTraceparentWithSampling), - new Metadata.Entry(FoobarService.RequestHeaderFailWithStatusCode, StatusCode.ResourceExhausted.ToString()), - new Metadata.Entry(FoobarService.RequestHeaderErrorDescription, "fubar"), - ]); - - await Assert.ThrowsAsync(async () => await clientRequestFunc(client, additionalMetadata).ConfigureAwait(false)); - - var activity = activityListener.Activity; - - GrpcCoreClientInterceptorTests.ValidateCommonActivityTags( - activity, - expectedMethodName, - StatusCode.ResourceExhausted, - interceptorOptions.RecordMessageEvents, - interceptorOptions.RecordException); - - Assert.NotNull(activity); - Assert.Equal(FoobarService.DefaultParentFromTraceparentHeader.SpanId, activity.ParentSpanId); + if (DockerHelper.IsAvailable(DockerPlatform.Linux)) + { + await WeaverTelemetryVerifier.VerifyAsync( + ([activity], []), + GrpcCoreInstrumentation.SemanticConventionsVersion, + weaver, + outputHelper, + [new("missing_attribute", "Attribute 'activityidentifier' does not exist in the registry.")]); + } + } } } diff --git a/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/OpenTelemetry.Instrumentation.GrpcCore.Tests.csproj b/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/OpenTelemetry.Instrumentation.GrpcCore.Tests.csproj index 8428d96538..d1f00a0401 100644 --- a/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/OpenTelemetry.Instrumentation.GrpcCore.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.GrpcCore.Tests/OpenTelemetry.Instrumentation.GrpcCore.Tests.csproj @@ -8,6 +8,12 @@ + + + + + + @@ -19,7 +25,17 @@ + + + + + + + + + + diff --git a/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.client.cs b/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.client.cs index 1323ff8f8a..6d6ec7e541 100644 --- a/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.client.cs +++ b/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.client.cs @@ -11,15 +11,18 @@ using Microsoft.Extensions.DependencyInjection; #if !NETFRAMEWORK using OpenTelemetry.Context.Propagation; -using OpenTelemetry.Tests; #endif using OpenTelemetry.Instrumentation.Grpc.Tests.GrpcTestHelpers; using OpenTelemetry.Instrumentation.GrpcNetClient; +using OpenTelemetry.Instrumentation.GrpcNetClient.Implementation; +using OpenTelemetry.Tests; using OpenTelemetry.Trace; +using RpcException = Grpc.Core.RpcException; namespace OpenTelemetry.Instrumentation.Grpc.Tests; -public partial class GrpcTests +public partial class GrpcTests(WeaverFixture weaver, ITestOutputHelper outputHelper) + : IClassFixture { [Theory] [InlineData("http://localhost")] @@ -28,7 +31,7 @@ public partial class GrpcTests [InlineData("http://127.0.0.1", false)] [InlineData("http://[::1]")] [InlineData("http://[::1]", false)] - public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool shouldEnrich = true) + public async Task GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool shouldEnrich = true) { var enrichWithHttpRequestMessageCalled = false; var enrichWithHttpResponseMessageCalled = false; @@ -81,8 +84,7 @@ public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool sho Assert.Equal($"greet.Greeter/SayHello", activity.DisplayName); Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystemName)); - Assert.Equal("greet.Greeter", activity.GetTagValue(SemanticConventions.AttributeRpcService)); - Assert.Equal("SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); + Assert.Equal("greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); Assert.Equal(uri.Host, activity.GetTagValue(SemanticConventions.AttributeServerAddress)); Assert.Equal(uri.Port, activity.GetTagValue(SemanticConventions.AttributeServerPort)); @@ -91,13 +93,114 @@ public void GrpcClientCallsAreCollectedSuccessfully(string baseAddress, bool sho // 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.AttributeRpcResponseStatusCode)); + Assert.Equal("OK", activity.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); if (shouldEnrich) { Assert.True(enrichWithHttpRequestMessageCalled); Assert.True(enrichWithHttpResponseMessageCalled); } + + if (DockerHelper.IsAvailable(DockerPlatform.Linux)) + { + await WeaverTelemetryVerifier.VerifyAsync( + (exportedItems, []), + GrpcClientDiagnosticListener.SemanticConventionsVersion, + weaver, + outputHelper); + } + } + + [Fact] + public void GrpcClientCancelledCallIsRecordedAsErrorPerSemanticConventions() + { + // The gRPC semantic conventions specify that, for client spans, all status codes + // other than OK are errors. A cancelled call (status code CANCELLED) therefore has + // its span status set to Error and the error.type attribute set to the status code name. + // See https://github.com/open-telemetry/semantic-conventions/blob/v1.42.0/docs/rpc/grpc.md + var uri = new UriBuilder("http://localhost") { Port = 1234 }.Uri; + + using var httpClient = ClientTestHelpers.CreateTestClient(async request => + { + var streamContent = await ClientTestHelpers.CreateResponseContent(new HelloReply()); + return ResponseUtils.CreateResponse(HttpStatusCode.OK, streamContent, grpcStatusCode: global::Grpc.Core.StatusCode.Cancelled); + }); + + var exportedItems = new List(); + + using var parent = new Activity("parent") + .SetIdFormat(ActivityIdFormat.W3C) + .Start(); + + using (Sdk.CreateTracerProviderBuilder() + .SetSampler(new AlwaysOnSampler()) + .AddGrpcClientInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build()) + { + var channel = GrpcChannel.ForAddress(uri, new GrpcChannelOptions + { + HttpClient = httpClient, + }); + var client = new Greeter.GreeterClient(channel); + Assert.Throws(() => client.SayHello(new HelloRequest())); + } + + var activity = Assert.Single(exportedItems); + + Assert.Equal("CANCELLED", activity.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); + Assert.Equal(ActivityStatusCode.Error, activity.Status); + Assert.Equal("CANCELLED", activity.GetTagValue(SemanticConventions.AttributeErrorType)); + } + + [Fact] + public void GrpcClientCancellationCanBeTreatedAsNonErrorWithCustomProcessor() + { + // The gRPC semantic conventions classify all non-OK client status codes as errors. + // An application that deliberately cancels a call has additional context and MAY + // override the span status to not report the cancellation as an error. This test + // demonstrates how to do that using a custom processor. + // + // Note that the EnrichWithHttpResponseMessage option cannot be used for this purpose + // because it is not invoked when a call is cancelled (there is no response message). + // See https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/2066 + var uri = new UriBuilder("http://localhost") { Port = 1234 }.Uri; + + using var httpClient = ClientTestHelpers.CreateTestClient(async request => + { + var streamContent = await ClientTestHelpers.CreateResponseContent(new HelloReply()); + return ResponseUtils.CreateResponse(HttpStatusCode.OK, streamContent, grpcStatusCode: global::Grpc.Core.StatusCode.Cancelled); + }); + + var exportedItems = new List(); + + using var parent = new Activity("parent") + .SetIdFormat(ActivityIdFormat.W3C) + .Start(); + + using (Sdk.CreateTracerProviderBuilder() + .SetSampler(new AlwaysOnSampler()) + .AddGrpcClientInstrumentation() + .AddProcessor(new CancelledGrpcCallStatusProcessor()) + .AddInMemoryExporter(exportedItems) + .Build()) + { + var channel = GrpcChannel.ForAddress(uri, new GrpcChannelOptions + { + HttpClient = httpClient, + }); + var client = new Greeter.GreeterClient(channel); + Assert.Throws(() => client.SayHello(new HelloRequest())); + } + + var activity = Assert.Single(exportedItems); + + // The status code attribute is still recorded as required by the semantic conventions... + Assert.Equal("CANCELLED", activity.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); + + // ...but the application's processor has cleared the error signal. + Assert.Equal(ActivityStatusCode.Unset, activity.Status); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeErrorType)); } #if NET @@ -147,7 +250,7 @@ public void GrpcAndHttpClientInstrumentationIsInvoked(bool shouldEnrich) ValidateGrpcActivity(grpcSpan); Assert.Equal($"greet.Greeter/SayHello", grpcSpan.DisplayName); - Assert.Equal(0, grpcSpan.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); + Assert.Equal("OK", grpcSpan.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); Assert.Equal("POST", httpSpan.DisplayName); Assert.Equal(grpcSpan.SpanId, httpSpan.ParentSpanId); @@ -200,26 +303,22 @@ public void GrpcAndHttpClientInstrumentationWithSuppressInstrumentation() ValidateGrpcActivity(grpcSpan1); Assert.Equal($"greet.Greeter/SayHello", grpcSpan1.DisplayName); - Assert.Equal(0, grpcSpan1.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); + Assert.Equal("OK", grpcSpan1.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); ValidateGrpcActivity(grpcSpan2); Assert.Equal($"greet.Greeter/SayHello", grpcSpan2.DisplayName); - Assert.Equal(0, grpcSpan2.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); + Assert.Equal("OK", grpcSpan2.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); ValidateGrpcActivity(grpcSpan3); Assert.Equal($"greet.Greeter/SayHello", grpcSpan3.DisplayName); - Assert.Equal(0, grpcSpan3.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); + Assert.Equal("OK", grpcSpan3.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); ValidateGrpcActivity(grpcSpan4); Assert.Equal($"greet.Greeter/SayHello", grpcSpan4.DisplayName); - Assert.Equal(0, grpcSpan4.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); + Assert.Equal("OK", grpcSpan4.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); } -#if NET [Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1727")] -#else - [Fact] -#endif public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() { try @@ -267,9 +366,9 @@ public void GrpcPropagatesContextWithSuppressInstrumentationOptionSetToTrue() Assert.Equal($"greet.Greeter/SayHello", clientActivity.DisplayName); Assert.Equal($"POST /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("OK", clientActivity.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); Assert.Equal("customValue", serverActivity.GetCustomProperty("customField") as string); + Assert.Equal(clientActivity.SpanId, serverActivity.ParentSpanId); } finally { @@ -413,4 +512,19 @@ private static void ValidateGrpcActivity(Activity activityToValidate) Assert.Equal(ActivityKind.Client, activityToValidate.Kind); Assert.StartsWith("https://opentelemetry.io/schemas/", activityToValidate.Source.TelemetrySchemaUrl); } + + private sealed class CancelledGrpcCallStatusProcessor : BaseProcessor + { + public override void OnEnd(Activity activity) + { + // The application knows a CANCELLED result is the result of a deliberate + // cancellation rather than a failure, so the error signal is cleared while + // leaving the rpc.response.status_code attribute in place. + if ((activity.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode) as string) == "CANCELLED") + { + activity.SetStatus(ActivityStatusCode.Unset); + activity.SetTag(SemanticConventions.AttributeErrorType, null); + } + } + } } diff --git a/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.server.cs b/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.server.cs index 7c3985f51b..59f10d3471 100644 --- a/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.server.cs +++ b/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/GrpcTests.server.cs @@ -11,6 +11,8 @@ using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Context.Propagation; using OpenTelemetry.Instrumentation.Grpc.Services.Tests; +using OpenTelemetry.Instrumentation.GrpcNetClient.Implementation; +using OpenTelemetry.Tests; using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation.Grpc.Tests; @@ -39,7 +41,7 @@ public async Task DisposeAsync() [InlineData("false")] [InlineData("True")] [InlineData("False")] - public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(string? enableGrpcAspNetCoreSupport) + public async Task GrpcAspNetCoreInstrumentationAddsCorrectAttributes(string? enableGrpcAspNetCoreSupport) { var configuration = new ConfigurationBuilder() .AddInMemoryCollection(new Dictionary @@ -74,19 +76,24 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(string? enableGrp Assert.Equal(ActivityKind.Server, activity.Kind); - if (enableGrpcAspNetCoreSupport != null && enableGrpcAspNetCoreSupport.Equals("true", StringComparison.OrdinalIgnoreCase)) + var aspNetCoreSupportEnabled = + enableGrpcAspNetCoreSupport != null && + string.Equals(enableGrpcAspNetCoreSupport, "true", StringComparison.OrdinalIgnoreCase); + + if (aspNetCoreSupportEnabled) { - 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.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystemName)); + Assert.Equal("greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); + Assert.Equal("greet.Greeter/SayHello", activity.DisplayName); + Assert.Contains(activity.GetTagValue(SemanticConventions.AttributeNetworkPeerAddress), 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)); + Assert.Equal("OK", activity.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); } else { + Assert.Equal("POST /greet.Greeter/SayHello", activity.DisplayName); Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); } @@ -101,9 +108,18 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributes(string? enableGrp Assert.Equal("/greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeUrlPath)); Assert.Equal("2", activity.GetTagValue(SemanticConventions.AttributeNetworkProtocolVersion)); Assert.StartsWith("grpc-dotnet", activity.GetTagValue(SemanticConventions.AttributeUserAgentOriginal) as string); + + if (aspNetCoreSupportEnabled && DockerHelper.IsAvailable(DockerPlatform.Linux)) + { + await WeaverTelemetryVerifier.VerifyAsync( + (exportedItems, []), + GrpcClientDiagnosticListener.SemanticConventionsVersion, + weaver, + outputHelper); + } } - [Theory(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1778")] + [Theory] [InlineData(null)] [InlineData("true")] [InlineData("false")] @@ -152,19 +168,20 @@ public void GrpcAspNetCoreInstrumentationAddsCorrectAttributesWhenItCreatesNewAc Assert.Equal(ActivityKind.Server, activity.Kind); - if (enableGrpcAspNetCoreSupport != null && enableGrpcAspNetCoreSupport.Equals("true", StringComparison.OrdinalIgnoreCase)) + if (enableGrpcAspNetCoreSupport != null && string.Equals(enableGrpcAspNetCoreSupport, "true", StringComparison.OrdinalIgnoreCase)) { Assert.Equal("grpc", activity.GetTagValue(SemanticConventions.AttributeRpcSystemName)); - 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.Equal("greet.Greeter/SayHello", activity.GetTagValue(SemanticConventions.AttributeRpcMethod)); + Assert.Equal("greet.Greeter/SayHello", activity.DisplayName); + Assert.Contains(activity.GetTagValue(SemanticConventions.AttributeNetworkPeerAddress), 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.AttributeRpcResponseStatusCode)); + Assert.Equal("OK", activity.GetTagValue(SemanticConventions.AttributeRpcResponseStatusCode)); } else { + Assert.Equal("POST /greet.Greeter/SayHello", activity.DisplayName); Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcMethodTagName)); Assert.NotNull(activity.GetTagValue(GrpcTagHelper.GrpcStatusCodeTagName)); } diff --git a/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/OpenTelemetry.Instrumentation.GrpcNetClient.Tests.csproj b/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/OpenTelemetry.Instrumentation.GrpcNetClient.Tests.csproj index 15738810a4..07c52b4025 100644 --- a/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/OpenTelemetry.Instrumentation.GrpcNetClient.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.GrpcNetClient.Tests/OpenTelemetry.Instrumentation.GrpcNetClient.Tests.csproj @@ -6,6 +6,10 @@ Unit test project for OpenTelemetry Grpc for .NET instrumentation. + + + + @@ -18,8 +22,8 @@ - + @@ -27,15 +31,26 @@ + + + + + + + + + + + diff --git a/test/Shared/WeaverTelemetryVerifier.cs b/test/Shared/WeaverTelemetryVerifier.cs index a986d03b0c..edddfe0506 100644 --- a/test/Shared/WeaverTelemetryVerifier.cs +++ b/test/Shared/WeaverTelemetryVerifier.cs @@ -165,6 +165,10 @@ private static void AssertReport( { ignore = true; } + else if (suppressAdvice.Contains(new(id, advice.Message))) + { + ignore = true; + } else if (advice.ExtensionData.TryGetValue("signal_name", out var extensionValue) && extensionValue.ValueKind == JsonValueKind.String && suppressAdvice.Contains(new(id, extensionValue.GetString())))