From 7667f7ba2580f7b822d6652c09685f9fac5046b5 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Mon, 1 Apr 2024 10:30:34 -0400 Subject: [PATCH] Add support for AddLink to the OpenCensus bridge (#5116) * add support for AddLink to the OpenCensus bridge * Update CHANGELOG.md Co-authored-by: Tyler Yahn --------- Co-authored-by: Tyler Yahn --- CHANGELOG.md | 1 + bridge/opencensus/doc.go | 2 - .../opencensus/internal/oc2otel/attributes.go | 11 +++++ .../internal/oc2otel/attributes_test.go | 23 +++++++++ bridge/opencensus/internal/span.go | 14 +++++- bridge/opencensus/internal/span_test.go | 47 +++++++++++++++++-- 6 files changed, 90 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59d54dcca22..c6a9a9c1aad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm At which point, users will be required to migrage their code, and this package will be deprecated then removed. (#5085) - Add support for `Summary` metrics in the `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` exporters. (#5100) - Add `otel.scope.name` and `otel.scope.version` tags to spans exported by `go.opentelemetry.io/otel/exporters/zipkin`. (#5108) +- Add support for `AddLink` to `go.opentelemetry.io/otel/bridge/opencensus`. (#5116) ### Changed diff --git a/bridge/opencensus/doc.go b/bridge/opencensus/doc.go index 8d363ce4dbc..0f5d4abb8cf 100644 --- a/bridge/opencensus/doc.go +++ b/bridge/opencensus/doc.go @@ -37,8 +37,6 @@ // // There are known limitations to the trace bridge: // -// - The AddLink method for OpenCensus Spans is ignored, and an error is sent -// to the OpenTelemetry ErrorHandler. // - The NewContext method of the OpenCensus Tracer cannot embed an OpenCensus // Span in a context unless that Span was created by that Tracer. // - Conversion of custom OpenCensus Samplers to OpenTelemetry is not diff --git a/bridge/opencensus/internal/oc2otel/attributes.go b/bridge/opencensus/internal/oc2otel/attributes.go index 1b9e931e073..7c6ae45d583 100644 --- a/bridge/opencensus/internal/oc2otel/attributes.go +++ b/bridge/opencensus/internal/oc2otel/attributes.go @@ -20,6 +20,17 @@ func Attributes(attr []octrace.Attribute) []attribute.KeyValue { return otelAttr } +func AttributesFromMap(attr map[string]interface{}) []attribute.KeyValue { + otelAttr := make([]attribute.KeyValue, 0, len(attr)) + for k, v := range attr { + otelAttr = append(otelAttr, attribute.KeyValue{ + Key: attribute.Key(k), + Value: AttributeValue(v), + }) + } + return otelAttr +} + func AttributeValue(ocval interface{}) attribute.Value { switch v := ocval.(type) { case bool: diff --git a/bridge/opencensus/internal/oc2otel/attributes_test.go b/bridge/opencensus/internal/oc2otel/attributes_test.go index 8c1447341fd..7e3efeed723 100644 --- a/bridge/opencensus/internal/oc2otel/attributes_test.go +++ b/bridge/opencensus/internal/oc2otel/attributes_test.go @@ -37,6 +37,29 @@ func TestAttributes(t *testing.T) { } } +func TestAttributesFromMap(t *testing.T) { + in := map[string]interface{}{ + "bool": true, + "int64": int64(49), + "float64": float64(1.618), + "key": "val", + } + + want := []attribute.KeyValue{ + attribute.Bool("bool", true), + attribute.Int64("int64", 49), + attribute.Float64("float64", 1.618), + attribute.String("key", "val"), + } + got := AttributesFromMap(in) + + gotAttributeSet := attribute.NewSet(got...) + wantAttributeSet := attribute.NewSet(want...) + if !gotAttributeSet.Equals(&wantAttributeSet) { + t.Errorf("Attributes conversion want %v, got %v", wantAttributeSet.Encoded(attribute.DefaultEncoder()), gotAttributeSet.Encoded(attribute.DefaultEncoder())) + } +} + func TestAttributeValueUnknown(t *testing.T) { got := AttributeValue([]byte{}) if got != attribute.StringValue("unknown") { diff --git a/bridge/opencensus/internal/span.go b/bridge/opencensus/internal/span.go index e3b76064bfc..9e7ee39fb5b 100644 --- a/bridge/opencensus/internal/span.go +++ b/bridge/opencensus/internal/span.go @@ -110,8 +110,20 @@ func (s *Span) AddMessageReceiveEvent(messageID, uncompressedByteSize, compresse } // AddLink adds a link to this span. +// This drops the OpenCensus LinkType because there is no such concept in OpenTelemetry. func (s *Span) AddLink(l octrace.Link) { - Handle(fmt.Errorf("ignoring OpenCensus link %+v for span %q because OpenTelemetry doesn't support setting links after creation", l, s.String())) + s.otelSpan.AddLink(trace.Link{ + SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: trace.TraceID(l.TraceID), + SpanID: trace.SpanID(l.SpanID), + // We don't know if this was sampled or not. + // Mark it as sampled, since sampled means + // "the caller may have recorded trace data": + // https://www.w3.org/TR/trace-context/#sampled-flag + TraceFlags: trace.FlagsSampled, + }), + Attributes: oc2otel.AttributesFromMap(l.Attributes), + }) } // String prints a string representation of this span. diff --git a/bridge/opencensus/internal/span_test.go b/bridge/opencensus/internal/span_test.go index e11632e0252..949018b8f0c 100644 --- a/bridge/opencensus/internal/span_test.go +++ b/bridge/opencensus/internal/span_test.go @@ -28,6 +28,7 @@ type span struct { attrs []attribute.KeyValue eName string eOpts []trace.EventOption + links []trace.Link } func (s *span) IsRecording() bool { return s.recording } @@ -37,6 +38,7 @@ func (s *span) SetName(n string) { s.name = n } func (s *span) SetStatus(c codes.Code, d string) { s.sCode, s.sMsg = c, d } func (s *span) SetAttributes(a ...attribute.KeyValue) { s.attrs = a } func (s *span) AddEvent(n string, o ...trace.EventOption) { s.eName, s.eOpts = n, o } +func (s *span) AddLink(l trace.Link) { s.links = append(s.links, l) } func TestSpanIsRecordingEvents(t *testing.T) { s := &span{recording: true} @@ -230,16 +232,51 @@ func TestSpanAddMessageReceiveEvent(t *testing.T) { } func TestSpanAddLinkFails(t *testing.T) { - h, restore := withHandler() - defer restore() - // OpenCensus does not try to set links if not recording. s := &span{recording: true} ocS := internal.NewSpan(s) ocS.AddLink(octrace.Link{}) + ocS.AddLink(octrace.Link{ + TraceID: octrace.TraceID([16]byte{1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}), + SpanID: octrace.SpanID([8]byte{2, 0, 0, 0, 0, 0, 0, 0}), + Attributes: map[string]interface{}{ + "foo": "bar", + "number": int64(3), + }, + }) + + wantLinks := []trace.Link{ + { + SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceFlags: trace.FlagsSampled, + }), + }, + { + SpanContext: trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: trace.TraceID([]byte{1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}), + SpanID: trace.SpanID([]byte{2, 0, 0, 0, 0, 0, 0, 0}), + TraceFlags: trace.FlagsSampled, + }), + Attributes: []attribute.KeyValue{ + attribute.String("foo", "bar"), + attribute.Int64("number", 3), + }, + }, + } + + if len(s.links) != len(wantLinks) { + t.Fatalf("got wrong number of links; want %v, got %v", len(wantLinks), len(s.links)) + } - if h.err == nil { - t.Error("span.AddLink failed to raise an error") + for i, l := range s.links { + if !l.SpanContext.Equal(wantLinks[i].SpanContext) { + t.Errorf("link[%v] has the wrong span context; want %+v, got %+v", i, wantLinks[i].SpanContext, l.SpanContext) + } + gotAttributeSet := attribute.NewSet(l.Attributes...) + wantAttributeSet := attribute.NewSet(wantLinks[i].Attributes...) + if !gotAttributeSet.Equals(&wantAttributeSet) { + t.Errorf("link[%v] has the wrong attributes; want %v, got %v", i, wantAttributeSet.Encoded(attribute.DefaultEncoder()), gotAttributeSet.Encoded(attribute.DefaultEncoder())) + } } }