From 2d346e5b89a571fb150dcceace26abcc8819d0bd Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Thu, 11 Apr 2024 10:32:37 +0200 Subject: [PATCH] Allow adding links after tx/span creation in agent, and handle `AddLink` in apmotel (#1605) * fix(apmotel): add noop addlinks for compatibility with newer otel version * handle apmotel's links --------- Co-authored-by: kruskal <99559985+kruskall@users.noreply.github.com> --- module/apmotel/go.mod | 16 ++--- module/apmotel/go.sum | 32 +++++----- module/apmotel/span.go | 16 +++++ module/apmotel/span_test.go | 122 ++++++++++++++++++++++++++++++++++++ module/apmotel/tracer.go | 13 +++- span.go | 11 ++++ span_test.go | 28 +++++++++ transaction.go | 11 ++++ transaction_test.go | 23 +++++++ 9 files changed, 247 insertions(+), 25 deletions(-) diff --git a/module/apmotel/go.mod b/module/apmotel/go.mod index c954e889a..0aeae8c53 100644 --- a/module/apmotel/go.mod +++ b/module/apmotel/go.mod @@ -1,14 +1,14 @@ module go.elastic.co/apm/module/apmotel/v2 require ( - github.com/stretchr/testify v1.8.4 + github.com/stretchr/testify v1.9.0 go.elastic.co/apm/module/apmhttp/v2 v2.5.0 go.elastic.co/apm/v2 v2.5.0 - go.opentelemetry.io/otel v1.21.0 - go.opentelemetry.io/otel/metric v1.21.0 - go.opentelemetry.io/otel/sdk v1.17.0 - go.opentelemetry.io/otel/sdk/metric v0.40.0 - go.opentelemetry.io/otel/trace v1.21.0 + go.opentelemetry.io/otel v1.25.0 + go.opentelemetry.io/otel/metric v1.25.0 + go.opentelemetry.io/otel/sdk v1.25.0 + go.opentelemetry.io/otel/sdk/metric v1.25.0 + go.opentelemetry.io/otel/trace v1.25.0 ) require ( @@ -16,7 +16,7 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/elastic/go-sysinfo v1.7.1 // indirect github.com/elastic/go-windows v1.0.1 // indirect - github.com/go-logr/logr v1.3.0 // indirect + github.com/go-logr/logr v1.4.1 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/google/go-cmp v0.6.0 // indirect github.com/joeshaw/multierror v0.0.0-20140124173710-69b34d4ec901 // indirect @@ -24,7 +24,7 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/procfs v0.7.3 // indirect go.elastic.co/fastjson v1.1.0 // indirect - golang.org/x/sys v0.11.0 // indirect + golang.org/x/sys v0.18.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect howett.net/plist v1.0.0 // indirect ) diff --git a/module/apmotel/go.sum b/module/apmotel/go.sum index 544681ba4..f1ab35d5f 100644 --- a/module/apmotel/go.sum +++ b/module/apmotel/go.sum @@ -9,8 +9,8 @@ github.com/elastic/go-windows v1.0.0/go.mod h1:TsU0Nrp7/y3+VwE82FoZF8gC/XFg/Elz6 github.com/elastic/go-windows v1.0.1 h1:AlYZOldA+UJ0/2nBuqWdo90GFCgG9xuyw9SYzGUtJm0= github.com/elastic/go-windows v1.0.1/go.mod h1:FoVvqWSun28vaDQPbj2Elfc0JahhPB7WQEGa3c814Ss= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= -github.com/go-logr/logr v1.3.0 h1:2y3SDp0ZXuc6/cjLSZ+Q3ir+QB9T/iG5yYRXqsagWSY= -github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= +github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= @@ -35,21 +35,21 @@ github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0 github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= go.elastic.co/fastjson v1.1.0 h1:3MrGBWWVIxe/xvsbpghtkFoPciPhOCmjsR/HfwEeQR4= go.elastic.co/fastjson v1.1.0/go.mod h1:boNGISWMjQsUPy/t6yqt2/1Wx4YNPSe+mZjlyw9vKKI= -go.opentelemetry.io/otel v1.21.0 h1:hzLeKBZEL7Okw2mGzZ0cc4k/A7Fta0uoPgaJCr8fsFc= -go.opentelemetry.io/otel v1.21.0/go.mod h1:QZzNPQPm1zLX4gZK4cMi+71eaorMSGT3A4znnUvNNEo= -go.opentelemetry.io/otel/metric v1.21.0 h1:tlYWfeo+Bocx5kLEloTjbcDwBuELRrIFxwdQ36PlJu4= -go.opentelemetry.io/otel/metric v1.21.0/go.mod h1:o1p3CA8nNHW8j5yuQLdc1eeqEaPfzug24uvsyIEJRWM= -go.opentelemetry.io/otel/sdk v1.17.0 h1:FLN2X66Ke/k5Sg3V623Q7h7nt3cHXaW1FOvKKrW0IpE= -go.opentelemetry.io/otel/sdk v1.17.0/go.mod h1:U87sE0f5vQB7hwUoW98pW5Rz4ZDuCFBZFNUBlSgmDFQ= -go.opentelemetry.io/otel/sdk/metric v0.40.0 h1:qOM29YaGcxipWjL5FzpyZDpCYrDREvX0mVlmXdOjCHU= -go.opentelemetry.io/otel/sdk/metric v0.40.0/go.mod h1:dWxHtdzdJvg+ciJUKLTKwrMe5P6Dv3FyDbh8UkfgkVs= -go.opentelemetry.io/otel/trace v1.21.0 h1:WD9i5gzvoUPuXIXH24ZNBudiarZDKuekPqi/E8fpfLc= -go.opentelemetry.io/otel/trace v1.21.0/go.mod h1:LGbsEB0f9LGjN+OZaQQ26sohbOmiMR+BaslueVtS/qQ= +go.opentelemetry.io/otel v1.25.0 h1:gldB5FfhRl7OJQbUHt/8s0a7cE8fbsPAtdpRaApKy4k= +go.opentelemetry.io/otel v1.25.0/go.mod h1:Wa2ds5NOXEMkCmUou1WA7ZBfLTHWIsp034OVD7AO+Vg= +go.opentelemetry.io/otel/metric v1.25.0 h1:LUKbS7ArpFL/I2jJHdJcqMGxkRdxpPHE0VU/D4NuEwA= +go.opentelemetry.io/otel/metric v1.25.0/go.mod h1:rkDLUSd2lC5lq2dFNrX9LGAbINP5B7WBkC78RXCpH5s= +go.opentelemetry.io/otel/sdk v1.25.0 h1:PDryEJPC8YJZQSyLY5eqLeafHtG+X7FWnf3aXMtxbqo= +go.opentelemetry.io/otel/sdk v1.25.0/go.mod h1:oFgzCM2zdsxKzz6zwpTZYLLQsFwc+K0daArPdIhuxkw= +go.opentelemetry.io/otel/sdk/metric v1.25.0 h1:7CiHOy08LbrxMAp4vWpbiPcklunUshVpAvGBrdDRlGw= +go.opentelemetry.io/otel/sdk/metric v1.25.0/go.mod h1:LzwoKptdbBBdYfvtGCzGwk6GWMA3aUzBOwtQpR6Nz7o= +go.opentelemetry.io/otel/trace v1.25.0 h1:tqukZGLwQYRIFtSQM2u2+yfMVTgGVeqRLPUYx1Dq6RM= +go.opentelemetry.io/otel/trace v1.25.0/go.mod h1:hCCs70XM/ljO+BeQkyFnbK28SBIJ/Emuha+ccrCRT7I= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= @@ -65,8 +65,8 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191025021431-6c3a3bfe00ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= -golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= diff --git a/module/apmotel/span.go b/module/apmotel/span.go index bcc47088e..390e3a74f 100644 --- a/module/apmotel/span.go +++ b/module/apmotel/span.go @@ -120,6 +120,22 @@ func (s *span) AddEvent(name string, opts ...trace.EventOption) { s.mu.Unlock() } +func (s *span) AddLink(tl trace.Link) { + s.mu.Lock() + defer s.mu.Unlock() + + l := apm.SpanLink{ + Trace: [16]byte(tl.SpanContext.TraceID()), + Span: [8]byte(tl.SpanContext.SpanID()), + } + + if s.span != nil { + s.span.AddLink(l) + } else { + s.tx.AddLink(l) + } +} + func (s *span) IsRecording() bool { if s.span != nil { return !s.span.Dropped() diff --git a/module/apmotel/span_test.go b/module/apmotel/span_test.go index 8924cdbcd..ad0e0cce9 100644 --- a/module/apmotel/span_test.go +++ b/module/apmotel/span_test.go @@ -226,6 +226,27 @@ func TestSpanEnd(t *testing.T) { }, }, }, + { + name: "a root span with links", + getSpan: func(ctx context.Context, tracer trace.Tracer) trace.Span { + ctx, s := tracer.Start(ctx, "name", trace.WithLinks(trace.Link{})) + return s + }, + expectedTransactions: []model.Transaction{ + { + Name: "name", + Type: "unknown", + Outcome: "unknown", + OTel: &model.OTel{ + SpanKind: "unspecified", + Attributes: map[string]interface{}{}, + }, + Links: []model.SpanLink{ + {}, + }, + }, + }, + }, { name: "with a child span", getSpan: func(ctx context.Context, tracer trace.Tracer) trace.Span { @@ -514,6 +535,28 @@ func TestSpanEnd(t *testing.T) { }, }, }, + { + name: "a child span with links", + getSpan: func(ctx context.Context, tracer trace.Tracer) trace.Span { + ctx, _ = tracer.Start(ctx, "parentSpan") + ctx, s := tracer.Start(ctx, "name", trace.WithLinks(trace.Link{})) + return s + }, + expectedSpans: []model.Span{ + { + Name: "name", + Type: "custom", + Outcome: "unknown", + OTel: &model.OTel{ + SpanKind: "unspecified", + Attributes: map[string]interface{}{}, + }, + Links: []model.SpanLink{ + {}, + }, + }, + }, + }, { name: "with a grand child span", getSpan: func(ctx context.Context, tracer trace.Tracer) trace.Span { @@ -647,6 +690,85 @@ func TestSpanAddEvent(t *testing.T) { }, s.(*span).events) } +func TestSpanAddLink(t *testing.T) { + apmTracer, recorder := transporttest.NewRecorderTracer() + tp, err := NewTracerProvider( + WithAPMTracer(apmTracer), + WithResource(nil), + ) + assert.NoError(t, err) + tracer := newTracer(tp.(*tracerProvider)) + + ctx, _ := tracer.Start(context.Background(), "myTx") + ctx, s := tracer.Start(ctx, "mySpan") + s.AddLink(trace.Link{}) + s.End() + + apmTracer.Flush(nil) + payloads := recorder.Payloads() + + assert.Len(t, payloads.Spans, 1) + + payloads.Spans[0].ID = model.SpanID{} + payloads.Spans[0].TransactionID = model.SpanID{} + payloads.Spans[0].ParentID = model.SpanID{} + payloads.Spans[0].TraceID = model.TraceID{} + payloads.Spans[0].SampleRate = nil + payloads.Spans[0].Duration = 0 + payloads.Spans[0].Timestamp = model.Time{} + + assert.Equal(t, model.Span{ + Name: "mySpan", + Type: "custom", + Outcome: "unknown", + OTel: &model.OTel{ + SpanKind: "unspecified", + Attributes: map[string]interface{}{}, + }, + Links: []model.SpanLink{ + {}, + }, + }, payloads.Spans[0]) +} + +func TestTransactionAddLink(t *testing.T) { + apmTracer, recorder := transporttest.NewRecorderTracer() + tp, err := NewTracerProvider( + WithAPMTracer(apmTracer), + WithResource(nil), + ) + assert.NoError(t, err) + tracer := newTracer(tp.(*tracerProvider)) + + _, s := tracer.Start(context.Background(), "myTx") + s.AddLink(trace.Link{}) + s.End() + + apmTracer.Flush(nil) + payloads := recorder.Payloads() + + assert.Len(t, payloads.Transactions, 1) + + payloads.Transactions[0].ID = model.SpanID{} + payloads.Transactions[0].TraceID = model.TraceID{} + payloads.Transactions[0].SampleRate = nil + payloads.Transactions[0].Duration = 0 + payloads.Transactions[0].Timestamp = model.Time{} + + assert.Equal(t, model.Transaction{ + Name: "myTx", + Type: "unknown", + Outcome: "unknown", + OTel: &model.OTel{ + SpanKind: "unspecified", + Attributes: map[string]interface{}{}, + }, + Links: []model.SpanLink{ + {}, + }, + }, payloads.Transactions[0]) +} + func TestSpanRecording(t *testing.T) { for _, tt := range []struct { name string diff --git a/module/apmotel/tracer.go b/module/apmotel/tracer.go index ace9274b6..b0e78a461 100644 --- a/module/apmotel/tracer.go +++ b/module/apmotel/tracer.go @@ -53,6 +53,14 @@ func (t *tracer) Start(ctx context.Context, spanName string, opts ...trace.SpanS startTime: startTime, } + var links []apm.SpanLink + for _, l := range config.Links() { + links = append(links, apm.SpanLink{ + Trace: [16]byte(l.SpanContext.TraceID()), + Span: [8]byte(l.SpanContext.SpanID()), + }) + } + var psc trace.SpanContext if config.NewRoot() { ctx = trace.ContextWithSpanContext(ctx, psc) @@ -85,6 +93,7 @@ func (t *tracer) Start(ctx context.Context, spanName string, opts ...trace.SpanS s.span = parent.tx.StartSpanOptions(spanName, "", apm.SpanOptions{ Parent: tc, Start: startTime, + Links: links, }) ctx = apm.ContextWithSpan(ctx, s.span) s.tx = parent.tx @@ -97,7 +106,9 @@ func (t *tracer) Start(ctx context.Context, spanName string, opts ...trace.SpanS } } - var tranOpts apm.TransactionOptions + tranOpts := apm.TransactionOptions{ + Links: links, + } if psc.HasTraceID() && psc.HasSpanID() { tranOpts.TraceContext = apm.TraceContext{ Trace: [16]byte(psc.TraceID()), diff --git a/span.go b/span.go index b440c849e..7495fec36 100644 --- a/span.go +++ b/span.go @@ -605,6 +605,17 @@ func (s *Span) IsExitSpan() bool { return s.exit } +// AddLink adds a link. +func (s *Span) AddLink(l SpanLink) { + s.mu.Lock() + defer s.mu.Unlock() + if s.ended() { + return + } + + s.links = append(s.links, l) +} + // aggregateDroppedSpanStats aggregates the current span into the transaction // dropped spans stats timings. // diff --git a/span_test.go b/span_test.go index 029eb2c83..4b79e1d67 100644 --- a/span_test.go +++ b/span_test.go @@ -165,6 +165,34 @@ func TestSpanLink(t *testing.T) { assert.Equal(t, expectedLinks, payloads.Spans[0].Links) } +func TestSpanAddLink(t *testing.T) { + tracer := apmtest.NewRecordingTracer() + defer tracer.Close() + + tx := tracer.StartTransaction("name", "type") + span := tx.StartSpanOptions("name", "type", apm.SpanOptions{}) + + span.AddLink(apm.SpanLink{ + Trace: apm.TraceID{1}, + Span: apm.SpanID{1}, + }) + + span.End() + tx.End() + + tracer.Flush(nil) + + payloads := tracer.Payloads() + require.Len(t, payloads.Spans, 1) + require.Len(t, payloads.Spans[0].Links, 1) + + // Assert span links are identical. + expectedLinks := []model.SpanLink{ + {TraceID: model.TraceID{1}, SpanID: model.SpanID{1}}, + } + assert.Equal(t, expectedLinks, payloads.Spans[0].Links) +} + func TestSpanTiming(t *testing.T) { var spanStart, spanEnd time.Time txStart := time.Now() diff --git a/transaction.go b/transaction.go index 03f62599c..18c41cbe4 100644 --- a/transaction.go +++ b/transaction.go @@ -267,6 +267,17 @@ func (tx *Transaction) EnsureParent() SpanID { return tx.parentID } +// AddLink adds a link. +func (tx *Transaction) AddLink(l SpanLink) { + tx.mu.Lock() + defer tx.mu.Unlock() + if tx.ended() { + return + } + + tx.links = append(tx.links, l) +} + // ParentID returns the ID of the transaction's Parent or a zero (invalid) SpanID. func (tx *Transaction) ParentID() SpanID { if tx == nil { diff --git a/transaction_test.go b/transaction_test.go index 9b605de57..7892cc9c2 100644 --- a/transaction_test.go +++ b/transaction_test.go @@ -511,6 +511,29 @@ func TestTransactionSpanLink(t *testing.T) { assert.Equal(t, expectedLinks, payloads.Transactions[0].Links) } +func TestTransactionAddLink(t *testing.T) { + tracer := apmtest.NewRecordingTracer() + defer tracer.Close() + + tx := tracer.StartTransactionOptions("name", "type", apm.TransactionOptions{}) + tx.AddLink(apm.SpanLink{ + Trace: apm.TraceID{1}, + Span: apm.SpanID{1}, + }) + tx.End() + + tracer.Flush(nil) + + payloads := tracer.Payloads() + assert.Len(t, payloads.Transactions, 1) + + // Assert span links are identical. + expectedLinks := []model.SpanLink{ + {TraceID: model.TraceID{1}, SpanID: model.SpanID{1}}, + } + assert.Equal(t, expectedLinks, payloads.Transactions[0].Links) +} + func TestTransactionDiscard(t *testing.T) { tracer, transport := transporttest.NewRecorderTracer() defer tracer.Close()