diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 011b1ab7231..a2e20890cb7 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -5,9 +5,10 @@ * In order to align with the [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-status) the `Status` (otel.status_code) tag (added on `Activity` using the `SetStatus` - extension) will now be set as the `Unset`, `Error`, or `Ok` string + extension) will now be set as the `UNSET`, `OK`, or `ERROR` string representation instead of the `0`, `1`, or `2` integer representation. - ([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579)) + ([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579) & + [#1620](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1620)) * Metrics API/SDK support is in an experimental state and is not recommended for production use. All metric APIs have been marked with the `Obsolete` attribute. See diff --git a/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs b/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs index 48cc88636cc..cdd5d92cd07 100644 --- a/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs +++ b/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs @@ -188,7 +188,7 @@ public bool ForEach(KeyValuePair item) switch (item.Key) { case SpanAttributeConstants.StatusCodeKey: - this.StatusCode = StatusHelper.GetStatusCodeForStringName(item.Value as string); + this.StatusCode = StatusHelper.GetStatusCodeForTagValue(item.Value as string); break; case SpanAttributeConstants.StatusDescriptionKey: this.StatusDescription = item.Value as string; diff --git a/src/OpenTelemetry.Api/Internal/StatusHelper.cs b/src/OpenTelemetry.Api/Internal/StatusHelper.cs index b3ce9ac5845..4ca8fa2b2a6 100644 --- a/src/OpenTelemetry.Api/Internal/StatusHelper.cs +++ b/src/OpenTelemetry.Api/Internal/StatusHelper.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System; using System.Runtime.CompilerServices; using OpenTelemetry.Trace; @@ -21,8 +22,12 @@ namespace OpenTelemetry.Internal { internal static class StatusHelper { + public const string UnsetStatusCodeTagValue = "UNSET"; + public const string OkStatusCodeTagValue = "OK"; + public const string ErrorStatusCodeTagValue = "ERROR"; + [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static string GetStringNameForStatusCode(StatusCode statusCode) + public static string GetTagValueForStatusCode(StatusCode statusCode) { return statusCode switch { @@ -31,26 +36,26 @@ public static string GetStringNameForStatusCode(StatusCode statusCode) * first because assumption is most spans will be * Unset, then Error. Ok is not set by the SDK. */ - StatusCode.Unset => "Unset", - StatusCode.Error => "Error", - StatusCode.Ok => "Ok", + StatusCode.Unset => UnsetStatusCodeTagValue, + StatusCode.Error => ErrorStatusCodeTagValue, + StatusCode.Ok => OkStatusCodeTagValue, _ => null, }; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static StatusCode? GetStatusCodeForStringName(string statusCodeName) + public static StatusCode? GetStatusCodeForTagValue(string statusCodeTagValue) { - return statusCodeName switch + return statusCodeTagValue switch { /* * Note: Order here does matter for perf. Unset is * first because assumption is most spans will be * Unset, then Error. Ok is not set by the SDK. */ - "Unset" => StatusCode.Unset, - "Error" => StatusCode.Error, - "Ok" => StatusCode.Ok, + string _ when UnsetStatusCodeTagValue.Equals(statusCodeTagValue, StringComparison.OrdinalIgnoreCase) => StatusCode.Unset, + string _ when ErrorStatusCodeTagValue.Equals(statusCodeTagValue, StringComparison.OrdinalIgnoreCase) => StatusCode.Error, + string _ when OkStatusCodeTagValue.Equals(statusCodeTagValue, StringComparison.OrdinalIgnoreCase) => StatusCode.Ok, _ => (StatusCode?)null, }; } diff --git a/src/OpenTelemetry.Api/README.md b/src/OpenTelemetry.Api/README.md index 4d4f052fcef..551631997ca 100644 --- a/src/OpenTelemetry.Api/README.md +++ b/src/OpenTelemetry.Api/README.md @@ -358,11 +358,11 @@ to be associated with `Activity`. There is no `Status` class in .NET, and hence Example: ```csharp -activity?.SetTag("otel.status_code", "Error"); +activity?.SetTag("otel.status_code", "ERROR"); activity?.SetTag("otel.status_description", "error status description"); ``` -Values for the StatusCode tag must be one of the strings "Unset", "Ok", or "Error", +Values for the StatusCode tag must be one of the strings "UNSET", "OK", or "ERROR", which correspond respectively to the enums `Unset`, `Ok`, and `Error` from [`StatusCode`](./Trace/StatusCode.cs). diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index e61df3726ae..04260f30fdd 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -45,7 +45,7 @@ public static void SetStatus(this Activity activity, Status status) { Debug.Assert(activity != null, "Activity should not be null"); - activity.SetTag(SpanAttributeConstants.StatusCodeKey, StatusHelper.GetStringNameForStatusCode(status.StatusCode)); + activity.SetTag(SpanAttributeConstants.StatusCodeKey, StatusHelper.GetTagValueForStatusCode(status.StatusCode)); activity.SetTag(SpanAttributeConstants.StatusDescriptionKey, status.Description); } diff --git a/src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md b/src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md index 8021f670b29..20635db9476 100644 --- a/src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md @@ -5,11 +5,13 @@ * In `JaegerExporterOptions`: Exporter options now include a switch for Batch vs Simple exporter, and settings for batch exporting properties. -* Jaeger will now set the `error` tag when `otel.status_code` is set to `Error`. - ([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579)) +* Jaeger will now set the `error` tag when `otel.status_code` is set to `ERROR`. + ([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579) & + [#1620](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1620)) -* Jaeger will no longer send the `otel.status_code` tag if the value is `Unset`. - ([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609)) +* Jaeger will no longer send the `otel.status_code` tag if the value is `UNSET`. + ([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609) & + [#1620](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1620)) * Span Event.Name will now be populated as the `event` field on Jaeger Logs instead of `message`. diff --git a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs index cefb7355b01..5a48163439d 100644 --- a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs +++ b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs @@ -259,14 +259,20 @@ private static void ProcessJaegerTag(ref TagEnumerationState state, string key, if (key == SpanAttributeConstants.StatusCodeKey) { - if (jaegerTag.VStr == "Error") + StatusCode? statusCode = StatusHelper.GetStatusCodeForTagValue(jaegerTag.VStr); + if (statusCode == StatusCode.Error) { + // Error flag: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/jaeger.md#error-flag PooledList.Add(ref state.Tags, new JaegerTag("error", JaegerTagType.BOOL, vBool: true)); } - else if (jaegerTag.VStr == "Unset") + else if (!statusCode.HasValue || statusCode == StatusCode.Unset) { + // Unset Status is not sent: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/jaeger.md#status return; } + + // Normalize status since it is user-driven. + jaegerTag = new JaegerTag(key, JaegerTagType.STRING, vStr: StatusHelper.GetTagValueForStatusCode(statusCode.Value)); } } else if (jaegerTag.VLong.HasValue) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs index ebbae69c270..3810e603ba7 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs @@ -239,7 +239,7 @@ internal static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair activityTag) if (key == SpanAttributeConstants.StatusCodeKey) { - if (strVal == "Error") + StatusCode? statusCode = StatusHelper.GetStatusCodeForTagValue(strVal); + if (statusCode == StatusCode.Error) { - PooledList>.Add(ref this.Tags, new KeyValuePair("error", "true")); + // Error flag: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#error-flag + PooledList>.Add(ref this.Tags, new KeyValuePair("error", string.Empty)); } - else if (strVal == "Unset") + else if (!statusCode.HasValue || statusCode == StatusCode.Unset) { // Unset Status is not sent: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#status return true; } + + // Normalize status since it is user-driven. + activityTag = new KeyValuePair(key, StatusHelper.GetTagValueForStatusCode(statusCode.Value)); } } else if (activityTag.Value is int intVal && activityTag.Key == SemanticConventions.AttributeNetPeerPort) diff --git a/test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs b/test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs index 52a87f0b217..defdb0465dc 100644 --- a/test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs +++ b/test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs @@ -436,39 +436,35 @@ public void JaegerActivityConverterTest_NullTagValueTest() } [Theory] - [InlineData(StatusCode.Unset, false)] - [InlineData(StatusCode.Ok, false)] - [InlineData(StatusCode.Error, true)] - public void JaegerActivityConverterTest_Status_ErrorFlagTest(StatusCode statusCode, bool hasErrorFlag) + [InlineData(StatusCode.Unset, "unset")] + [InlineData(StatusCode.Ok, "Ok")] + [InlineData(StatusCode.Error, "ERROR")] + [InlineData(StatusCode.Unset, "iNvAlId")] + public void JaegerActivityConverterTest_Status_ErrorFlagTest(StatusCode expectedStatusCode, string statusCodeTagValue) { - var status = statusCode switch - { - StatusCode.Unset => Status.Unset, - StatusCode.Ok => Status.Ok, - StatusCode.Error => Status.Error, - _ => throw new InvalidOperationException(), - }; - // Arrange - var activity = CreateTestActivity(status: status); + var activity = CreateTestActivity(); + activity.SetTag(SpanAttributeConstants.StatusCodeKey, statusCodeTagValue); // Act var jaegerSpan = activity.ToJaegerSpan(); // Assert - if (statusCode == StatusCode.Unset) + Assert.Equal(expectedStatusCode, activity.GetStatus().StatusCode); + + if (expectedStatusCode == StatusCode.Unset) { Assert.DoesNotContain(jaegerSpan.Tags, t => t.Key == SpanAttributeConstants.StatusCodeKey); } else { Assert.Equal( - StatusHelper.GetStringNameForStatusCode(statusCode), + StatusHelper.GetTagValueForStatusCode(expectedStatusCode), jaegerSpan.Tags.FirstOrDefault(t => t.Key == SpanAttributeConstants.StatusCodeKey).VStr); } - if (hasErrorFlag) + if (expectedStatusCode == StatusCode.Error) { Assert.Contains(jaegerSpan.Tags, t => t.Key == "error" && t.VType == JaegerTagType.BOOL && (t.VBool ?? false)); } diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs index fd2f20e01f3..22f8635f29c 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs @@ -89,41 +89,37 @@ public void ToZipkinSpan_NoEvents() } [Theory] - [InlineData(StatusCode.Unset, false)] - [InlineData(StatusCode.Ok, false)] - [InlineData(StatusCode.Error, true)] - public void ToZipkinSpan_Status_ErrorFlagTest(StatusCode statusCode, bool hasErrorFlag) + [InlineData(StatusCode.Unset, "unset")] + [InlineData(StatusCode.Ok, "Ok")] + [InlineData(StatusCode.Error, "ERROR")] + [InlineData(StatusCode.Unset, "iNvAlId")] + public void ToZipkinSpan_Status_ErrorFlagTest(StatusCode expectedStatusCode, string statusCodeTagValue) { - var status = statusCode switch - { - StatusCode.Unset => Status.Unset, - StatusCode.Ok => Status.Ok, - StatusCode.Error => Status.Error, - _ => throw new InvalidOperationException(), - }; - // Arrange - var activity = ZipkinExporterTests.CreateTestActivity(status: status); + var activity = ZipkinExporterTests.CreateTestActivity(); + activity.SetTag(SpanAttributeConstants.StatusCodeKey, statusCodeTagValue); // Act var zipkinSpan = activity.ToZipkinSpan(DefaultZipkinEndpoint); // Assert - if (statusCode == StatusCode.Unset) + Assert.Equal(expectedStatusCode, activity.GetStatus().StatusCode); + + if (expectedStatusCode == StatusCode.Unset) { Assert.DoesNotContain(zipkinSpan.Tags, t => t.Key == SpanAttributeConstants.StatusCodeKey); } else { Assert.Equal( - StatusHelper.GetStringNameForStatusCode(statusCode), + StatusHelper.GetTagValueForStatusCode(expectedStatusCode), zipkinSpan.Tags.FirstOrDefault(t => t.Key == SpanAttributeConstants.StatusCodeKey).Value); } - if (hasErrorFlag) + if (expectedStatusCode == StatusCode.Error) { - Assert.Contains(zipkinSpan.Tags, t => t.Key == "error" && (string)t.Value == "true"); + Assert.Contains(zipkinSpan.Tags, t => t.Key == "error" && (string)t.Value == string.Empty); } else { diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index abd1c67bf6f..4446f5fc001 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -137,8 +137,18 @@ public void SuppresssesInstrumentation() [InlineData(false, false, false)] [InlineData(false, true, false)] [InlineData(false, false, true)] - public void IntegrationTest(bool useShortTraceIds, bool useTestResource, bool isRootSpan) + [InlineData(false, false, false, StatusCode.Ok)] + [InlineData(false, false, false, StatusCode.Error)] + public void IntegrationTest(bool useShortTraceIds, bool useTestResource, bool isRootSpan, StatusCode statusCode = StatusCode.Unset) { + var status = statusCode switch + { + StatusCode.Unset => Status.Unset, + StatusCode.Ok => Status.Ok, + StatusCode.Error => Status.Error, + _ => throw new InvalidOperationException(), + }; + Guid requestId = Guid.NewGuid(); ZipkinExporter exporter = new ZipkinExporter( @@ -150,7 +160,7 @@ public void IntegrationTest(bool useShortTraceIds, bool useTestResource, bool is var serviceName = ZipkinExporterOptions.DefaultServiceName; var resoureTags = string.Empty; - var activity = CreateTestActivity(isRootSpan: isRootSpan); + var activity = CreateTestActivity(isRootSpan: isRootSpan, status: status); if (useTestResource) { serviceName = "MyService"; @@ -192,8 +202,15 @@ public void IntegrationTest(bool useShortTraceIds, bool useTestResource, bool is var traceId = useShortTraceIds ? TraceId.Substring(TraceId.Length - 16, 16) : TraceId; + var statusTag = statusCode switch + { + StatusCode.Ok => $@"""{SpanAttributeConstants.StatusCodeKey}"":""OK"",", + StatusCode.Error => $@"""error"":"""",""{SpanAttributeConstants.StatusCodeKey}"":""ERROR"",", + _ => string.Empty, + }; + Assert.Equal( - $@"[{{""traceId"":""{traceId}"",""name"":""Name"",{parentId}""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""{serviceName}""{ipInformation}}},""remoteEndpoint"":{{""serviceName"":""http://localhost:44312/""}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{{resoureTags}""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""longArrayKey"":""1,2"",""boolKey"":""true"",""boolArrayKey"":""true,false"",""http.host"":""http://localhost:44312/"",""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""}}}}]", + $@"[{{""traceId"":""{traceId}"",""name"":""Name"",{parentId}""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""{serviceName}""{ipInformation}}},""remoteEndpoint"":{{""serviceName"":""http://localhost:44312/""}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{{resoureTags}""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""longArrayKey"":""1,2"",""boolKey"":""true"",""boolArrayKey"":""true,false"",""http.host"":""http://localhost:44312/"",{statusTag}""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""}}}}]", Responses[requestId]); } diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs index 21c1cc72a8b..db47ec2f5cb 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs @@ -96,17 +96,10 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut Assert.Equal(ActivityKind.Client, activity.Kind); Assert.Equal(tc.SpanName, activity.DisplayName); - var d = new Dictionary() - { - { "Ok", "OK" }, - { "Error", "ERROR" }, - { "Unset", "UNSET" }, - }; - // Assert.Equal(tc.SpanStatus, d[span.Status.CanonicalCode]); Assert.Equal( tc.SpanStatus, - d[activity.GetTagValue(SpanAttributeConstants.StatusCodeKey) as string]); + activity.GetTagValue(SpanAttributeConstants.StatusCodeKey) as string); if (tc.SpanStatusHasDescription.HasValue) { diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.netfx.cs index 88d779d4119..ce513b12de0 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.netfx.cs @@ -88,13 +88,6 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc ValidateHttpWebRequestActivity(activity); Assert.Equal(tc.SpanName, activity.DisplayName); - var d = new Dictionary() - { - { "Ok", "OK" }, - { "Error", "ERROR" }, - { "Unset", "UNSET" }, - }; - tc.SpanAttributes = tc.SpanAttributes.ToDictionary( x => x.Key, x => @@ -115,7 +108,7 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc { if (tag.Key == SpanAttributeConstants.StatusCodeKey) { - Assert.Equal(tc.SpanStatus, d[tagValue]); + Assert.Equal(tc.SpanStatus, tagValue); continue; }