From 15e245fb12cbdee302cdabaafbdb364c2b796d81 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Fri, 12 Jan 2024 11:41:37 +0100 Subject: [PATCH 1/8] Make test server return an empty JSON object on abritrary GET URLs --- ddtrace/opentelemetry/span_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ddtrace/opentelemetry/span_test.go b/ddtrace/opentelemetry/span_test.go index f60a0a5f27..3169e5b7aa 100644 --- a/ddtrace/opentelemetry/span_test.go +++ b/ddtrace/opentelemetry/span_test.go @@ -43,6 +43,14 @@ func mockTracerProvider(t *testing.T, opts ...tracer.StartOption) (tp *TracerPro var js bytes.Buffer msgp.UnmarshalAsJSON(&js, buf) payloads <- js.String() + default: + if r.Method == "GET" { + // Write an empty JSON object to the output, to avoid spurious decoding + // errors to be reported in the logs, which may lead someone + // investigating a test failure into the wrong direction. + w.Write([]byte{'{', '}'}) + return + } } w.WriteHeader(200) })) From 71d33c5da0a14cb97d3bed173ced245317c5c88a Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Fri, 12 Jan 2024 13:45:23 -0500 Subject: [PATCH 2/8] log error message if test agent fails receiving traces --- ddtrace/opentelemetry/span_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/opentelemetry/span_test.go b/ddtrace/opentelemetry/span_test.go index 3169e5b7aa..f322964d8f 100644 --- a/ddtrace/opentelemetry/span_test.go +++ b/ddtrace/opentelemetry/span_test.go @@ -38,7 +38,7 @@ func mockTracerProvider(t *testing.T, opts ...tracer.StartOption) (tp *TracerPro } buf, err := io.ReadAll(r.Body) if err != nil || len(buf) == 0 { - t.Fatalf("Test agent: Error receiving traces") + t.Fatalf("Test agent: Error receiving traces: %v", err) } var js bytes.Buffer msgp.UnmarshalAsJSON(&js, buf) From cb176f2d11326bf381613d67a60f025fccb150ac Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Fri, 12 Jan 2024 14:39:02 -0500 Subject: [PATCH 3/8] fix masked error in trace writer --- ddtrace/tracer/writer.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ddtrace/tracer/writer.go b/ddtrace/tracer/writer.go index a4c56e0c90..877c8ada20 100644 --- a/ddtrace/tracer/writer.go +++ b/ddtrace/tracer/writer.go @@ -106,7 +106,8 @@ func (h *agentTraceWriter) flush() { for attempt := 0; attempt <= h.config.sendRetries; attempt++ { size, count = p.size(), p.itemCount() log.Debug("Sending payload: size: %d traces: %d\n", size, count) - rc, err := h.config.transport.send(p) + var rc io.ReadCloser + rc, err = h.config.transport.send(p) if err == nil { log.Debug("sent traces after %d attempts", attempt+1) h.statsd.Count("datadog.tracer.flush_bytes", int64(size), nil, 1) From 3ffe4b9d76d960aadd9e10fd2bf62438619fdcc9 Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Fri, 12 Jan 2024 16:30:42 -0500 Subject: [PATCH 4/8] refactoring tests for future use --- ddtrace/opentelemetry/otel_test.go | 20 ++- ddtrace/opentelemetry/span_test.go | 229 +++++++++++++++++------------ 2 files changed, 146 insertions(+), 103 deletions(-) diff --git a/ddtrace/opentelemetry/otel_test.go b/ddtrace/opentelemetry/otel_test.go index 81ea64d62a..78c17670f9 100644 --- a/ddtrace/opentelemetry/otel_test.go +++ b/ddtrace/opentelemetry/otel_test.go @@ -11,7 +11,6 @@ import ( "context" "net/http" "net/http/httptest" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -23,6 +22,7 @@ import ( ) func TestHttpDistributedTrace(t *testing.T) { + assert := assert.New(t) tp, payloads, cleanup := mockTracerProvider(t) defer cleanup() otel.SetTracerProvider(tp) @@ -33,11 +33,10 @@ func TestHttpDistributedTrace(t *testing.T) { w := otelhttp.NewHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { receivedSpan := oteltrace.SpanFromContext(r.Context()) - assert.Equal(t, rootSpan.SpanContext().TraceID(), receivedSpan.SpanContext().TraceID()) + assert.Equal(rootSpan.SpanContext().TraceID(), receivedSpan.SpanContext().TraceID()) }), "testOperation") testServer := httptest.NewServer(w) defer testServer.Close() - c := http.Client{Transport: otelhttp.NewTransport(nil)} req, err := http.NewRequestWithContext(sctx, http.MethodGet, testServer.URL, nil) require.NoError(t, err) @@ -47,12 +46,11 @@ func TestHttpDistributedTrace(t *testing.T) { rootSpan.End() p := <-payloads - numSpans := strings.Count(p, "\"span_id\"") - assert.Equal(t, 3, numSpans) - assert.Contains(t, p, `"name":"internal"`) - assert.Contains(t, p, `"name":"server.request`) - assert.Contains(t, p, `"name":"client.request"`) - assert.Contains(t, p, `"resource":"testRootSpan"`) - assert.Contains(t, p, `"resource":"testOperation"`) - assert.Contains(t, p, `"resource":"HTTP GET"`) + assert.Len(p, 2) + assert.Equal("server.request", p[0][0]["name"]) + assert.Equal("internal", p[1][0]["name"]) + assert.Equal("client.request", p[1][1]["name"]) + assert.Equal("testOperation", p[0][0]["resource"]) + assert.Equal("testRootSpan", p[1][0]["resource"]) + assert.Equal("HTTP GET", p[1][1]["resource"]) } diff --git a/ddtrace/opentelemetry/span_test.go b/ddtrace/opentelemetry/span_test.go index f322964d8f..243ef5f662 100644 --- a/ddtrace/opentelemetry/span_test.go +++ b/ddtrace/opentelemetry/span_test.go @@ -8,6 +8,7 @@ package opentelemetry import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io" @@ -28,27 +29,39 @@ import ( oteltrace "go.opentelemetry.io/otel/trace" ) -func mockTracerProvider(t *testing.T, opts ...tracer.StartOption) (tp *TracerProvider, payloads chan string, cleanup func()) { - payloads = make(chan string) +type traces [][]map[string]interface{} + +func mockTracerProvider(t *testing.T, opts ...tracer.StartOption) (tp *TracerProvider, payloads chan traces, cleanup func()) { + payloads = make(chan traces) s, c := httpmem.ServerAndClient(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/v0.4/traces": if h := r.Header.Get("X-Datadog-Trace-Count"); h == "0" { return } - buf, err := io.ReadAll(r.Body) + req := r.Clone(context.Background()) + defer req.Body.Close() + buf, err := io.ReadAll(req.Body) if err != nil || len(buf) == 0 { t.Fatalf("Test agent: Error receiving traces: %v", err) } - var js bytes.Buffer - msgp.UnmarshalAsJSON(&js, buf) - payloads <- js.String() + var payload bytes.Buffer + _, err = msgp.UnmarshalAsJSON(&payload, buf) + if err != nil { + t.Fatalf("Failed to unmarshal payload bytes as JSON: %v", err) + } + var tr [][]map[string]interface{} + err = json.Unmarshal(payload.Bytes(), &tr) + if err != nil || len(tr) == 0 { + t.Fatalf("Failed to unmarshal payload bytes as trace: %v", err) + } + payloads <- tr default: if r.Method == "GET" { // Write an empty JSON object to the output, to avoid spurious decoding // errors to be reported in the logs, which may lead someone // investigating a test failure into the wrong direction. - w.Write([]byte{'{', '}'}) + w.Write([]byte("{}")) return } } @@ -63,10 +76,10 @@ func mockTracerProvider(t *testing.T, opts ...tracer.StartOption) (tp *TracerPro } } -func waitForPayload(ctx context.Context, payloads chan string) (string, error) { +func waitForPayload(ctx context.Context, payloads chan traces) (traces, error) { select { case <-ctx.Done(): - return "", fmt.Errorf("Timed out waiting for traces") + return nil, fmt.Errorf("Timed out waiting for traces") case p := <-payloads: return p, nil } @@ -85,12 +98,13 @@ func TestSpanResourceNameDefault(t *testing.T) { sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(p, `"name":"internal"`) - assert.Contains(p, `"resource":"OperationName"`) + p := traces[0] + assert.Equal("internal", p[0]["name"]) + assert.Equal("OperationName", p[0]["resource"]) } func TestSpanSetName(t *testing.T) { @@ -107,11 +121,12 @@ func TestSpanSetName(t *testing.T) { sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(p, strings.ToLower("NewName")) + p := traces[0] + assert.Equal(strings.ToLower("NewName"), p[0]["name"]) } func TestSpanEnd(t *testing.T) { @@ -147,22 +162,22 @@ func TestSpanEnd(t *testing.T) { } tracer.Flush() - payload, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } + p := traces[0] - assert.Contains(payload, name) - assert.NotContains(payload, ignoredName) - assert.Contains(payload, msg) - assert.NotContains(payload, ignoredMsg) - assert.Contains(payload, `"error":1`) // this should be an error span - + assert.Equal(name, p[0]["resource"]) + assert.Equal(ext.SpanKindInternal, p[0]["name"]) // default + assert.Equal(1.0, p[0]["error"]) // this should be an error span + meta := fmt.Sprintf("%v", p[0]["meta"]) + assert.Contains(meta, msg) for k, v := range attributes { - assert.Contains(payload, fmt.Sprintf("\"%s\":\"%s\"", k, v)) + assert.Contains(meta, fmt.Sprintf("%s:%s", k, v)) } for k, v := range ignoredAttributes { - assert.NotContains(payload, fmt.Sprintf("\"%s\":\"%s\"", k, v)) + assert.NotContains(meta, fmt.Sprintf("%s:%s", k, v)) } } @@ -208,19 +223,21 @@ func TestSpanSetStatus(t *testing.T) { testStatus := func() { sp.End() tracer.Flush() - payload, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } + p := traces[0] // An error description is set IFF the span has an error // status code value. Messages related to any other status code // are ignored. + meta := fmt.Sprintf("%v", p[0]["meta"]) if test.code == codes.Error { - assert.Contains(payload, test.msg) + assert.Contains(meta, test.msg) } else { - assert.NotContains(payload, test.msg) + assert.NotContains(meta, test.msg) } - assert.NotContains(payload, test.ignoredCode) + assert.NotContains(meta, test.ignoredCode) } _, sp = tr.Start(context.Background(), "test") sp.SetStatus(test.code, test.msg) @@ -270,21 +287,23 @@ func TestSpanContextWithStartOptions(t *testing.T) { sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - if strings.Count(p, "span_id") != 2 { - t.Fatalf("payload does not contain two spans\n%s", p) - } - assert.Contains(p, `"service":"persisted_srv"`) - assert.Contains(p, `"resource":"persisted_ctx_rsc"`) - assert.Contains(p, `"span.kind":"producer"`) - assert.Contains(p, fmt.Sprint(spanID)) - assert.Contains(p, fmt.Sprint(startTime.UnixNano())) - assert.Contains(p, fmt.Sprint(duration.Nanoseconds())) + p := traces[0] + t.Logf("%v", p[0]) + assert.Len(p, 2) + assert.Equal("persisted_srv", p[0]["service"]) + assert.Equal("persisted_ctx_rsc", p[0]["resource"]) + assert.Equal(1234567890.0, p[0]["span_id"]) + assert.Equal("producer", p[0]["name"]) + meta := fmt.Sprintf("%v", p[0]["meta"]) + assert.Contains(meta, "producer") + assert.Equal(float64(startTime.UnixNano()), p[0]["start"]) + assert.Equal(float64(duration.Nanoseconds()), p[0]["duration"]) assert.NotContains(p, "discarded") - assert.Equal(1, strings.Count(p, `"span_id":1234567890`)) + assert.NotEqual(1234567890.0, p[1]["span_id"]) } func TestSpanContextWithStartOptionsPriorityOrder(t *testing.T) { @@ -307,13 +326,15 @@ func TestSpanContextWithStartOptionsPriorityOrder(t *testing.T) { sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(p, "persisted_ctx_rsc") - assert.Contains(p, "persisted_srv") - assert.Contains(p, `"span.kind":"producer"`) + p := traces[0] + assert.Equal("persisted_srv", p[0]["service"]) + assert.Equal("persisted_ctx_rsc", p[0]["resource"]) + meta := fmt.Sprintf("%v", p[0]["meta"]) + assert.Contains(meta, "producer") assert.NotContains(p, "discarded") } @@ -339,19 +360,18 @@ func TestSpanEndOptionsPriorityOrder(t *testing.T) { EndOptions(sp, tracer.FinishTime(startTime.Add(time.Second*5))) // EndOptions timestamp should prevail sp.End(oteltrace.WithTimestamp(startTime.Add(time.Second * 3))) + duration := time.Second * 5 // making sure end options don't have effect after the span has returned - EndOptions(sp, tracer.FinishTime(startTime.Add(time.Second*2))) + EndOptions(sp, tracer.FinishTime(startTime.Add(duration))) sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(p, `"duration":5000000000,`) - assert.NotContains(p, `"duration":2000000000,`) - assert.NotContains(p, `"duration":1000000000,`) - assert.NotContains(p, `"duration":3000000000,`) + p := traces[0] + assert.Equal(float64(duration.Nanoseconds()), p[0]["duration"]) } func TestSpanEndOptions(t *testing.T) { @@ -362,30 +382,33 @@ func TestSpanEndOptions(t *testing.T) { tr := otel.Tracer("") defer cleanup() + spanID := uint64(1234567890) startTime := time.Now() + duration := time.Second * 5 _, sp := tr.Start( ContextWithStartOptions(context.Background(), tracer.ResourceName("ctx_rsc"), tracer.ServiceName("ctx_srv"), tracer.StartTime(startTime), - tracer.WithSpanID(1234567890), + tracer.WithSpanID(spanID), ), "op_name") - - EndOptions(sp, tracer.FinishTime(startTime.Add(time.Second*5)), + EndOptions(sp, tracer.FinishTime(startTime.Add(duration)), tracer.WithError(errors.New("persisted_option"))) sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(p, "ctx_srv") - assert.Contains(p, "ctx_rsc") - assert.Contains(p, "1234567890") - assert.Contains(p, fmt.Sprint(startTime.UnixNano())) - assert.Contains(p, `"duration":5000000000,`) - assert.Contains(p, `persisted_option`) - assert.Contains(p, `"error":1`) + p := traces[0] + assert.Equal("ctx_srv", p[0]["service"]) + assert.Equal("ctx_rsc", p[0]["resource"]) + assert.Equal(1234567890.0, p[0]["span_id"]) + assert.Equal(float64(startTime.UnixNano()), p[0]["start"]) + assert.Equal(float64(duration.Nanoseconds()), p[0]["duration"]) + meta := fmt.Sprintf("%v", p[0]["meta"]) + assert.Contains(meta, "persisted_option") + assert.Equal(1.0, p[0]["error"]) // this should be an error span } func TestSpanSetAttributes(t *testing.T) { @@ -397,42 +420,55 @@ func TestSpanSetAttributes(t *testing.T) { tr := otel.Tracer("") defer cleanup() - attributes := [][]string{{"k1", "v1_old"}, - {"k2", "v2"}, - {"k1", "v1_new"}, + toBeIgnored := map[string]string{"k1": "v1_old"} + attributes := map[string]string{ + "k2": "v2", + "k1": "v1_new", // maps to 'name' - {"operation.name", "ops"}, + "operation.name": "ops", // maps to 'service' - {"service.name", "srv"}, + "service.name": "srv", // maps to 'resource' - {"resource.name", "rsr"}, + "resource.name": "rsr", // maps to 'type' - {"span.type", "db"}, + "span.type": "db", } _, sp := tr.Start(context.Background(), "test") - for _, tag := range attributes { - sp.SetAttributes(attribute.String(tag[0], tag[1])) + for k, v := range toBeIgnored { + sp.SetAttributes(attribute.String(k, v)) + } + for k, v := range attributes { + sp.SetAttributes(attribute.String(k, v)) } // maps to '_dd1.sr.eausr' sp.SetAttributes(attribute.Int("analytics.event", 1)) sp.End() tracer.Flush() - payload, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(payload, `"k1":"v1_new"`) - assert.Contains(payload, `"k2":"v2"`) - assert.NotContains(payload, "v1_old") + p := traces[0] + meta := fmt.Sprintf("%v", p[0]["meta"]) + for k, v := range toBeIgnored { + assert.NotContains(meta, fmt.Sprintf("%s:%s", k, v)) + } + assert.Contains(meta, fmt.Sprintf("%s:%s", "k1", "v1_new")) + assert.Contains(meta, fmt.Sprintf("%s:%s", "k2", "v2")) // reserved attributes - assert.Contains(payload, `"name":"ops"`) - assert.Contains(payload, `"service":"srv"`) - assert.Contains(payload, `"resource":"rsr"`) - assert.Contains(payload, `"type":"db"`) - assert.Contains(payload, `"_dd1.sr.eausr":1`) + assert.NotContains(meta, fmt.Sprintf("%s:%s", "name", "ops")) + assert.NotContains(meta, fmt.Sprintf("%s:%s", "service", "srv")) + assert.NotContains(meta, fmt.Sprintf("%s:%s", "resource", "rsr")) + assert.NotContains(meta, fmt.Sprintf("%s:%s", "type", "db")) + assert.Equal("ops", p[0]["name"]) + assert.Equal("srv", p[0]["service"]) + assert.Equal("rsr", p[0]["resource"]) + assert.Equal("db", p[0]["type"]) + metrics := fmt.Sprintf("%v", p[0]["metrics"]) + assert.Contains(metrics, fmt.Sprintf("%s:%s", "_dd1.sr.eausr", "1")) } func TestSpanSetAttributesWithRemapping(t *testing.T) { @@ -451,11 +487,12 @@ func TestSpanSetAttributesWithRemapping(t *testing.T) { sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(p, "graphql.server.request") + p := traces[0] + assert.Equal("graphql.server.request", p[0]["name"]) } func TestTracerStartOptions(t *testing.T) { @@ -470,12 +507,14 @@ func TestTracerStartOptions(t *testing.T) { _, sp := tr.Start(context.Background(), "test") sp.End() tracer.Flush() - payload, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(payload, "\"service\":\"test_serv\"") - assert.Contains(payload, "\"env\":\"test_env\"") + p := traces[0] + assert.Equal("test_serv", p[0]["service"]) + meta := fmt.Sprintf("%v", p[0]["meta"]) + assert.Contains(meta, fmt.Sprintf("%s:%s", "env", "test_env")) } func TestOperationNameRemapping(t *testing.T) { @@ -491,13 +530,15 @@ func TestOperationNameRemapping(t *testing.T) { sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(p, "graphql.server.request") + p := traces[0] + assert.Equal("graphql.server.request", p[0]["name"]) } func TestRemapName(t *testing.T) { + assert := assert.New(t) testCases := []struct { spanKind oteltrace.SpanKind in []attribute.KeyValue @@ -620,16 +661,18 @@ func TestRemapName(t *testing.T) { sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(t, p, test.out) + p := traces[0] + assert.Equal(test.out, p[0]["name"]) }) } } func TestRemapWithMultipleSetAttributes(t *testing.T) { + assert := assert.New(t) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -649,13 +692,15 @@ func TestRemapWithMultipleSetAttributes(t *testing.T) { sp.End() tracer.Flush() - p, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(ctx, payloads) if err != nil { t.Fatalf(err.Error()) } - assert.Contains(t, p, `"name":"overriden.name"`) - assert.Contains(t, p, `"resource":"new.name"`) - assert.Contains(t, p, `"service":"new.service.name"`) - assert.Contains(t, p, `"type":"new.span.type"`) - assert.Contains(t, p, `"_dd1.sr.eausr":1`) + p := traces[0] + assert.Equal("overriden.name", p[0]["name"]) + assert.Equal("new.name", p[0]["resource"]) + assert.Equal("new.service.name", p[0]["service"]) + assert.Equal("new.span.type", p[0]["type"]) + metrics := fmt.Sprintf("%v", p[0]["metrics"]) + assert.Contains(metrics, fmt.Sprintf("%s:%s", "_dd1.sr.eausr", "1")) } From 59342b5ed879120454457821cd510e9b631cdd2c Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Fri, 12 Jan 2024 16:35:47 -0500 Subject: [PATCH 5/8] fix opentelemetry/tracer_test.go --- ddtrace/opentelemetry/tracer_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddtrace/opentelemetry/tracer_test.go b/ddtrace/opentelemetry/tracer_test.go index c9a6a3f91a..aa4108cc0f 100644 --- a/ddtrace/opentelemetry/tracer_test.go +++ b/ddtrace/opentelemetry/tracer_test.go @@ -156,10 +156,10 @@ func TestForceFlush(t *testing.T) { _, sp := tr.Start(context.Background(), "test_span") sp.End() tp.forceFlush(tc.timeOut, setFlushStatus, tc.flushFunc) - payload, err := waitForPayload(ctx, payloads) + p, err := waitForPayload(ctx, payloads) if tc.flushed { assert.NoError(err) - assert.Contains(payload, "test_span") + assert.Equal("test_span", p[0][0]["resource"]) assert.Equal(OK, flushStatus) } else { assert.Equal(ERROR, flushStatus) From 3012fcafe65cfd962e04282df465e5ca0a5f0eed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 16 Jan 2024 15:29:13 +0100 Subject: [PATCH 6/8] ddtrace/tracer: fix typo --- ddtrace/tracer/tracer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index 1c86293c40..ddef5ffa6f 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -372,7 +372,7 @@ func (t *tracer) worker(tick <-chan time.Time) { t.statsd.Flush() t.stats.flushAndSend(time.Now(), withCurrentBucket) // TODO(x): In reality, the traceWriter.flush() call is not synchronous - // when using the agent traceWriter. However, this functionnality is used + // when using the agent traceWriter. However, this functionality is used // in Lambda so for that purpose this mechanism should suffice. done <- struct{}{} From 62d6ec6b4cb41c71d75b8c61b2dec52c192d99e8 Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Thu, 18 Jan 2024 12:26:27 -0500 Subject: [PATCH 7/8] log warnings if test cleanup fails --- ddtrace/opentelemetry/span_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ddtrace/opentelemetry/span_test.go b/ddtrace/opentelemetry/span_test.go index 243ef5f662..db36f9bf59 100644 --- a/ddtrace/opentelemetry/span_test.go +++ b/ddtrace/opentelemetry/span_test.go @@ -71,8 +71,12 @@ func mockTracerProvider(t *testing.T, opts ...tracer.StartOption) (tp *TracerPro tp = NewTracerProvider(opts...) otel.SetTracerProvider(tp) return tp, payloads, func() { - s.Close() - tp.Shutdown() + if err := s.Close(); err != nil { + t.Fatalf("Test Agent server Close failure: %v", err) + } + if err := tp.Shutdown(); err != nil { + t.Fatalf("Tracer Provider shutdown failure: %v", err) + } } } From 8b46ec8f71ddf5b794a70a7e1e12114414f8587c Mon Sep 17 00:00:00 2001 From: Katie Hockman Date: Thu, 18 Jan 2024 12:37:41 -0500 Subject: [PATCH 8/8] use time.After for timing out the OTel tests, instead of a test-scoped context --- ddtrace/opentelemetry/span_test.go | 67 ++++++++++------------------ ddtrace/opentelemetry/tracer_test.go | 4 +- 2 files changed, 24 insertions(+), 47 deletions(-) diff --git a/ddtrace/opentelemetry/span_test.go b/ddtrace/opentelemetry/span_test.go index db36f9bf59..127f23fce7 100644 --- a/ddtrace/opentelemetry/span_test.go +++ b/ddtrace/opentelemetry/span_test.go @@ -80,19 +80,18 @@ func mockTracerProvider(t *testing.T, opts ...tracer.StartOption) (tp *TracerPro } } -func waitForPayload(ctx context.Context, payloads chan traces) (traces, error) { +func waitForPayload(payloads chan traces) (traces, error) { select { - case <-ctx.Done(): - return nil, fmt.Errorf("Timed out waiting for traces") case p := <-payloads: return p, nil + case <-time.After(10 * time.Second): + return nil, fmt.Errorf("Timed out waiting for traces") } } func TestSpanResourceNameDefault(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + ctx := context.Background() _, payloads, cleanup := mockTracerProvider(t) tr := otel.Tracer("") @@ -102,7 +101,7 @@ func TestSpanResourceNameDefault(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -113,19 +112,19 @@ func TestSpanResourceNameDefault(t *testing.T) { func TestSpanSetName(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() _, payloads, cleanup := mockTracerProvider(t) tr := otel.Tracer("") defer cleanup() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() _, sp := tr.Start(ctx, "OldName") sp.SetName("NewName") sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -135,8 +134,6 @@ func TestSpanSetName(t *testing.T) { func TestSpanEnd(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() _, payloads, cleanup := mockTracerProvider(t) tr := otel.Tracer("") defer cleanup() @@ -166,7 +163,7 @@ func TestSpanEnd(t *testing.T) { } tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -220,14 +217,11 @@ func TestSpanSetStatus(t *testing.T) { for _, test := range testData { t.Run(fmt.Sprintf("Setting Code: %d", test.code), func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - var sp oteltrace.Span testStatus := func() { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -258,8 +252,6 @@ func TestSpanSetStatus(t *testing.T) { func TestSpanContextWithStartOptions(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() _, payloads, cleanup := mockTracerProvider(t) tr := otel.Tracer("") defer cleanup() @@ -291,7 +283,7 @@ func TestSpanContextWithStartOptions(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -312,8 +304,7 @@ func TestSpanContextWithStartOptions(t *testing.T) { func TestSpanContextWithStartOptionsPriorityOrder(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + _, payloads, cleanup := mockTracerProvider(t) tr := otel.Tracer("") defer cleanup() @@ -330,7 +321,7 @@ func TestSpanContextWithStartOptionsPriorityOrder(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -344,8 +335,7 @@ func TestSpanContextWithStartOptionsPriorityOrder(t *testing.T) { func TestSpanEndOptionsPriorityOrder(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + _, payloads, cleanup := mockTracerProvider(t) tr := otel.Tracer("") defer cleanup() @@ -370,7 +360,7 @@ func TestSpanEndOptionsPriorityOrder(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -380,8 +370,7 @@ func TestSpanEndOptionsPriorityOrder(t *testing.T) { func TestSpanEndOptions(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() + _, payloads, cleanup := mockTracerProvider(t) tr := otel.Tracer("") defer cleanup() @@ -400,7 +389,7 @@ func TestSpanEndOptions(t *testing.T) { tracer.WithError(errors.New("persisted_option"))) sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -417,8 +406,6 @@ func TestSpanEndOptions(t *testing.T) { func TestSpanSetAttributes(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() _, payloads, cleanup := mockTracerProvider(t) tr := otel.Tracer("") @@ -450,7 +437,7 @@ func TestSpanSetAttributes(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -491,7 +478,7 @@ func TestSpanSetAttributesWithRemapping(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -501,8 +488,6 @@ func TestSpanSetAttributesWithRemapping(t *testing.T) { func TestTracerStartOptions(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() _, payloads, cleanup := mockTracerProvider(t, tracer.WithEnv("test_env"), tracer.WithService("test_serv")) tr := otel.Tracer("") @@ -511,7 +496,7 @@ func TestTracerStartOptions(t *testing.T) { _, sp := tr.Start(context.Background(), "test") sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -534,7 +519,7 @@ func TestOperationNameRemapping(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -650,10 +635,6 @@ func TestRemapName(t *testing.T) { out: "internal", }, } - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - _, payloads, cleanup := mockTracerProvider(t, tracer.WithEnv("test_env"), tracer.WithService("test_serv")) tr := otel.Tracer("") defer cleanup() @@ -665,7 +646,7 @@ func TestRemapName(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } @@ -677,8 +658,6 @@ func TestRemapName(t *testing.T) { func TestRemapWithMultipleSetAttributes(t *testing.T) { assert := assert.New(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() _, payloads, cleanup := mockTracerProvider(t, tracer.WithEnv("test_env"), tracer.WithService("test_serv")) tr := otel.Tracer("") @@ -696,7 +675,7 @@ func TestRemapWithMultipleSetAttributes(t *testing.T) { sp.End() tracer.Flush() - traces, err := waitForPayload(ctx, payloads) + traces, err := waitForPayload(payloads) if err != nil { t.Fatalf(err.Error()) } diff --git a/ddtrace/opentelemetry/tracer_test.go b/ddtrace/opentelemetry/tracer_test.go index aa4108cc0f..92e9357da2 100644 --- a/ddtrace/opentelemetry/tracer_test.go +++ b/ddtrace/opentelemetry/tracer_test.go @@ -139,8 +139,6 @@ func TestForceFlush(t *testing.T) { } for _, tc := range testData { t.Run(fmt.Sprintf("Flush success: %t", tc.flushed), func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), time.Second) - defer cancel() tp, payloads, cleanup := mockTracerProvider(t) defer cleanup() @@ -156,7 +154,7 @@ func TestForceFlush(t *testing.T) { _, sp := tr.Start(context.Background(), "test_span") sp.End() tp.forceFlush(tc.timeOut, setFlushStatus, tc.flushFunc) - p, err := waitForPayload(ctx, payloads) + p, err := waitForPayload(payloads) if tc.flushed { assert.NoError(err) assert.Equal("test_span", p[0][0]["resource"])