Skip to content

Commit

Permalink
Exporter spec updates. (#1609)
Browse files Browse the repository at this point in the history 
* Exporter spec updates.

* CHANGELOG updates.

Co-authored-by: Cijo Thomas <[email protected]>
  • Loading branch information
@CodeBlanch @cijothomas
CodeBlanch and cijothomas authored Nov 23, 2020
1 parent 9f27b4c commit d3edc47
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 40 deletions.
7 changes: 7 additions & 0 deletions src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md
View file
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
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")
{
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
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
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)
{
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
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
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
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
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

0 comments on commit d3edc47

Please sign in to comment.