From 0a9e86154979ce05eecc7d9b056e25b3ab2e29bc Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 8 Apr 2020 16:49:53 -0700 Subject: [PATCH] Update the OTLP exporter batching of traces (#623) Instead of batching based on the Resource pointer which is not guaranteed to uniquely identify common Resources, use the `String` method which is. Renames the external test package to be identified as the integration testing. This testing structure (the mock exporter) batches exported resources transmitted from the resource. This is needed as there is no guarantees that a batch will be exported in a single payload, therefore, it is not an ideal place to add resource batch testing for the exporter itself. Add unit tests to ensure the Exporter is correctly batching spans by resource. --- exporters/otlp/internal/transform/span.go | 13 +- .../otlp/internal/transform/span_test.go | 8 +- ...{otlp_test.go => otlp_integration_test.go} | 0 exporters/otlp/otlp_span_test.go | 253 ++++++++++++++++++ 4 files changed, 268 insertions(+), 6 deletions(-) rename exporters/otlp/{otlp_test.go => otlp_integration_test.go} (100%) create mode 100644 exporters/otlp/otlp_span_test.go diff --git a/exporters/otlp/internal/transform/span.go b/exporters/otlp/internal/transform/span.go index ca67527cbe3..d42decece85 100644 --- a/exporters/otlp/internal/transform/span.go +++ b/exporters/otlp/internal/transform/span.go @@ -21,7 +21,6 @@ import ( apitrace "go.opentelemetry.io/otel/api/trace" export "go.opentelemetry.io/otel/sdk/export/trace" - "go.opentelemetry.io/otel/sdk/resource" ) const ( @@ -33,11 +32,17 @@ func SpanData(sdl []*export.SpanData) []*tracepb.ResourceSpans { if len(sdl) == 0 { return nil } - rsm := make(map[*resource.Resource]*tracepb.ResourceSpans) + // Group by the unique string representation of the Resource. + rsm := make(map[string]*tracepb.ResourceSpans) for _, sd := range sdl { if sd != nil { - rs, ok := rsm[sd.Resource] + var key string + if sd.Resource != nil { + key = sd.Resource.String() + } + + rs, ok := rsm[key] if !ok { rs = &tracepb.ResourceSpans{ Resource: Resource(sd.Resource), @@ -47,7 +52,7 @@ func SpanData(sdl []*export.SpanData) []*tracepb.ResourceSpans { }, }, } - rsm[sd.Resource] = rs + rsm[key] = rs } rs.InstrumentationLibrarySpans[0].Spans = append(rs.InstrumentationLibrarySpans[0].Spans, span(sd)) diff --git a/exporters/otlp/internal/transform/span_test.go b/exporters/otlp/internal/transform/span_test.go index 9525e3e2569..52c28b9735b 100644 --- a/exporters/otlp/internal/transform/span_test.go +++ b/exporters/otlp/internal/transform/span_test.go @@ -333,8 +333,8 @@ func TestSpanData(t *testing.T) { ParentSpanId: []byte{0xEF, 0xEE, 0xED, 0xEC, 0xEB, 0xEA, 0xE9, 0xE8}, Name: spanData.Name, Kind: tracepb.Span_SERVER, - StartTimeUnixNano: uint64(1585674086000001234), - EndTimeUnixNano: uint64(1585674096000001234), + StartTimeUnixNano: uint64(startTime.UnixNano()), + EndTimeUnixNano: uint64(endTime.UnixNano()), Status: status(spanData.StatusCode, spanData.StatusMessage), Events: spanEvents(spanData.MessageEvents), Links: links(spanData.Links), @@ -369,3 +369,7 @@ func TestRootSpanData(t *testing.T) { // Empty means root span. assert.Nil(t, got, "incorrect transform of root parent span ID") } + +func TestSpanDataNilResource(t *testing.T) { + assert.NotPanics(t, func() { SpanData([]*export.SpanData{{}}) }) +} diff --git a/exporters/otlp/otlp_test.go b/exporters/otlp/otlp_integration_test.go similarity index 100% rename from exporters/otlp/otlp_test.go rename to exporters/otlp/otlp_integration_test.go diff --git a/exporters/otlp/otlp_span_test.go b/exporters/otlp/otlp_span_test.go new file mode 100644 index 00000000000..1f336de55ff --- /dev/null +++ b/exporters/otlp/otlp_span_test.go @@ -0,0 +1,253 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package otlp + +import ( + "context" + "testing" + "time" + + coltracepb "github.com/open-telemetry/opentelemetry-proto/gen/go/collector/trace/v1" + commonpb "github.com/open-telemetry/opentelemetry-proto/gen/go/common/v1" + resourcepb "github.com/open-telemetry/opentelemetry-proto/gen/go/resource/v1" + tracepb "github.com/open-telemetry/opentelemetry-proto/gen/go/trace/v1" + "github.com/stretchr/testify/assert" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + + "go.opentelemetry.io/otel/api/core" + apitrace "go.opentelemetry.io/otel/api/trace" + tracesdk "go.opentelemetry.io/otel/sdk/export/trace" + "go.opentelemetry.io/otel/sdk/resource" +) + +type traceServiceClientStub struct { + rs []tracepb.ResourceSpans +} + +func (t *traceServiceClientStub) Export(ctx context.Context, in *coltracepb.ExportTraceServiceRequest, opts ...grpc.CallOption) (*coltracepb.ExportTraceServiceResponse, error) { + for _, rs := range in.GetResourceSpans() { + if rs == nil { + continue + } + t.rs = append(t.rs, *rs) + } + return &coltracepb.ExportTraceServiceResponse{}, nil +} + +func (t *traceServiceClientStub) ResourceSpans() []tracepb.ResourceSpans { + return t.rs +} + +func (t *traceServiceClientStub) Reset() { + t.rs = nil +} + +func TestExportSpans(t *testing.T) { + tsc := &traceServiceClientStub{} + exp := NewUnstartedExporter() + exp.traceExporter = tsc + exp.started = true + + // March 31, 2020 5:01:26 1234nanos (UTC) + startTime := time.Unix(1585674086, 1234) + endTime := startTime.Add(10 * time.Second) + + for _, test := range []struct { + sd []*tracesdk.SpanData + want []tracepb.ResourceSpans + }{ + { + []*tracesdk.SpanData(nil), + []tracepb.ResourceSpans(nil), + }, + { + []*tracesdk.SpanData{}, + []tracepb.ResourceSpans(nil), + }, + { + []*tracesdk.SpanData{ + { + SpanContext: core.SpanContext{ + TraceID: core.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}), + SpanID: core.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 1}), + TraceFlags: byte(1), + }, + SpanKind: apitrace.SpanKindServer, + Name: "parent process", + StartTime: startTime, + EndTime: endTime, + Attributes: []core.KeyValue{ + core.Key("user").String("alice"), + core.Key("authenticated").Bool(true), + }, + StatusCode: codes.OK, + StatusMessage: "Ok", + Resource: resource.New(core.Key("instance").String("tester-a")), + }, + { + SpanContext: core.SpanContext{ + TraceID: core.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}), + SpanID: core.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 2}), + TraceFlags: byte(1), + }, + ParentSpanID: core.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 1}), + SpanKind: apitrace.SpanKindInternal, + Name: "internal process", + StartTime: startTime, + EndTime: endTime, + Attributes: []core.KeyValue{ + core.Key("user").String("alice"), + core.Key("authenticated").Bool(true), + }, + StatusCode: codes.OK, + StatusMessage: "Ok", + Resource: resource.New(core.Key("instance").String("tester-a")), + }, + { + SpanContext: core.SpanContext{ + TraceID: core.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}), + SpanID: core.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 1}), + TraceFlags: byte(1), + }, + SpanKind: apitrace.SpanKindServer, + Name: "parent process", + StartTime: startTime, + EndTime: endTime, + Attributes: []core.KeyValue{ + core.Key("user").String("bob"), + core.Key("authenticated").Bool(false), + }, + StatusCode: codes.Unauthenticated, + StatusMessage: "Unauthenticated", + Resource: resource.New(core.Key("instance").String("tester-b")), + }, + }, + []tracepb.ResourceSpans{ + { + Resource: &resourcepb.Resource{ + Attributes: []*commonpb.AttributeKeyValue{ + { + Key: "instance", + Type: commonpb.AttributeKeyValue_STRING, + StringValue: "tester-a", + }, + }, + }, + InstrumentationLibrarySpans: []*tracepb.InstrumentationLibrarySpans{ + { + Spans: []*tracepb.Span{ + { + TraceId: []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + SpanId: []byte{0, 0, 0, 0, 0, 0, 0, 1}, + Name: "parent process", + Kind: tracepb.Span_SERVER, + StartTimeUnixNano: uint64(startTime.UnixNano()), + EndTimeUnixNano: uint64(endTime.UnixNano()), + Attributes: []*commonpb.AttributeKeyValue{ + { + Key: "user", + Type: commonpb.AttributeKeyValue_STRING, + StringValue: "alice", + }, + { + Key: "authenticated", + Type: commonpb.AttributeKeyValue_BOOL, + BoolValue: true, + }, + }, + Status: &tracepb.Status{ + Code: tracepb.Status_Ok, + Message: "Ok", + }, + }, + { + TraceId: []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, + SpanId: []byte{0, 0, 0, 0, 0, 0, 0, 2}, + ParentSpanId: []byte{0, 0, 0, 0, 0, 0, 0, 1}, + Name: "internal process", + Kind: tracepb.Span_INTERNAL, + StartTimeUnixNano: uint64(startTime.UnixNano()), + EndTimeUnixNano: uint64(endTime.UnixNano()), + Attributes: []*commonpb.AttributeKeyValue{ + { + Key: "user", + Type: commonpb.AttributeKeyValue_STRING, + StringValue: "alice", + }, + { + Key: "authenticated", + Type: commonpb.AttributeKeyValue_BOOL, + BoolValue: true, + }, + }, + Status: &tracepb.Status{ + Code: tracepb.Status_Ok, + Message: "Ok", + }, + }, + }, + }, + }, + }, + { + Resource: &resourcepb.Resource{ + Attributes: []*commonpb.AttributeKeyValue{ + { + Key: "instance", + Type: commonpb.AttributeKeyValue_STRING, + StringValue: "tester-b", + }, + }, + }, + InstrumentationLibrarySpans: []*tracepb.InstrumentationLibrarySpans{ + { + Spans: []*tracepb.Span{ + { + TraceId: []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}, + SpanId: []byte{0, 0, 0, 0, 0, 0, 0, 1}, + Name: "parent process", + Kind: tracepb.Span_SERVER, + StartTimeUnixNano: uint64(startTime.UnixNano()), + EndTimeUnixNano: uint64(endTime.UnixNano()), + Attributes: []*commonpb.AttributeKeyValue{ + { + Key: "user", + Type: commonpb.AttributeKeyValue_STRING, + StringValue: "bob", + }, + { + Key: "authenticated", + Type: commonpb.AttributeKeyValue_BOOL, + BoolValue: false, + }, + }, + Status: &tracepb.Status{ + Code: tracepb.Status_Unauthenticated, + Message: "Unauthenticated", + }, + }, + }, + }, + }, + }, + }, + }, + } { + tsc.Reset() + exp.ExportSpans(context.Background(), test.sd) + assert.ElementsMatch(t, test.want, tsc.ResourceSpans()) + } +}