Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TraceState to SpanContext in API #1340

Merged
merged 15 commits into from
Dec 21, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- An `EventOption` and the related `NewEventConfig` function are added to the `go.opentelemetry.io/otel` package to configure Span events. (#1254)
- A `TextMapPropagator` and associated `TextMapCarrier` are added to the `go.opentelemetry.io/otel/oteltest` package to test `TextMap` type propagators and their use. (#1259)
- `SpanContextFromContext` returns `SpanContext` from context. (#1255)
- `TraceState` has been added to `SpanContext`. (#1340)
- `DeploymentEnvironmentKey` added to `go.opentelemetry.io/otel/semconv` package. (#1323)
- Add an OpenCensus to OpenTelemetry tracing bridge. (#1305)
- Add a parent context argument to `SpanProcessor.OnStart` to follow the specification. (#1333)
Expand Down
2 changes: 1 addition & 1 deletion bridge/opencensus/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestOCSpanContextToOTel(t *testing.T) {
} {
t.Run(tc.description, func(t *testing.T) {
output := OCSpanContextToOTel(tc.input)
if output != tc.expected {
if !output.IsEqualWith(tc.expected) {
t.Fatalf("Got %+v spancontext, exepected %+v.", output, tc.expected)
}
})
Expand Down
22 changes: 11 additions & 11 deletions exporters/otlp/internal/transform/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,17 @@ func span(sd *export.SpanData) *tracepb.Span {
}

s := &tracepb.Span{
TraceId: sd.SpanContext.TraceID[:],
SpanId: sd.SpanContext.SpanID[:],
Status: status(sd.StatusCode, sd.StatusMessage),
StartTimeUnixNano: uint64(sd.StartTime.UnixNano()),
EndTimeUnixNano: uint64(sd.EndTime.UnixNano()),
Links: links(sd.Links),
Kind: spanKind(sd.SpanKind),
Name: sd.Name,
Attributes: Attributes(sd.Attributes),
Events: spanEvents(sd.MessageEvents),
// TODO (rghetia): Add Tracestate: when supported.
TraceId: sd.SpanContext.TraceID[:],
SpanId: sd.SpanContext.SpanID[:],
TraceState: sd.SpanContext.TraceState.String(),
Status: status(sd.StatusCode, sd.StatusMessage),
StartTimeUnixNano: uint64(sd.StartTime.UnixNano()),
EndTimeUnixNano: uint64(sd.EndTime.UnixNano()),
Links: links(sd.Links),
Kind: spanKind(sd.SpanKind),
Name: sd.Name,
Attributes: Attributes(sd.Attributes),
Events: spanEvents(sd.MessageEvents),
DroppedAttributesCount: uint32(sd.DroppedAttributeCount),
DroppedEventsCount: uint32(sd.DroppedMessageEventCount),
DroppedLinksCount: uint32(sd.DroppedLinkCount),
Expand Down
7 changes: 5 additions & 2 deletions exporters/otlp/internal/transform/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,12 @@ func TestSpanData(t *testing.T) {
// March 31, 2020 5:01:26 1234nanos (UTC)
startTime := time.Unix(1585674086, 1234)
endTime := startTime.Add(10 * time.Second)
traceState, _ := trace.TraceStateFromKeyValues(label.String("key1", "val1"), label.String("key2", "val2"))
spanData := &export.SpanData{
SpanContext: trace.SpanContext{
TraceID: trace.TraceID{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
SpanID: trace.SpanID{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8},
TraceID: trace.TraceID{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
SpanID: trace.SpanID{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8},
TraceState: traceState,
},
SpanKind: trace.SpanKindServer,
ParentSpanID: trace.SpanID{0xEF, 0xEE, 0xED, 0xEC, 0xEB, 0xEA, 0xE9, 0xE8},
Expand Down Expand Up @@ -266,6 +268,7 @@ func TestSpanData(t *testing.T) {
TraceId: []byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F},
SpanId: []byte{0xFF, 0xFE, 0xFD, 0xFC, 0xFB, 0xFA, 0xF9, 0xF8},
ParentSpanId: []byte{0xEF, 0xEE, 0xED, 0xEC, 0xEB, 0xEA, 0xE9, 0xE8},
TraceState: "key1=val1,key2=val2",
Name: spanData.Name,
Kind: tracepb.Span_SPAN_KIND_SERVER,
StartTimeUnixNano: uint64(startTime.UnixNano()),
Expand Down
13 changes: 10 additions & 3 deletions exporters/stdout/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,16 @@ func TestExporter_ExportSpan(t *testing.T) {
now := time.Now()
traceID, _ := trace.TraceIDFromHex("0102030405060708090a0b0c0d0e0f10")
spanID, _ := trace.SpanIDFromHex("0102030405060708")
traceState, _ := trace.TraceStateFromKeyValues(label.String("key", "val"))
keyValue := "value"
doubleValue := 123.456
resource := resource.NewWithAttributes(label.String("rk1", "rv11"))

testSpan := &export.SpanData{
SpanContext: trace.SpanContext{
TraceID: traceID,
SpanID: spanID,
TraceID: traceID,
SpanID: spanID,
TraceState: traceState,
},
Name: "/foo",
StartTime: now,
Expand All @@ -76,7 +78,12 @@ func TestExporter_ExportSpan(t *testing.T) {
got := b.String()
expectedOutput := `[{"SpanContext":{` +
`"TraceID":"0102030405060708090a0b0c0d0e0f10",` +
`"SpanID":"0102030405060708","TraceFlags":0},` +
`"SpanID":"0102030405060708","TraceFlags":0,` +
`"TraceState":[` +
`{` +
`"Key":"key",` +
`"Value":{"Type":"STRING","Value":"val"}` +
`}]},` +
`"ParentSpanID":"0000000000000000",` +
`"SpanKind":1,` +
`"Name":"/foo",` +
Expand Down
12 changes: 2 additions & 10 deletions oteltest/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type Span struct {
statusMessage string
attributes map[label.Key]label.Value
events []Event
links map[trace.SpanContext][]label.KeyValue
links []trace.Link
spanKind trace.SpanKind
}

Expand Down Expand Up @@ -206,15 +206,7 @@ func (s *Span) Events() []Event { return s.events }

// Links returns the links set on s at creation time. If multiple links for
// the same SpanContext were set, the last link will be used.
func (s *Span) Links() map[trace.SpanContext][]label.KeyValue {
links := make(map[trace.SpanContext][]label.KeyValue)

for sc, attributes := range s.links {
links[sc] = append([]label.KeyValue{}, attributes...)
}

return links
}
func (s *Span) Links() []trace.Link { return s.links }

// StartTime returns the time at which s was started. This will be the
// wall-clock time unless a specific start time was provided.
Expand Down
20 changes: 16 additions & 4 deletions oteltest/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOptio
tracer: t,
startTime: startTime,
attributes: make(map[label.Key]label.Value),
links: make(map[trace.SpanContext][]label.KeyValue),
links: make([]trace.Link, 0),
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
spanKind: c.SpanKind,
}

Expand All @@ -56,10 +56,16 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOptio

iodKey := label.Key("ignored-on-demand")
if lsc := trace.SpanContextFromContext(ctx); lsc.IsValid() {
span.links[lsc] = []label.KeyValue{iodKey.String("current")}
span.links = append(span.links, trace.Link{
SpanContext: lsc,
Attributes: []label.KeyValue{iodKey.String("current")},
})
}
if rsc := trace.RemoteSpanContextFromContext(ctx); rsc.IsValid() {
span.links[rsc] = []label.KeyValue{iodKey.String("remote")}
span.links = append(span.links, trace.Link{
SpanContext: rsc,
Attributes: []label.KeyValue{iodKey.String("remote")},
})
}
} else {
span.spanContext = t.config.SpanContextFunc(ctx)
Expand All @@ -73,7 +79,13 @@ func (t *Tracer) Start(ctx context.Context, name string, opts ...trace.SpanOptio
}

for _, link := range c.Links {
span.links[link.SpanContext] = link.Attributes
for i, sl := range span.links {
if sl.SpanContext.IsEqualWith(link.SpanContext) {
span.links[i].Attributes = link.Attributes
break
}
}
span.links = append(span.links, link)
}

span.SetName(name)
Expand Down
13 changes: 3 additions & 10 deletions oteltest/tracer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,7 @@ func TestTracer(t *testing.T) {
},
},
}
tsLinks := testSpan.Links()
gotLinks := make([]trace.Link, 0, len(tsLinks))
for sc, attributes := range tsLinks {
gotLinks = append(gotLinks, trace.Link{
SpanContext: sc,
Attributes: attributes,
})
}
gotLinks := testSpan.Links()
e.Expect(gotLinks).ToMatchInAnyOrder(expectedLinks)
})

Expand Down Expand Up @@ -251,8 +244,8 @@ func TestTracer(t *testing.T) {
e.Expect(ok).ToBeTrue()

links := testSpan.Links()
e.Expect(links[link1.SpanContext]).ToEqual(link1.Attributes)
e.Expect(links[link2.SpanContext]).ToEqual(link2.Attributes)
e.Expect(links[0].Attributes).ToEqual(link1.Attributes)
e.Expect(links[1].Attributes).ToEqual(link2.Attributes)
})
})
}
Expand Down
4 changes: 2 additions & 2 deletions propagation/trace_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestExtractValidTraceContextFromHTTPReq(t *testing.T) {
ctx := context.Background()
ctx = prop.Extract(ctx, req.Header)
gotSc := trace.RemoteSpanContextFromContext(ctx)
if diff := cmp.Diff(gotSc, tt.wantSc); diff != "" {
if diff := cmp.Diff(gotSc, tt.wantSc, cmp.AllowUnexported(trace.TraceState{})); diff != "" {
t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff)
}
})
Expand Down Expand Up @@ -201,7 +201,7 @@ func TestExtractInvalidTraceContextFromHTTPReq(t *testing.T) {
ctx := context.Background()
ctx = prop.Extract(ctx, req.Header)
gotSc := trace.RemoteSpanContextFromContext(ctx)
if diff := cmp.Diff(gotSc, wantSc); diff != "" {
if diff := cmp.Diff(gotSc, wantSc, cmp.AllowUnexported(trace.TraceState{})); diff != "" {
t.Errorf("Extract Tracecontext: %s: -got +want %s", tt.name, diff)
}
})
Expand Down
2 changes: 1 addition & 1 deletion sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func startSpanInternal(tr *tracer, name string, parent trace.SpanContext, remote

cfg := tr.provider.config.Load().(*Config)

if parent == emptySpanContext {
if parent.IsEqualWith(emptySpanContext) {
span.spanContext.TraceID = cfg.IDGenerator.NewTraceID()
noParent = true
}
Expand Down
27 changes: 15 additions & 12 deletions sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,34 +305,38 @@ func TestStartSpanWithParent(t *testing.T) {
TraceFlags: 0x1,
}
_, s1 := tr.Start(trace.ContextWithRemoteSpanContext(ctx, sc1), "span1-unsampled-parent1")
if err := checkChild(sc1, s1); err != nil {
if err := checkChild(t, sc1, s1); err != nil {
t.Error(err)
}

_, s2 := tr.Start(trace.ContextWithRemoteSpanContext(ctx, sc1), "span2-unsampled-parent1")
if err := checkChild(sc1, s2); err != nil {
if err := checkChild(t, sc1, s2); err != nil {
t.Error(err)
}

ts, err := trace.TraceStateFromKeyValues(label.String("k", "v"))
if err != nil {
t.Error(err)
}
sc2 := trace.SpanContext{
TraceID: tid,
SpanID: sid,
TraceFlags: 0x1,
//Tracestate: testTracestate,
TraceState: ts,
}
_, s3 := tr.Start(trace.ContextWithRemoteSpanContext(ctx, sc2), "span3-sampled-parent2")
if err := checkChild(sc2, s3); err != nil {
if err := checkChild(t, sc2, s3); err != nil {
t.Error(err)
}

ctx2, s4 := tr.Start(trace.ContextWithRemoteSpanContext(ctx, sc2), "span4-sampled-parent2")
if err := checkChild(sc2, s4); err != nil {
if err := checkChild(t, sc2, s4); err != nil {
t.Error(err)
}

s4Sc := s4.SpanContext()
_, s5 := tr.Start(ctx2, "span5-implicit-childof-span4")
if err := checkChild(s4Sc, s5); err != nil {
if err := checkChild(t, s4Sc, s5); err != nil {
t.Error(err)
}
}
Expand Down Expand Up @@ -740,7 +744,8 @@ func TestSetSpanStatus(t *testing.T) {
func cmpDiff(x, y interface{}) string {
return cmp.Diff(x, y,
cmp.AllowUnexported(label.Value{}),
cmp.AllowUnexported(export.Event{}))
cmp.AllowUnexported(export.Event{}),
cmp.AllowUnexported(trace.TraceState{}))
}

func remoteSpanContext() trace.SpanContext {
Expand All @@ -753,7 +758,7 @@ func remoteSpanContext() trace.SpanContext {

// checkChild is test utility function that tests that c has fields set appropriately,
// given that it is a child span of p.
func checkChild(p trace.SpanContext, apiSpan trace.Span) error {
func checkChild(t *testing.T, p trace.SpanContext, apiSpan trace.Span) error {
s := apiSpan.(*span)
if s == nil {
return fmt.Errorf("got nil child span, want non-nil")
Expand All @@ -767,10 +772,8 @@ func checkChild(p trace.SpanContext, apiSpan trace.Span) error {
if got, want := s.spanContext.TraceFlags, p.TraceFlags; got != want {
return fmt.Errorf("got child trace options %d, want %d", got, want)
}
// TODO [rgheita] : Fix tracestate test
//if got, want := c.spanContext.Tracestate, p.Tracestate; got != want {
// return fmt.Errorf("got child tracestate %v, want %v", got, want)
//}
got, want := s.spanContext.TraceState, p.TraceState
assert.Equal(t, want, got)
return nil
}

Expand Down
Loading