Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exporter spec fixup 2 #1620

Merged
merged 6 commits into from
Nov 28, 2020
Merged

Exporter spec fixup 2 #1620

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9a4b550
Jaeger & Zipkin status enums are now uppercase. Zipkin error tag is a…
CodeBlanch Nov 25, 2020
60473e3
Updated CHANGELOGs.
CodeBlanch Nov 25, 2020
6a76617
Merge branch 'master' into exporter-spec2
CodeBlanch Nov 25, 2020
e288b64
Attempting to fix build.
CodeBlanch Nov 25, 2020
33c6381
Merge branch 'master' into exporter-spec2
cijothomas Nov 25, 2020
77334ae
Merge branch 'master' into exporter-spec2
cijothomas Nov 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/OpenTelemetry.Api/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public bool ForEach(KeyValuePair<string, object> 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;
Expand Down
23 changes: 14 additions & 9 deletions src/OpenTelemetry.Api/Internal/StatusHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,20 @@
// limitations under the License.
// </copyright>

using System;
using System.Runtime.CompilerServices;
using OpenTelemetry.Trace;

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
{
Expand All @@ -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,
};
}
Expand Down
4 changes: 2 additions & 2 deletions src/OpenTelemetry.Api/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry.Api/Trace/ActivityExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
10 changes: 6 additions & 4 deletions src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
10 changes: 8 additions & 2 deletions src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<JaegerTag>.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)
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ internal static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair<string, ob
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static OtlpTrace.Status ToOtlpStatus(ref TagEnumerationState otlpTags)
{
var status = StatusHelper.GetStatusCodeForStringName(otlpTags.StatusCode);
var status = StatusHelper.GetStatusCodeForTagValue(otlpTags.StatusCode);

if (!status.HasValue)
{
Expand Down
10 changes: 6 additions & 4 deletions src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

## Unreleased

* Zipkin will now set the `error` tag when `otel.status_code` is set to `Error`.
([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579))
* Zipkin 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))

* Zipkin will no longer send the `otel.status_code` tag if the value is `Unset`.
([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609))
* Zipkin 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))

* Zipkin bool tag values will now be sent as `true`/`false` instead of
`True`/`False`.
Expand Down
11 changes: 8 additions & 3 deletions src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,20 @@ public bool ForEach(KeyValuePair<string, object> activityTag)

if (key == SpanAttributeConstants.StatusCodeKey)
{
if (strVal == "Error")
StatusCode? statusCode = StatusHelper.GetStatusCodeForTagValue(strVal);
if (statusCode == StatusCode.Error)
{
PooledList<KeyValuePair<string, object>>.Add(ref this.Tags, new KeyValuePair<string, object>("error", "true"));
// Error flag: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#error-flag
PooledList<KeyValuePair<string, object>>.Add(ref this.Tags, new KeyValuePair<string, object>("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<string, object>(key, StatusHelper.GetTagValueForStatusCode(statusCode.Value));
}
}
else if (activityTag.Value is int intVal && activityTag.Key == SemanticConventions.AttributeNetPeerPort)
Expand Down
28 changes: 12 additions & 16 deletions test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
30 changes: 13 additions & 17 deletions test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
23 changes: 20 additions & 3 deletions test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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";
Expand Down Expand Up @@ -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]);
}

Expand Down
9 changes: 1 addition & 8 deletions test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.netcore31.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>()
{
{ "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)
{
Expand Down
Loading