diff --git a/cmd/collector/app/metrics.go b/cmd/collector/app/metrics.go index 0015f1c08a5..bf018cbee1d 100644 --- a/cmd/collector/app/metrics.go +++ b/cmd/collector/app/metrics.go @@ -141,7 +141,7 @@ func (m metricsBySvc) ReportServiceNameForSpan(span *model.Span) { if span.Flags.IsDebug() { m.countDebugSpansByServiceName(serviceName) } - if span.ParentSpanID == 0 { + if span.ParentSpanID() == 0 { m.countTracesByServiceName(serviceName) } } diff --git a/cmd/collector/app/metrics_test.go b/cmd/collector/app/metrics_test.go index fca4fe289ff..3156857b8c6 100644 --- a/cmd/collector/app/metrics_test.go +++ b/cmd/collector/app/metrics_test.go @@ -43,7 +43,7 @@ func TestProcessorMetrics(t *testing.T) { } jFormat.ReceivedBySvc.ReportServiceNameForSpan(&mSpan) mSpan.Flags.SetDebug() - mSpan.ParentSpanID = model.SpanID(1234) + mSpan.ReplaceParentID(1234) jFormat.ReceivedBySvc.ReportServiceNameForSpan(&mSpan) counters, gauges := baseMetrics.LocalBackend.Snapshot() diff --git a/model/adjuster/clockskew.go b/model/adjuster/clockskew.go index 85755a36fde..42b7b7280ea 100644 --- a/model/adjuster/clockskew.go +++ b/model/adjuster/clockskew.go @@ -115,14 +115,14 @@ func (a *clockSkewAdjuster) buildSubGraphs() { a.roots = make(map[model.SpanID]*node) for _, n := range a.spans { // TODO handle FOLLOWS_FROM references - if n.span.ParentSpanID == 0 { + if n.span.ParentSpanID() == 0 { a.roots[n.span.SpanID] = n continue } - if p, ok := a.spans[n.span.ParentSpanID]; ok { + if p, ok := a.spans[n.span.ParentSpanID()]; ok { p.children = append(p.children, n) } else { - warning := fmt.Sprintf(warningFormatInvalidParentID, n.span.ParentSpanID) + warning := fmt.Sprintf(warningFormatInvalidParentID, n.span.ParentSpanID()) n.span.Warnings = append(n.span.Warnings, warning) // Treat spans with invalid parent ID as root spans a.roots[n.span.SpanID] = n diff --git a/model/adjuster/clockskew_test.go b/model/adjuster/clockskew_test.go index aafbb4fe7b6..39e0b7db1a7 100644 --- a/model/adjuster/clockskew_test.go +++ b/model/adjuster/clockskew_test.go @@ -53,13 +53,14 @@ func TestClockSkewAdjuster(t *testing.T) { Fields: []model.KeyValue{model.String("event", "some event")}, }) } + traceID := model.TraceID{Low: 1} span := &model.Span{ - TraceID: model.TraceID{Low: 1}, - SpanID: model.SpanID(spanProto.id), - ParentSpanID: model.SpanID(spanProto.parent), - StartTime: toTime(spanProto.startTime), - Duration: toDuration(spanProto.duration), - Logs: logs, + TraceID: traceID, + SpanID: model.SpanID(spanProto.id), + References: []model.SpanRef{model.NewChildOfRef(traceID, model.SpanID(spanProto.parent))}, + StartTime: toTime(spanProto.startTime), + Duration: toDuration(spanProto.duration), + Logs: logs, Process: &model.Process{ ServiceName: spanProto.host, Tags: []model.KeyValue{ diff --git a/model/adjuster/span_id_deduper.go b/model/adjuster/span_id_deduper.go index b72f0b98ab3..aaf8e52cdf9 100644 --- a/model/adjuster/span_id_deduper.go +++ b/model/adjuster/span_id_deduper.go @@ -82,7 +82,7 @@ func (d *spanIDDeduper) dedupeSpanIDs() { continue } oldToNewSpanIDs[span.SpanID] = newID - span.ParentSpanID = span.SpanID // previously shared ID is the new parent + span.ReplaceParentID(span.SpanID) // previously shared ID is the new parent span.SpanID = newID } } @@ -93,9 +93,9 @@ func (d *spanIDDeduper) dedupeSpanIDs() { // spans whose IDs we deduped. func (d *spanIDDeduper) swapParentIDs(oldToNewSpanIDs map[model.SpanID]model.SpanID) { for _, span := range d.trace.Spans { - if parentID, ok := oldToNewSpanIDs[span.ParentSpanID]; ok { + if parentID, ok := oldToNewSpanIDs[span.ParentSpanID()]; ok { if span.SpanID != parentID { - span.ParentSpanID = parentID + span.ReplaceParentID(parentID) } } } diff --git a/model/adjuster/span_id_deduper_test.go b/model/adjuster/span_id_deduper_test.go index d14678b2c83..783294de92d 100644 --- a/model/adjuster/span_id_deduper_test.go +++ b/model/adjuster/span_id_deduper_test.go @@ -29,11 +29,13 @@ const ( ) func newTrace() *model.Trace { + traceID := model.TraceID{Low: 42} return &model.Trace{ Spans: []*model.Span{ { // client span - SpanID: clientSpanID, + TraceID: traceID, + SpanID: clientSpanID, Tags: model.KeyValues{ // span.kind = client model.String(string(ext.SpanKind), string(ext.SpanKindRPCClientEnum)), @@ -41,7 +43,8 @@ func newTrace() *model.Trace { }, { // server span - SpanID: clientSpanID, // shared span ID + TraceID: traceID, + SpanID: clientSpanID, // shared span ID Tags: model.KeyValues{ // span.kind = server model.String(string(ext.SpanKind), string(ext.SpanKindRPCServerEnum)), @@ -49,8 +52,9 @@ func newTrace() *model.Trace { }, { // some other span, child of server span - SpanID: anotherSpanID, - ParentSpanID: clientSpanID, + TraceID: traceID, + SpanID: anotherSpanID, + References: []model.SpanRef{model.NewChildOfRef(traceID, clientSpanID)}, }, }, } @@ -67,11 +71,11 @@ func TestSpanIDDeduperTriggered(t *testing.T) { serverSpan := trace.Spans[1] assert.Equal(t, clientSpanID+1, serverSpan.SpanID, "server span ID should be reassigned") - assert.Equal(t, clientSpan.SpanID, serverSpan.ParentSpanID, "client span should be server span's parent") + assert.Equal(t, clientSpan.SpanID, serverSpan.ParentSpanID(), "client span should be server span's parent") thirdSpan := trace.Spans[2] assert.Equal(t, anotherSpanID, thirdSpan.SpanID, "3rd span ID should not change") - assert.Equal(t, serverSpan.SpanID, thirdSpan.ParentSpanID, "server span should be 3rd span's parent") + assert.Equal(t, serverSpan.SpanID, thirdSpan.ParentSpanID(), "server span should be 3rd span's parent") } func TestSpanIDDeduperNotTriggered(t *testing.T) { @@ -88,7 +92,7 @@ func TestSpanIDDeduperNotTriggered(t *testing.T) { thirdSpan := trace.Spans[1] assert.Equal(t, anotherSpanID, thirdSpan.SpanID, "3rd span ID should not change") - assert.Equal(t, serverSpan.SpanID, thirdSpan.ParentSpanID, "server span should be 3rd span's parent") + assert.Equal(t, serverSpan.SpanID, thirdSpan.ParentSpanID(), "server span should be 3rd span's parent") } func TestSpanIDDeduperError(t *testing.T) { diff --git a/model/converter/json/fixtures/domain_01.json b/model/converter/json/fixtures/domain_01.json index 7c53b83cdab..8ba2e987a58 100644 --- a/model/converter/json/fixtures/domain_01.json +++ b/model/converter/json/fixtures/domain_01.json @@ -70,7 +70,13 @@ { "traceID": "1", "spanID": "3", - "parentSpanID": "2", + "references": [ + { + "refType": "child-of", + "traceID": "1", + "spanID": "2" + } + ], "operationName": "some-operation", "startTime": "2017-01-26T16:46:31.639875139-05:00", "duration": 5000, @@ -111,7 +117,6 @@ { "traceID": "1", "spanID": "5", - "parentSpanID": "4", "operationName": "preserveParentID-test", "references": [ { diff --git a/model/converter/json/fixtures/domain_es_01.json b/model/converter/json/fixtures/domain_es_01.json index f8b46df1f42..0cbd3064ed0 100644 --- a/model/converter/json/fixtures/domain_es_01.json +++ b/model/converter/json/fixtures/domain_es_01.json @@ -1,7 +1,6 @@ { "traceID": "1", "spanID": "2", - "parentSpanID": "3", "operationName": "test-general-conversion", "references": [ { diff --git a/model/converter/json/fixtures/es_01.json b/model/converter/json/fixtures/es_01.json index d98ce574b40..fc2c17ee177 100644 --- a/model/converter/json/fixtures/es_01.json +++ b/model/converter/json/fixtures/es_01.json @@ -1,7 +1,6 @@ { "traceID": "1", "spanID": "2", - "parentSpanID": "3", "flags": 1, "operationName": "test-general-conversion", "references": [ diff --git a/model/converter/json/from_domain.go b/model/converter/json/from_domain.go index b1aa19b22cf..78a4a766719 100644 --- a/model/converter/json/from_domain.go +++ b/model/converter/json/from_domain.go @@ -77,7 +77,7 @@ func (fd fromDomain) convertSpan(span *model.Span, processID json.ProcessID) jso s := fd.convertSpanInternal(span) s.ProcessID = processID s.Warnings = span.Warnings - s.References = fd.convertReferences(span, false) + s.References = fd.convertReferences(span) return s } @@ -85,37 +85,18 @@ func (fd fromDomain) convertSpanEmbedProcess(span *model.Span) *json.Span { s := fd.convertSpanInternal(span) process := fd.convertProcess(span.Process) s.Process = &process - s.ParentSpanID = json.SpanID(span.ParentSpanID.String()) - s.References = fd.convertReferences(span, true) + s.References = fd.convertReferences(span) return &s } -// when preserveParentID==false the parent ID is converted to a CHILD_OF reference -func (fd fromDomain) convertReferences(span *model.Span, preserveParentID bool) []json.Reference { - length := len(span.References) - if span.ParentSpanID != 0 && !preserveParentID { - length++ - } - out := make([]json.Reference, 0, length) - var parentRefAdded bool +func (fd fromDomain) convertReferences(span *model.Span) []json.Reference { + out := make([]json.Reference, 0, len(span.References)) for _, ref := range span.References { out = append(out, json.Reference{ RefType: fd.convertRefType(ref.RefType), TraceID: json.TraceID(ref.TraceID.String()), SpanID: json.SpanID(ref.SpanID.String()), }) - if ref.TraceID == span.TraceID && ref.SpanID == span.ParentSpanID { - parentRefAdded = true - } - } - if span.ParentSpanID != 0 && !preserveParentID && !parentRefAdded { - // By this point, if ParentSpanID != 0 but there are no other references, - // then the ParentSpanID does refer to child-of type - out = append(out, json.Reference{ - RefType: json.ChildOf, - TraceID: json.TraceID(span.TraceID.String()), - SpanID: json.SpanID(span.ParentSpanID.String()), - }) } return out } diff --git a/model/converter/json/from_domain_test.go b/model/converter/json/from_domain_test.go index 6e5cb387391..73a5ace846f 100644 --- a/model/converter/json/from_domain_test.go +++ b/model/converter/json/from_domain_test.go @@ -33,13 +33,13 @@ const NumberOfFixtures = 1 func TestFromDomain(t *testing.T) { for i := 1; i <= NumberOfFixtures; i++ { - inStr, outStr := testReadFixtures(t, i, false) + domainStr, jsonStr := testReadFixtures(t, i, false) var trace model.Trace - require.NoError(t, json.Unmarshal(inStr, &trace)) + require.NoError(t, json.Unmarshal(domainStr, &trace)) uiTrace := FromDomain(&trace) - testOutput(t, i, outStr, uiTrace, false) + testOutput(t, i, jsonStr, uiTrace, false) } // this is just to confirm the uint64 representation of float64(72.5) used as a "temperature" tag assert.Equal(t, int64(4634802150889750528), int64(math.Float64bits(72.5))) @@ -60,6 +60,7 @@ func TestFromDomainEmbedProcess(t *testing.T) { } } +// Loads and returns domain model and JSON model fixtures with given number i. func testReadFixtures(t *testing.T, i int, processEmbedded bool) ([]byte, []byte) { var in string if processEmbedded { @@ -80,7 +81,7 @@ func testReadFixtures(t *testing.T, i int, processEmbedded bool) ([]byte, []byte return inStr, outStr } -func testOutput(t *testing.T, i int, outStr []byte, object interface{}, processEmbedded bool) { +func testOutput(t *testing.T, i int, expectedStr []byte, object interface{}, processEmbedded bool) { buf := &bytes.Buffer{} enc := json.NewEncoder(buf) enc.SetIndent("", " ") @@ -94,7 +95,7 @@ func testOutput(t *testing.T, i int, outStr []byte, object interface{}, processE require.NoError(t, enc.Encode(object.(*jModel.Trace))) } - if !assert.Equal(t, string(outStr), string(buf.Bytes())) { + if !assert.Equal(t, string(expectedStr), string(buf.Bytes())) { err := ioutil.WriteFile(outFile+"-actual.json", buf.Bytes(), 0644) assert.NoError(t, err) } diff --git a/model/converter/json/to_domain.go b/model/converter/json/to_domain.go index 528026c22e2..572c29b1fe6 100644 --- a/model/converter/json/to_domain.go +++ b/model/converter/json/to_domain.go @@ -58,15 +58,18 @@ func (td toDomain) spanToDomain(dbSpan *json.Span) (*model.Span, error) { if err != nil { return nil, err } - parentSpanIDInt, err := model.SpanIDFromString(string(dbSpan.ParentSpanID)) - if err != nil { - return nil, err + + if dbSpan.ParentSpanID != "" { + parentSpanID, err := model.SpanIDFromString(string(dbSpan.ParentSpanID)) + if err != nil { + return nil, err + } + refs = model.MaybeAddParentSpanID(traceID, parentSpanID, refs) } span := &model.Span{ TraceID: traceID, SpanID: model.SpanID(spanIDInt), - ParentSpanID: model.SpanID(parentSpanIDInt), OperationName: dbSpan.OperationName, References: refs, Flags: model.Flags(uint32(dbSpan.Flags)), diff --git a/model/converter/json/to_domain_test.go b/model/converter/json/to_domain_test.go index f82741974f2..68f4808e6dc 100644 --- a/model/converter/json/to_domain_test.go +++ b/model/converter/json/to_domain_test.go @@ -244,10 +244,3 @@ func TestFailureBadSpanID(t *testing.T) { badSpanIDESSpan.SpanID = "zz" failingSpanTransformAnyMsg(t, &badSpanIDESSpan) } - -func TestFailureBadParentSpanID(t *testing.T) { - badParentSpanIDESSpan, err := createGoodSpan(1) - require.NoError(t, err) - badParentSpanIDESSpan.ParentSpanID = "zz" - failingSpanTransformAnyMsg(t, &badParentSpanIDESSpan) -} diff --git a/model/converter/thrift/jaeger/fixtures/model_01.json b/model/converter/thrift/jaeger/fixtures/model_01.json index 0eae334cddd..b7f9f42d09f 100644 --- a/model/converter/thrift/jaeger/fixtures/model_01.json +++ b/model/converter/thrift/jaeger/fixtures/model_01.json @@ -2,8 +2,14 @@ { "traceID":"52969a8955571a3f", "spanID":"647d98", - "parentSpanID":"68c4e3", "operationName":"get", + "references": [ + { + "refType": "child-of", + "traceID": "52969a8955571a3f", + "spanID": "68c4e3" + } + ], "startTime":"2017-01-26T16:46:31.639875-05:00", "duration":22938000, "tags":[ diff --git a/model/converter/thrift/jaeger/fixtures/model_02.json b/model/converter/thrift/jaeger/fixtures/model_02.json index 26f6a1c9b3f..6e47f1d32e8 100644 --- a/model/converter/thrift/jaeger/fixtures/model_02.json +++ b/model/converter/thrift/jaeger/fixtures/model_02.json @@ -2,8 +2,14 @@ { "traceID":"52969a8955571a3f", "spanID":"647d98", - "parentSpanID":"68c4e3", "operationName":"get", + "references": [ + { + "refType": "child-of", + "traceID": "52969a8955571a3f", + "spanID": "68c4e3" + } + ], "startTime":"2017-01-26T16:46:31.639875-05:00", "duration":22938000, "tags":[ diff --git a/model/converter/thrift/jaeger/fixtures/model_03.json b/model/converter/thrift/jaeger/fixtures/model_03.json index 848982df64a..090df36af94 100644 --- a/model/converter/thrift/jaeger/fixtures/model_03.json +++ b/model/converter/thrift/jaeger/fixtures/model_03.json @@ -2,7 +2,6 @@ { "traceID":"52969a8955571a3f", "spanID":"52969a", - "parentSpanID":"52969a", "operationName":"get", "startTime":"2017-01-26T16:46:31.639875-05:00", "duration":22938000, @@ -93,7 +92,6 @@ { "traceID":"52969a8955571a3f", "spanID":"52947b", - "parentSpanID":"52969a", "operationName":"get", "startTime":"2017-01-26T16:46:31.639875-05:00", "duration":22938000, diff --git a/model/converter/thrift/jaeger/from_domain.go b/model/converter/thrift/jaeger/from_domain.go index d3d09301500..87b5184e159 100644 --- a/model/converter/thrift/jaeger/from_domain.go +++ b/model/converter/thrift/jaeger/from_domain.go @@ -140,7 +140,7 @@ func (d domainToJaegerTransformer) transformSpan(span *model.Span) *jaeger.Span TraceIdLow: int64(span.TraceID.Low), TraceIdHigh: int64(span.TraceID.High), SpanId: int64(span.SpanID), - ParentSpanId: int64(span.ParentSpanID), + ParentSpanId: int64(span.ParentSpanID()), OperationName: span.OperationName, References: refs, Flags: int32(span.Flags), diff --git a/model/converter/thrift/jaeger/to_domain.go b/model/converter/thrift/jaeger/to_domain.go index 8c11b6a923b..82dca4702e9 100644 --- a/model/converter/thrift/jaeger/to_domain.go +++ b/model/converter/thrift/jaeger/to_domain.go @@ -53,17 +53,25 @@ func (td toDomain) ToDomainSpan(jSpan *jaeger.Span, jProcess *jaeger.Process) *m } func (td toDomain) transformSpan(jSpan *jaeger.Span, mProcess *model.Process) *model.Span { + traceID := model.TraceID{ + High: uint64(jSpan.TraceIdHigh), + Low: uint64(jSpan.TraceIdLow), + } + spanID := model.SpanID(jSpan.SpanId) tags := td.getTags(jSpan.Tags) refs := td.getReferences(jSpan.References) + // We no longer store ParentSpanID in the domain model, but the data in Thrift model + // might still have these IDs without representing them in the References, so we + // convert it back into child-of reference. + if jSpan.ParentSpanId != 0 { + parentSpanID := model.SpanID(jSpan.ParentSpanId) + refs = model.MaybeAddParentSpanID(traceID, parentSpanID, refs) + } return &model.Span{ - TraceID: model.TraceID{ - High: uint64(jSpan.TraceIdHigh), - Low: uint64(jSpan.TraceIdLow), - }, - SpanID: model.SpanID(jSpan.SpanId), + TraceID: traceID, + SpanID: spanID, OperationName: jSpan.OperationName, References: refs, - ParentSpanID: model.SpanID(jSpan.GetParentSpanId()), Flags: model.Flags(jSpan.Flags), StartTime: model.EpochMicrosecondsAsTime(uint64(jSpan.StartTime)), Duration: model.MicrosecondsAsDuration(uint64(jSpan.Duration)), diff --git a/model/converter/thrift/zipkin/fixtures/jaeger_01.json b/model/converter/thrift/zipkin/fixtures/jaeger_01.json index fef351bea3f..a882259ba36 100644 --- a/model/converter/thrift/zipkin/fixtures/jaeger_01.json +++ b/model/converter/thrift/zipkin/fixtures/jaeger_01.json @@ -63,8 +63,14 @@ { "traceID": "20000000000000001", "spanID": "3", - "parentSpanID": "2", "operationName": "some-operation", + "references": [ + { + "refType": "child-of", + "traceID": "20000000000000001", + "spanID": "2" + } + ], "startTime": "1970-01-01T00:00:00-00:00", "tags": [ { diff --git a/model/converter/thrift/zipkin/to_domain.go b/model/converter/thrift/zipkin/to_domain.go index 320f9adcbb5..94bf840912c 100644 --- a/model/converter/thrift/zipkin/to_domain.go +++ b/model/converter/thrift/zipkin/to_domain.go @@ -124,20 +124,23 @@ func (td toDomain) transformSpan(zSpan *zipkincore.Span) []*model.Span { if spanKindTag, ok := td.getSpanKindTag(zSpan.Annotations); ok { tags = append(tags, spanKindTag) } - var traceIDHigh, parentID int64 + var traceIDHigh int64 if zSpan.TraceIDHigh != nil { traceIDHigh = *zSpan.TraceIDHigh } + traceID := model.TraceID{High: uint64(traceIDHigh), Low: uint64(zSpan.TraceID)} + var refs []model.SpanRef if zSpan.ParentID != nil { - parentID = *zSpan.ParentID + parentSpanID := model.SpanID(*zSpan.ParentID) + refs = model.MaybeAddParentSpanID(traceID, parentSpanID, refs) } flags := td.getFlags(zSpan) result := []*model.Span{{ - TraceID: model.TraceID{High: uint64(traceIDHigh), Low: uint64(zSpan.TraceID)}, + TraceID: traceID, SpanID: model.SpanID(zSpan.ID), OperationName: zSpan.Name, - ParentSpanID: model.SpanID(parentID), + References: refs, Flags: flags, StartTime: model.EpochMicrosecondsAsTime(uint64(zSpan.GetTimestamp())), Duration: model.MicrosecondsAsDuration(uint64(zSpan.GetDuration())), @@ -150,10 +153,10 @@ func (td toDomain) transformSpan(zSpan *zipkincore.Span) []*model.Span { if cs != nil && sr != nil { // if the span is client and server we split it into two separate spans s := &model.Span{ - TraceID: model.TraceID{High: uint64(traceIDHigh), Low: uint64(zSpan.TraceID)}, + TraceID: traceID, SpanID: model.SpanID(zSpan.ID), OperationName: zSpan.Name, - ParentSpanID: model.SpanID(parentID), + References: refs, Flags: flags, } // if the first span is a client span we create server span and vice-versa. diff --git a/model/converter/thrift/zipkin/to_domain_test.go b/model/converter/thrift/zipkin/to_domain_test.go index aa135fcd652..0e18be80493 100644 --- a/model/converter/thrift/zipkin/to_domain_test.go +++ b/model/converter/thrift/zipkin/to_domain_test.go @@ -114,7 +114,6 @@ func TestToDomainMultipleSpanKinds(t *testing.T) { } for _, test := range tests { - fmt.Println(test.json) trace, err := ToDomain(getZipkinSpans(t, test.json)) require.Nil(t, err) diff --git a/model/json/model.go b/model/json/model.go index c739853c551..e38b3001a3a 100644 --- a/model/json/model.go +++ b/model/json/model.go @@ -68,7 +68,7 @@ type Trace struct { type Span struct { TraceID TraceID `json:"traceID"` SpanID SpanID `json:"spanID"` - ParentSpanID SpanID `json:"parentSpanID,omitempty"` + ParentSpanID SpanID `json:"parentSpanID,omitempty"` // deprecated Flags uint32 `json:"flags,omitempty"` OperationName string `json:"operationName"` References []Reference `json:"references"` diff --git a/model/span.go b/model/span.go index 48439a7f4ee..0b09ce2955d 100644 --- a/model/span.go +++ b/model/span.go @@ -47,7 +47,6 @@ type SpanID uint64 type Span struct { TraceID TraceID `json:"traceID"` SpanID SpanID `json:"spanID"` - ParentSpanID SpanID `json:"parentSpanID"` OperationName string `json:"operationName"` References []SpanRef `json:"references,omitempty"` Flags Flags `json:"flags,omitempty"` @@ -67,6 +66,17 @@ func (s *Span) Hash(w io.Writer) (err error) { return enc.Encode(s) } +// ParentSpanID returns the ID of a parent span if it exists. +// It searches the references for a reference pointing to the same trace ID. +func (s *Span) ParentSpanID() SpanID { + for i := range s.References { + if s.References[i].TraceID == s.TraceID { + return s.References[i].SpanID + } + } + return SpanID(0) +} + // HasSpanKind returns true if the span has a `span.kind` tag set to `kind`. func (s *Span) HasSpanKind(kind ext.SpanKindEnum) bool { if tag, ok := KeyValues(s.Tags).FindByKey(string(ext.SpanKind)); ok { @@ -95,6 +105,18 @@ func (s *Span) NormalizeTimestamps() { } } +// ReplaceParentID replaces span ID in the span reference that is considered a parent span. +func (s *Span) ReplaceParentID(newParentID SpanID) { + oldParentID := s.ParentSpanID() + for i := range s.References { + if s.References[i].SpanID == oldParentID && s.References[i].TraceID == s.TraceID { + s.References[i].SpanID = newParentID + return + } + } + s.References = MaybeAddParentSpanID(s.TraceID, newParentID, s.References) +} + // ------- Flags ------- // SetSampled sets the Flags as sampled diff --git a/model/spanref.go b/model/spanref.go index 267e17111d0..92726cc1f89 100644 --- a/model/spanref.go +++ b/model/spanref.go @@ -14,7 +14,9 @@ package model -import "fmt" +import ( + "fmt" +) // SpanRefType describes the type of a span reference type SpanRefType int @@ -74,3 +76,39 @@ func (p *SpanRefType) UnmarshalText(text []byte) error { *p = q return nil } + +// MaybeAddParentSpanID adds non-zero parentSpanID to refs as a child-of reference. +// We no longer store ParentSpanID in the domain model, but the data in the database +// or other formats might still have these IDs without representing them in the References, +// so this converts parent IDs to canonical reference format. +func MaybeAddParentSpanID(traceID TraceID, parentSpanID SpanID, refs []SpanRef) []SpanRef { + if parentSpanID == 0 { + return refs + } + for _, r := range refs { + if r.SpanID == parentSpanID && r.TraceID == traceID { + return refs + } + } + newRef := SpanRef{ + TraceID: traceID, + SpanID: parentSpanID, + RefType: ChildOf, + } + if len(refs) == 0 { + return []SpanRef{newRef} + } + newRefs := make([]SpanRef, len(refs)+1) + newRefs[0] = newRef + copy(newRefs[1:], refs) + return newRefs +} + +// NewChildOfRef creates a new child-of span reference. +func NewChildOfRef(traceID TraceID, spanID SpanID) SpanRef { + return SpanRef{ + RefType: ChildOf, + TraceID: traceID, + SpanID: spanID, + } +} diff --git a/plugin/storage/cassandra/savetracetest/main.go b/plugin/storage/cassandra/savetracetest/main.go index d01de4036a3..b21c7a07912 100644 --- a/plugin/storage/cassandra/savetracetest/main.go +++ b/plugin/storage/cassandra/savetracetest/main.go @@ -102,12 +102,12 @@ func getSomeProcess() *model.Process { } func getSomeSpan() *model.Span { + traceID := model.TraceID{High: 1, Low: 2} return &model.Span{ - TraceID: model.TraceID{High: 1, Low: 2}, + TraceID: traceID, SpanID: model.SpanID(3), - ParentSpanID: model.SpanID(4), OperationName: "opName", - References: getReferences(), + References: append([]model.SpanRef{model.NewChildOfRef(traceID, 4)}, getReferences()...), Flags: model.Flags(uint32(5)), StartTime: time.Now(), Duration: 50000 * time.Microsecond, diff --git a/plugin/storage/cassandra/spanstore/dbmodel/converter.go b/plugin/storage/cassandra/spanstore/dbmodel/converter.go index 50683f7d5d0..434c51ab1fc 100644 --- a/plugin/storage/cassandra/spanstore/dbmodel/converter.go +++ b/plugin/storage/cassandra/spanstore/dbmodel/converter.go @@ -47,7 +47,6 @@ func (c converter) fromDomain(span *model.Span) *Span { return &Span{ TraceID: TraceIDFromDomain(span.TraceID), SpanID: int64(span.SpanID), - ParentID: int64(span.ParentSpanID), OperationName: span.OperationName, Flags: int32(span.Flags), StartTime: int64(model.TimeAsEpochMicroseconds(span.StartTime)), @@ -78,10 +77,13 @@ func (c converter) toDomain(dbSpan *Span) (*model.Span, error) { if err != nil { return nil, err } + traceID := dbSpan.TraceID.ToDomain() + if dbSpan.ParentID != 0 { + refs = model.MaybeAddParentSpanID(traceID, model.SpanID(dbSpan.ParentID), refs) + } span := &model.Span{ - TraceID: dbSpan.TraceID.ToDomain(), + TraceID: traceID, SpanID: model.SpanID(dbSpan.SpanID), - ParentSpanID: model.SpanID(dbSpan.ParentID), OperationName: dbSpan.OperationName, References: refs, Flags: model.Flags(uint32(dbSpan.Flags)), diff --git a/plugin/storage/cassandra/spanstore/dbmodel/converter_test.go b/plugin/storage/cassandra/spanstore/dbmodel/converter_test.go index 41e1880df90..21bb742f039 100644 --- a/plugin/storage/cassandra/spanstore/dbmodel/converter_test.go +++ b/plugin/storage/cassandra/spanstore/dbmodel/converter_test.go @@ -122,7 +122,6 @@ func getTestJaegerSpan() *model.Span { return &model.Span{ TraceID: someTraceID, SpanID: someSpanID, - ParentSpanID: someParentSpanID, OperationName: someOperationName, References: someRefs, Flags: someFlags, @@ -145,7 +144,6 @@ func getTestSpan() *Span { span := &Span{ TraceID: someDBTraceID, SpanID: int64(someSpanID), - ParentID: int64(someParentSpanID), OperationName: someOperationName, Flags: int32(someFlags), StartTime: int64(model.TimeAsEpochMicroseconds(someStartTime)), @@ -194,8 +192,11 @@ func TestToSpan(t *testing.T) { } func TestFromSpan(t *testing.T) { + testDBSpan := getTestSpan() + testDBSpan.ParentID = testDBSpan.Refs[0].SpanID + testDBSpan.Refs = nil expectedSpan := getTestJaegerSpan() - actualJSpan, err := ToDomain(getTestSpan()) + actualJSpan, err := ToDomain(testDBSpan) assert.NoError(t, err) if !assert.EqualValues(t, expectedSpan, actualJSpan) { for _, diff := range pretty.Diff(expectedSpan, actualJSpan) { diff --git a/plugin/storage/memory/memory.go b/plugin/storage/memory/memory.go index a25f816becb..24bb1b72688 100644 --- a/plugin/storage/memory/memory.go +++ b/plugin/storage/memory/memory.go @@ -58,7 +58,7 @@ func (m *Store) GetDependencies(endTs time.Time, lookback time.Duration) ([]mode trace, _ := m.deduper.Adjust(orig) if m.traceIsBetweenStartAndEnd(startTs, endTs, trace) { for _, s := range trace.Spans { - parentSpan := m.findSpan(trace, s.ParentSpanID) + parentSpan := m.findSpan(trace, s.ParentSpanID()) if parentSpan != nil { if parentSpan.Process.ServiceName == s.Process.ServiceName { continue diff --git a/plugin/storage/memory/memory_test.go b/plugin/storage/memory/memory_test.go index 428b0118438..e9c2047d31e 100644 --- a/plugin/storage/memory/memory_test.go +++ b/plugin/storage/memory/memory_test.go @@ -24,12 +24,14 @@ import ( "github.com/jaegertracing/jaeger/storage/spanstore" ) +var traceID = model.TraceID{ + Low: 1, + High: 2, +} + var testingSpan = &model.Span{ - TraceID: model.TraceID{ - Low: 1, - High: 2, - }, - SpanID: model.SpanID(1), + TraceID: traceID, + SpanID: model.SpanID(1), Process: &model.Process{ ServiceName: "serviceName", Tags: model.KeyValues{}, @@ -51,12 +53,9 @@ var testingSpan = &model.Span{ } var childSpan1 = &model.Span{ - TraceID: model.TraceID{ - Low: 1, - High: 2, - }, - SpanID: model.SpanID(2), - ParentSpanID: model.SpanID(1), + TraceID: traceID, + SpanID: model.SpanID(2), + References: []model.SpanRef{model.NewChildOfRef(traceID, model.SpanID(1))}, Process: &model.Process{ ServiceName: "childService", Tags: model.KeyValues{}, @@ -78,12 +77,9 @@ var childSpan1 = &model.Span{ } var childSpan2 = &model.Span{ - TraceID: model.TraceID{ - Low: 1, - High: 2, - }, - SpanID: model.SpanID(3), - ParentSpanID: model.SpanID(1), + TraceID: traceID, + SpanID: model.SpanID(3), + References: []model.SpanRef{model.NewChildOfRef(traceID, model.SpanID(1))}, Process: &model.Process{ ServiceName: "childService", Tags: model.KeyValues{}, @@ -105,12 +101,10 @@ var childSpan2 = &model.Span{ } var childSpan2_1 = &model.Span{ - TraceID: model.TraceID{ - Low: 1, - High: 2, - }, - SpanID: model.SpanID(4), - ParentSpanID: model.SpanID(3), // child of childSpan2, but with the same service name + TraceID: traceID, + SpanID: model.SpanID(4), + // child of childSpan2, but with the same service name + References: []model.SpanRef{model.NewChildOfRef(traceID, model.SpanID(3))}, Process: &model.Process{ ServiceName: "childService", Tags: model.KeyValues{},