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 updates. #1609

Merged
merged 4 commits into from
Nov 23, 2020
Merged

Exporter spec updates. #1609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8a0a123
Exporter spec updates.
CodeBlanch Nov 23, 2020
68ded52
Merge branch 'master' into exporter-spec
CodeBlanch Nov 23, 2020
421496b
Merge branch 'master' into exporter-spec
cijothomas Nov 23, 2020
e948b45
CHANGELOG updates.
CodeBlanch Nov 23, 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
7 changes: 7 additions & 0 deletions src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@
* 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 no longer send the `otel.status_code` tag if the value is `Unset`.
([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609))

* Span Event.Name will now be populated as the `event` field on Jaeger Logs
instead of `message`.
([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609))

## 1.0.0-rc1.1

Released 2020-Nov-17
Expand Down
27 changes: 21 additions & 6 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 @@ -162,10 +162,11 @@ public static JaegerLog ToJaegerLog(this ActivityEvent timedEvent)

timedEvent.EnumerateTags(ref jaegerTags);

// Matches what OpenTracing and OpenTelemetry defines as the event name.
// https://github.com/opentracing/specification/blob/master/semantic_conventions.md#log-fields-table
// https://github.com/open-telemetry/opentelemetry-specification/pull/397/files
PooledList<JaegerTag>.Add(ref jaegerTags.Tags, new JaegerTag("message", JaegerTagType.STRING, vStr: timedEvent.Name));
if (!jaegerTags.HasEvent)
{
// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/jaeger.md#events
PooledList<JaegerTag>.Add(ref jaegerTags.Tags, new JaegerTag("event", JaegerTagType.STRING, vStr: timedEvent.Name));
}

// TODO: Use the same function as JaegerConversionExtensions or check that the perf here is acceptable.
return new JaegerLog(timedEvent.Timestamp.ToEpochMicroseconds(), jaegerTags.Tags);
Expand Down Expand Up @@ -256,9 +257,16 @@ private static void ProcessJaegerTag(ref TagEnumerationState state, string key,
{
PeerServiceResolver.InspectTag(ref state, key, jaegerTag.VStr);

if (key == SpanAttributeConstants.StatusCodeKey && jaegerTag.VStr == "Error")
if (key == SpanAttributeConstants.StatusCodeKey)
{
PooledList<JaegerTag>.Add(ref state.Tags, new JaegerTag("error", JaegerTagType.BOOL, vBool: true));
if (jaegerTag.VStr == "Error")
{
PooledList<JaegerTag>.Add(ref state.Tags, new JaegerTag("error", JaegerTagType.BOOL, vBool: true));
}
else if (jaegerTag.VStr == "Unset")
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}
}
}
else if (jaegerTag.VLong.HasValue)
Expand Down Expand Up @@ -342,6 +350,8 @@ private struct EventTagsEnumerationState : IActivityEnumerator<KeyValuePair<stri
{
public PooledList<JaegerTag> Tags;

public bool HasEvent;

public bool ForEach(KeyValuePair<string, object> tag)
{
if (tag.Value is Array)
Expand All @@ -353,6 +363,11 @@ public bool ForEach(KeyValuePair<string, object> tag)
PooledList<JaegerTag>.Add(ref this.Tags, tag.ToJaegerTag());
}

if (tag.Key == "event")
{
this.HasEvent = true;
}

return true;
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
* 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 no longer send the `otel.status_code` tag if the value is `Unset`.
([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609))

* Zipkin bool tag values will now be sent as `true`/`false` instead of
`True`/`False`.
([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609))

## 1.0.0-rc1.1

Released 2020-Nov-17
Expand Down
12 changes: 10 additions & 2 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 @@ -175,9 +175,17 @@ public bool ForEach(KeyValuePair<string, object> activityTag)
{
PeerServiceResolver.InspectTag(ref this, key, strVal);

if (key == SpanAttributeConstants.StatusCodeKey && strVal == "Error")
if (key == SpanAttributeConstants.StatusCodeKey)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to confirm the full behaviour:
Every status except Unset is send to Zipkin as tags.
Error means, we send additional "error=true" tag. -- (This is not explicitly requested in spec, but is a nice addition)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cijothomas Correct. I'm going to open a PR into spec for adding the "error" flag on Zipkin & Jaeger when Status=Error. I don't know if it will get in though because of the freeze?

{
PooledList<KeyValuePair<string, object>>.Add(ref this.Tags, new KeyValuePair<string, object>("error", "true"));
if (strVal == "Error")
{
PooledList<KeyValuePair<string, object>>.Add(ref this.Tags, new KeyValuePair<string, object>("error", "true"));
}
else if (strVal == "Unset")
{
// Unset Status is not sent: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#status
return true;
}
}
}
else if (activityTag.Value is int intVal && activityTag.Key == SemanticConventions.AttributeNetPeerPort)
Expand Down
47 changes: 27 additions & 20 deletions src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ public ZipkinSpan(
long? duration,
ZipkinEndpoint localEndpoint,
ZipkinEndpoint remoteEndpoint,
in PooledList<ZipkinAnnotation>? annotations,
in PooledList<KeyValuePair<string, object>>? tags,
in PooledList<ZipkinAnnotation> annotations,
in PooledList<KeyValuePair<string, object>> tags,
bool? debug,
bool? shared)
{
Expand Down Expand Up @@ -89,18 +89,18 @@ public ZipkinSpan(

public ZipkinEndpoint RemoteEndpoint { get; }

public PooledList<ZipkinAnnotation>? Annotations { get; }
public PooledList<ZipkinAnnotation> Annotations { get; }

public PooledList<KeyValuePair<string, object>>? Tags { get; }
public PooledList<KeyValuePair<string, object>> Tags { get; }

public bool? Debug { get; }

public bool? Shared { get; }

public void Return()
{
this.Annotations?.Return();
this.Tags?.Return();
this.Annotations.Return();
this.Tags.Return();
}

#if NET452
Expand Down Expand Up @@ -168,12 +168,12 @@ public void Write(JsonWriter writer)
this.RemoteEndpoint.Write(writer);
}

if (this.Annotations.HasValue)
if (!this.Annotations.IsEmpty)
{
writer.WritePropertyName("annotations");
writer.WriteStartArray();

foreach (var annotation in this.Annotations.Value)
foreach (var annotation in this.Annotations)
{
writer.WriteStartObject();

Expand All @@ -189,7 +189,7 @@ public void Write(JsonWriter writer)
writer.WriteEndArray();
}

if (this.Tags.HasValue || this.LocalEndpoint.Tags != null)
if (!this.Tags.IsEmpty || this.LocalEndpoint.Tags != null)
{
writer.WritePropertyName("tags");
writer.WriteStartObject();
Expand All @@ -203,13 +203,13 @@ public void Write(JsonWriter writer)
foreach (var tag in this.LocalEndpoint.Tags ?? Enumerable.Empty<KeyValuePair<string, object>>())
{
writer.WritePropertyName(tag.Key);
writer.WriteValue(this.ConvertObjectToString(tag.Value));
writer.WriteValue(ConvertObjectToString(tag.Value));
}

foreach (var tag in this.Tags ?? Enumerable.Empty<KeyValuePair<string, object>>())
foreach (var tag in this.Tags)
{
writer.WritePropertyName(tag.Key);
writer.WriteValue(this.ConvertObjectToString(tag.Value));
writer.WriteValue(ConvertObjectToString(tag.Value));
}
}
finally
Expand Down Expand Up @@ -278,12 +278,12 @@ public void Write(Utf8JsonWriter writer)
this.RemoteEndpoint.Write(writer);
}

if (this.Annotations.HasValue)
if (!this.Annotations.IsEmpty)
{
writer.WritePropertyName("annotations");
writer.WriteStartArray();

foreach (var annotation in this.Annotations.Value)
foreach (var annotation in this.Annotations)
{
writer.WriteStartObject();

Expand All @@ -297,7 +297,7 @@ public void Write(Utf8JsonWriter writer)
writer.WriteEndArray();
}

if (this.Tags.HasValue || this.LocalEndpoint.Tags != null)
if (!this.Tags.IsEmpty || this.LocalEndpoint.Tags != null)
{
writer.WritePropertyName("tags");
writer.WriteStartObject();
Expand All @@ -310,12 +310,12 @@ public void Write(Utf8JsonWriter writer)
{
foreach (var tag in this.LocalEndpoint.Tags ?? Enumerable.Empty<KeyValuePair<string, object>>())
{
writer.WriteString(tag.Key, this.ConvertObjectToString(tag.Value));
writer.WriteString(tag.Key, ConvertObjectToString(tag.Value));
}

foreach (var tag in this.Tags ?? Enumerable.Empty<KeyValuePair<string, object>>())
foreach (var tag in this.Tags)
{
writer.WriteString(tag.Key, this.ConvertObjectToString(tag.Value));
writer.WriteString(tag.Key, ConvertObjectToString(tag.Value));
}
}
finally
Expand All @@ -332,17 +332,24 @@ public void Write(Utf8JsonWriter writer)
#endif

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private string ConvertObjectToString(object obj)
private static string ConvertObjectToString(object obj)
{
return obj switch
{
string stringVal => stringVal,
bool boolVal => GetBoolString(boolVal),
int[] arrayValue => string.Join(",", arrayValue),
long[] arrayValue => string.Join(",", arrayValue),
double[] arrayValue => string.Join(",", arrayValue),
bool[] arrayValue => string.Join(",", arrayValue),
bool[] arrayValue => string.Join(",", arrayValue.Select(GetBoolString)),
_ => obj.ToString(),
};
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static string GetBoolString(bool value)
{
return value ? "true" : "false";
}
}
}
24 changes: 18 additions & 6 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 @@ -116,7 +116,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_AllPropertie
Assert.Equal("string_array", eventField.Key);
Assert.Equal("b", eventField.VStr);
eventField = eventFields[3];
Assert.Equal("message", eventField.Key);
Assert.Equal("event", eventField.Key);
Assert.Equal("Event1", eventField.VStr);

Assert.Equal(activity.Events.First().Timestamp.ToEpochMicroseconds(), jaegerLog.Timestamp);
Expand All @@ -128,7 +128,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_AllPropertie
Assert.Equal("key", eventField.Key);
Assert.Equal("value", eventField.VStr);
eventField = eventFields[1];
Assert.Equal("message", eventField.Key);
Assert.Equal("event", eventField.Key);
Assert.Equal("Event2", eventField.VStr);
}

Expand Down Expand Up @@ -175,7 +175,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_NoAttributes
Assert.Equal("key", eventField.Key);
Assert.Equal("value", eventField.VStr);
eventField = eventFields[3];
Assert.Equal("message", eventField.Key);
Assert.Equal("event", eventField.Key);
Assert.Equal("Event1", eventField.VStr);

Assert.Equal(activity.Events.First().Timestamp.ToEpochMicroseconds(), jaegerLog.Timestamp);
Expand All @@ -187,7 +187,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_NoAttributes
Assert.Equal("key", eventField.Key);
Assert.Equal("value", eventField.VStr);
eventField = eventFields[1];
Assert.Equal("message", eventField.Key);
Assert.Equal("event", eventField.Key);
Assert.Equal("Event2", eventField.VStr);
}

Expand Down Expand Up @@ -340,7 +340,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_NoLinks()
Assert.Equal("key", eventField.Key);
Assert.Equal("value", eventField.VStr);
eventField = eventFields[3];
Assert.Equal("message", eventField.Key);
Assert.Equal("event", eventField.Key);
Assert.Equal("Event1", eventField.VStr);
Assert.Equal(activity.Events.First().Timestamp.ToEpochMicroseconds(), jaegerLog.Timestamp);

Expand All @@ -351,7 +351,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_NoLinks()
Assert.Equal("key", eventField.Key);
Assert.Equal("value", eventField.VStr);
eventField = eventFields[1];
Assert.Equal("message", eventField.Key);
Assert.Equal("event", eventField.Key);
Assert.Equal("Event2", eventField.VStr);
}

Expand Down Expand Up @@ -456,6 +456,18 @@ public void JaegerActivityConverterTest_Status_ErrorFlagTest(StatusCode statusCo
var jaegerSpan = activity.ToJaegerSpan();

// Assert

if (statusCode == StatusCode.Unset)
{
Assert.DoesNotContain(jaegerSpan.Tags, t => t.Key == SpanAttributeConstants.StatusCodeKey);
}
else
{
Assert.Equal(
StatusHelper.GetStringNameForStatusCode(statusCode),
jaegerSpan.Tags.FirstOrDefault(t => t.Key == SpanAttributeConstants.StatusCodeKey).VStr);
}

if (hasErrorFlag)
{
Assert.Contains(jaegerSpan.Tags, t => t.Key == "error" && t.VType == JaegerTagType.BOOL && (t.VBool ?? false));
Expand Down
23 changes: 18 additions & 5 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 @@ -17,6 +17,7 @@
using System;
using System.Linq;
using OpenTelemetry.Exporter.Zipkin.Implementation;
using OpenTelemetry.Internal;
using OpenTelemetry.Trace;
using Xunit;

Expand Down Expand Up @@ -45,7 +46,7 @@ public void ToZipkinSpan_AllPropertiesSet()
Assert.Equal((long)(activity.Duration.TotalMilliseconds * 1000), zipkinSpan.Duration);

int counter = 0;
var tagsArray = zipkinSpan.Tags.Value.ToArray();
var tagsArray = zipkinSpan.Tags.ToArray();

foreach (var tags in activity.TagObjects)
{
Expand All @@ -70,12 +71,12 @@ public void ToZipkinSpan_NoEvents()
var zipkinSpan = activity.ToZipkinSpan(DefaultZipkinEndpoint);

Assert.Equal(ZipkinSpanName, zipkinSpan.Name);
Assert.Empty(zipkinSpan.Annotations.Value);
Assert.Empty(zipkinSpan.Annotations);
Assert.Equal(activity.TraceId.ToHexString(), zipkinSpan.TraceId);
Assert.Equal(activity.SpanId.ToHexString(), zipkinSpan.Id);

int counter = 0;
var tagsArray = zipkinSpan.Tags.Value.ToArray();
var tagsArray = zipkinSpan.Tags.ToArray();

foreach (var tags in activity.TagObjects)
{
Expand Down Expand Up @@ -108,13 +109,25 @@ public void ToZipkinSpan_Status_ErrorFlagTest(StatusCode statusCode, bool hasErr
var zipkinSpan = activity.ToZipkinSpan(DefaultZipkinEndpoint);

// Assert

if (statusCode == StatusCode.Unset)
{
Assert.DoesNotContain(zipkinSpan.Tags, t => t.Key == SpanAttributeConstants.StatusCodeKey);
}
else
{
Assert.Equal(
StatusHelper.GetStringNameForStatusCode(statusCode),
zipkinSpan.Tags.FirstOrDefault(t => t.Key == SpanAttributeConstants.StatusCodeKey).Value);
}

if (hasErrorFlag)
{
Assert.Contains(zipkinSpan.Tags.Value, t => t.Key == "error" && (string)t.Value == "true");
Assert.Contains(zipkinSpan.Tags, t => t.Key == "error" && (string)t.Value == "true");
}
else
{
Assert.DoesNotContain(zipkinSpan.Tags.Value, t => t.Key == "error");
Assert.DoesNotContain(zipkinSpan.Tags, t => t.Key == "error");
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public void IntegrationTest(bool useShortTraceIds, bool useTestResource, bool is
var traceId = useShortTraceIds ? TraceId.Substring(TraceId.Length - 16, 16) : TraceId;

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"",""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/"",""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""}}}}]",
Responses[requestId]);
}

Expand Down Expand Up @@ -223,6 +223,7 @@ internal static Activity CreateTestActivity(
{ "doubleKey2", 1F },
{ "longArrayKey", new long[] { 1, 2 } },
{ "boolKey", true },
{ "boolArrayKey", new bool[] { true, false } },
{ "http.host", "http://localhost:44312/" }, // simulating instrumentation tag adding http.host
};
if (additionalAttributes != null)
Expand Down