From 24c1569e972d5b33e9375ff570173f7392103a88 Mon Sep 17 00:00:00 2001 From: Dmitrii Anoshin Date: Sat, 2 May 2020 11:07:18 -0700 Subject: [PATCH] Improve performance of internal traces to jaeger proto translation (#906) This PR also fixes an attribute mutation bug when "service.name" got removed from resource data structure --- .../trace/jaeger/traces_to_jaegerproto.go | 158 +++++++++----- .../jaeger/traces_to_jaegerproto_test.go | 195 +++++++++--------- 2 files changed, 201 insertions(+), 152 deletions(-) diff --git a/translator/trace/jaeger/traces_to_jaegerproto.go b/translator/trace/jaeger/traces_to_jaegerproto.go index cc8e23e92c8..c5fd74f303e 100644 --- a/translator/trace/jaeger/traces_to_jaegerproto.go +++ b/translator/trace/jaeger/traces_to_jaegerproto.go @@ -115,39 +115,43 @@ func resourceToJaegerProtoProcess(resource pdata.Resource) *model.Process { } process := model.Process{} + attrsCount := attrs.Len() if serviceName, ok := attrs.Get(conventions.AttributeServiceName); ok { process.ServiceName = serviceName.StringVal() + attrsCount-- } - process.Tags = resourceAttributesToJaegerProtoTags(attrs) + if attrsCount == 0 { + return &process + } + + tags := make([]model.KeyValue, 0, attrsCount) + process.Tags = appendTagsFromResourceAttributes(tags, attrs) return &process } -func resourceAttributesToJaegerProtoTags(attrs pdata.AttributeMap) []model.KeyValue { +func appendTagsFromResourceAttributes(dest []model.KeyValue, attrs pdata.AttributeMap) []model.KeyValue { if attrs.Len() == 0 { - return nil + return dest } - tags := make([]model.KeyValue, 0, attrs.Len()) attrs.ForEach(func(key string, attr pdata.AttributeValue) { if key == conventions.AttributeServiceName { return } - tags = append(tags, attributeToJaegerProtoTag(key, attr)) + dest = append(dest, attributeToJaegerProtoTag(key, attr)) }) - return tags + return dest } -func attributesToJaegerProtoTags(attrs pdata.AttributeMap) []model.KeyValue { +func appendTagsFromAttributes(dest []model.KeyValue, attrs pdata.AttributeMap) []model.KeyValue { if attrs.Len() == 0 { - return nil + return dest } - - tags := make([]model.KeyValue, 0, attrs.Len()) attrs.ForEach(func(key string, attr pdata.AttributeValue) { - tags = append(tags, attributeToJaegerProtoTag(key, attr)) + dest = append(dest, attributeToJaegerProtoTag(key, attr)) }) - return tags + return dest } func attributeToJaegerProtoTag(key string, attr pdata.AttributeValue) model.KeyValue { @@ -191,9 +195,6 @@ func spanToJaegerProto(span pdata.Span) (*model.Span, error) { return nil, fmt.Errorf("error converting span links to Jaeger references: %w", err) } - tags := attributesToJaegerProtoTags(span.Attributes()) - tags = appendTagFromSpanKind(tags, span.Kind()) - tags = appendTagFromSpanStatus(tags, span.Status()) startTime := internal.UnixNanoToTime(span.StartTime()) return &model.Span{ @@ -203,11 +204,60 @@ func spanToJaegerProto(span pdata.Span) (*model.Span, error) { References: jReferences, StartTime: startTime, Duration: internal.UnixNanoToTime(span.EndTime()).Sub(startTime), - Tags: tags, + Tags: getJaegerProtoSpanTags(span), Logs: spanEventsToJaegerProtoLogs(span.Events()), }, nil } +func getJaegerProtoSpanTags(span pdata.Span) []model.KeyValue { + var spanKindTag, statusCodeTag, errorTag, statusMsgTag model.KeyValue + var spanKindTagFound, statusCodeTagFound, errorTagFound, statusMsgTagFound bool + + tagsCount := span.Attributes().Len() + + spanKindTag, spanKindTagFound = getTagFromSpanKind(span.Kind()) + if spanKindTagFound { + tagsCount++ + } + status := span.Status() + if !status.IsNil() { + statusCodeTag, statusCodeTagFound = getTagFromStatusCode(status.Code()) + if statusCodeTagFound { + tagsCount++ + } + + errorTag, errorTagFound = getErrorTagFromStatusCode(status.Code()) + if errorTagFound { + tagsCount++ + } + + statusMsgTag, statusMsgTagFound = getTagFromStatusMsg(status.Message()) + if statusMsgTagFound { + tagsCount++ + } + } + + if tagsCount == 0 { + return nil + } + + tags := make([]model.KeyValue, 0, tagsCount) + tags = appendTagsFromAttributes(tags, span.Attributes()) + if spanKindTagFound { + tags = append(tags, spanKindTag) + } + if statusCodeTagFound { + tags = append(tags, statusCodeTag) + } + if errorTagFound { + tags = append(tags, errorTag) + } + if statusMsgTagFound { + tags = append(tags, statusMsgTag) + } + return tags +} + func traceIDToJaegerProto(traceID pdata.TraceID) (model.TraceID, error) { traceIDHigh, traceIDLow, err := tracetranslator.BytesToUInt64TraceID([]byte(traceID)) if err != nil { @@ -308,67 +358,65 @@ func spanEventsToJaegerProtoLogs(events pdata.SpanEventSlice) []model.Log { continue } + fields := make([]model.KeyValue, 0, event.Attributes().Len()) + fields = appendTagsFromAttributes(fields, event.Attributes()) logs = append(logs, model.Log{ Timestamp: internal.UnixNanoToTime(event.Timestamp()), - Fields: attributesToJaegerProtoTags(event.Attributes()), + Fields: fields, }) } return logs } -func appendTagFromSpanKind(tags []model.KeyValue, spanKind pdata.SpanKind) []model.KeyValue { - tag := model.KeyValue{ - Key: tracetranslator.TagSpanKind, - VType: model.ValueType_STRING, - } - +func getTagFromSpanKind(spanKind pdata.SpanKind) (model.KeyValue, bool) { + var tagStr string switch spanKind { case pdata.SpanKindCLIENT: - tag.VStr = string(tracetranslator.OpenTracingSpanKindClient) + tagStr = string(tracetranslator.OpenTracingSpanKindClient) case pdata.SpanKindSERVER: - tag.VStr = string(tracetranslator.OpenTracingSpanKindServer) + tagStr = string(tracetranslator.OpenTracingSpanKindServer) case pdata.SpanKindPRODUCER: - tag.VStr = string(tracetranslator.OpenTracingSpanKindProducer) + tagStr = string(tracetranslator.OpenTracingSpanKindProducer) case pdata.SpanKindCONSUMER: - tag.VStr = string(tracetranslator.OpenTracingSpanKindConsumer) + tagStr = string(tracetranslator.OpenTracingSpanKindConsumer) default: - return tags + return model.KeyValue{}, false } - if tags == nil { - return []model.KeyValue{tag} - } - - return append(tags, tag) + return model.KeyValue{ + Key: tracetranslator.TagSpanKind, + VType: model.ValueType_STRING, + VStr: tagStr, + }, true } -func appendTagFromSpanStatus(tags []model.KeyValue, status pdata.SpanStatus) []model.KeyValue { - if status.IsNil() { - return tags - } - - tags = append(tags, model.KeyValue{ +func getTagFromStatusCode(statusCode pdata.StatusCode) (model.KeyValue, bool) { + return model.KeyValue{ Key: tracetranslator.TagStatusCode, - VInt64: int64(status.Code()), + VInt64: int64(statusCode), VType: model.ValueType_INT64, - }) + }, true +} - if status.Code() != pdata.StatusCode(otlptrace.Status_Ok) { - tags = append(tags, model.KeyValue{ - Key: tracetranslator.TagError, - VBool: true, - VType: model.ValueType_BOOL, - }) +func getErrorTagFromStatusCode(statusCode pdata.StatusCode) (model.KeyValue, bool) { + if statusCode == pdata.StatusCode(otlptrace.Status_Ok) { + return model.KeyValue{}, false } + return model.KeyValue{ + Key: tracetranslator.TagError, + VBool: true, + VType: model.ValueType_BOOL, + }, true +} - if status.Message() != "" { - tags = append(tags, model.KeyValue{ - Key: tracetranslator.TagStatusMsg, - VStr: status.Message(), - VType: model.ValueType_STRING, - }) +func getTagFromStatusMsg(statusMsg string) (model.KeyValue, bool) { + if statusMsg == "" { + return model.KeyValue{}, false } - - return tags + return model.KeyValue{ + Key: tracetranslator.TagStatusMsg, + VStr: statusMsg, + VType: model.ValueType_STRING, + }, true } diff --git a/translator/trace/jaeger/traces_to_jaegerproto_test.go b/translator/trace/jaeger/traces_to_jaegerproto_test.go index a351ee1b9a2..144910678c1 100644 --- a/translator/trace/jaeger/traces_to_jaegerproto_test.go +++ b/translator/trace/jaeger/traces_to_jaegerproto_test.go @@ -28,156 +28,148 @@ import ( tracetranslator "github.com/open-telemetry/opentelemetry-collector/translator/trace" ) -func TestAppendTagFromSpanStatus(t *testing.T) { - okStatus := pdata.NewSpanStatus() - okStatus.InitEmpty() - okStatus.SetCode(pdata.StatusCode(otlptrace.Status_Ok)) - - unknownStatus := pdata.NewSpanStatus() - unknownStatus.InitEmpty() - unknownStatus.SetCode(pdata.StatusCode(otlptrace.Status_UnknownError)) - - notFoundStatus := pdata.NewSpanStatus() - notFoundStatus.InitEmpty() - notFoundStatus.SetCode(pdata.StatusCode(otlptrace.Status_NotFound)) - notFoundStatus.SetMessage("404 Not Found") - +func TestGetTagFromStatusCode(t *testing.T) { tests := []struct { - name string - status pdata.SpanStatus - tags []model.KeyValue + name string + code pdata.StatusCode + tag model.KeyValue }{ { - name: "none", - status: pdata.NewSpanStatus(), - tags: []model.KeyValue(nil), - }, - - { - name: "ok", - status: okStatus, - tags: []model.KeyValue{ - { - Key: tracetranslator.TagStatusCode, - VInt64: int64(otlptrace.Status_Ok), - VType: model.ValueType_INT64, - }, + name: "ok", + code: pdata.StatusCode(otlptrace.Status_Ok), + tag: model.KeyValue{ + Key: tracetranslator.TagStatusCode, + VInt64: int64(otlptrace.Status_Ok), + VType: model.ValueType_INT64, }, }, { - name: "unknown", - status: unknownStatus, - tags: []model.KeyValue{ - { - Key: tracetranslator.TagStatusCode, - VInt64: int64(otlptrace.Status_UnknownError), - VType: model.ValueType_INT64, - }, - { - Key: tracetranslator.TagError, - VBool: true, - VType: model.ValueType_BOOL, - }, + name: "unknown", + code: pdata.StatusCode(otlptrace.Status_UnknownError), + tag: model.KeyValue{ + Key: tracetranslator.TagStatusCode, + VInt64: int64(otlptrace.Status_UnknownError), + VType: model.ValueType_INT64, }, }, { - name: "not-found-with-msg", - status: notFoundStatus, - tags: []model.KeyValue{ - { - Key: tracetranslator.TagStatusCode, - VInt64: int64(otlptrace.Status_NotFound), - VType: model.ValueType_INT64, - }, - { - Key: tracetranslator.TagError, - VBool: true, - VType: model.ValueType_BOOL, - }, - { - Key: tracetranslator.TagStatusMsg, - VStr: "404 Not Found", - VType: model.ValueType_STRING, - }, + name: "not-found", + code: pdata.StatusCode(otlptrace.Status_NotFound), + tag: model.KeyValue{ + Key: tracetranslator.TagStatusCode, + VInt64: int64(otlptrace.Status_NotFound), + VType: model.ValueType_INT64, }, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - tags := appendTagFromSpanStatus(nil, test.status) - assert.EqualValues(t, test.tags, tags) + got, ok := getTagFromStatusCode(test.code) + assert.True(t, ok) + assert.EqualValues(t, test.tag, got) }) } } -func TestAppendTagFromSpanKind(t *testing.T) { +func TestGetErrorTagFromStatusCode(t *testing.T) { + errTag := model.KeyValue{ + Key: tracetranslator.TagError, + VBool: true, + VType: model.ValueType_BOOL, + } + + got, ok := getErrorTagFromStatusCode(pdata.StatusCode(otlptrace.Status_Ok)) + assert.False(t, ok) + + got, ok = getErrorTagFromStatusCode(pdata.StatusCode(otlptrace.Status_UnknownError)) + assert.True(t, ok) + assert.EqualValues(t, errTag, got) + + got, ok = getErrorTagFromStatusCode(pdata.StatusCode(otlptrace.Status_NotFound)) + assert.True(t, ok) + assert.EqualValues(t, errTag, got) +} + +func TestGetTagFromStatusMsg(t *testing.T) { + got, ok := getTagFromStatusMsg("") + assert.False(t, ok) + + got, ok = getTagFromStatusMsg("test-error") + assert.True(t, ok) + assert.EqualValues(t, model.KeyValue{ + Key: tracetranslator.TagStatusMsg, + VStr: "test-error", + VType: model.ValueType_STRING, + }, got) +} + +func TestGetTagFromSpanKind(t *testing.T) { tests := []struct { name string kind pdata.SpanKind - tags []model.KeyValue + tag model.KeyValue + ok bool }{ { name: "unspecified", kind: pdata.SpanKindUNSPECIFIED, - tags: []model.KeyValue(nil), + tag: model.KeyValue{}, + ok: false, }, { name: "client", kind: pdata.SpanKindCLIENT, - tags: []model.KeyValue{ - { - Key: tracetranslator.TagSpanKind, - VType: model.ValueType_STRING, - VStr: string(tracetranslator.OpenTracingSpanKindClient), - }, + tag: model.KeyValue{ + Key: tracetranslator.TagSpanKind, + VType: model.ValueType_STRING, + VStr: string(tracetranslator.OpenTracingSpanKindClient), }, + ok: true, }, { name: "server", kind: pdata.SpanKindSERVER, - tags: []model.KeyValue{ - { - Key: tracetranslator.TagSpanKind, - VType: model.ValueType_STRING, - VStr: string(tracetranslator.OpenTracingSpanKindServer), - }, + tag: model.KeyValue{ + Key: tracetranslator.TagSpanKind, + VType: model.ValueType_STRING, + VStr: string(tracetranslator.OpenTracingSpanKindServer), }, + ok: true, }, { name: "producer", kind: pdata.SpanKindPRODUCER, - tags: []model.KeyValue{ - { - Key: tracetranslator.TagSpanKind, - VType: model.ValueType_STRING, - VStr: string(tracetranslator.OpenTracingSpanKindProducer), - }, + tag: model.KeyValue{ + Key: tracetranslator.TagSpanKind, + VType: model.ValueType_STRING, + VStr: string(tracetranslator.OpenTracingSpanKindProducer), }, + ok: true, }, { name: "consumer", kind: pdata.SpanKindCONSUMER, - tags: []model.KeyValue{ - { - Key: tracetranslator.TagSpanKind, - VType: model.ValueType_STRING, - VStr: string(tracetranslator.OpenTracingSpanKindConsumer), - }, + tag: model.KeyValue{ + Key: tracetranslator.TagSpanKind, + VType: model.ValueType_STRING, + VStr: string(tracetranslator.OpenTracingSpanKindConsumer), }, + ok: true, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - tags := appendTagFromSpanKind(nil, test.kind) - assert.EqualValues(t, test.tags, tags) + got, ok := getTagFromSpanKind(test.kind) + assert.Equal(t, test.ok, ok) + assert.EqualValues(t, test.tag, got) }) } } @@ -219,13 +211,11 @@ func TestAttributesToJaegerProtoTags(t *testing.T) { }, } - require.EqualValues(t, expected, attributesToJaegerProtoTags(attributes)) - - got := attributesToJaegerProtoTags(attributes) + got := appendTagsFromAttributes(make([]model.KeyValue, 0, len(expected)), attributes) require.EqualValues(t, expected, got) // The last item in expected ("service-name") must be skipped in resource tags translation - got = resourceAttributesToJaegerProtoTags(attributes) + got = appendTagsFromResourceAttributes(make([]model.KeyValue, 0, len(expected)-1), attributes) require.EqualValues(t, expected[:4], got) } @@ -319,3 +309,14 @@ func generateProtoChildSpanWithErrorTags() *model.Span { }) return span } + +func BenchmarkInternalTracesToJaegerProto(b *testing.B) { + td := generateTraceDataTwoSpansChildParent() + resource := generateTraceDataResourceOnly().ResourceSpans().At(0).Resource() + resource.CopyTo(td.ResourceSpans().At(0).Resource()) + + b.ResetTimer() + for n := 0; n < b.N; n++ { + InternalTracesToJaegerProto(td) + } +}