Use Zipkin annotations if the timestamp is zero (#1341)

* Use Zipkin annotations if the timestamp is zero

Signed-off-by: Geoffrey Beausire <g.beausire@criteo.com>

* Check if duration is zero before overriding

Signed-off-by: Geoffrey Beausire <g.beausire@criteo.com>

Author: geobeau

model/converter/thrift/zipkin/to_domain.go

@@ -136,15 +136,17 @@ func (td toDomain) transformSpan(zSpan *zipkincore.Span) []*model.Span {
 	}
 
 	flags := td.getFlags(zSpan)
-	// TODO StartTime and Duration could theoretically be defined only via cs/cr/sr/ss annotations.
+
+	startTime, duration := td.getStartTimeAndDuration(zSpan)
+
 	result := []*model.Span{{
 		TraceID:       traceID,
 		SpanID:        model.NewSpanID(uint64(zSpan.ID)),
 		OperationName: zSpan.Name,
 		References:    refs,
 		Flags:         flags,
-		StartTime:     model.EpochMicrosecondsAsTime(uint64(zSpan.GetTimestamp())),
-		Duration:      model.MicrosecondsAsDuration(uint64(zSpan.GetDuration())),
+		StartTime:     model.EpochMicrosecondsAsTime(uint64(startTime)),
+		Duration:      model.MicrosecondsAsDuration(uint64(duration)),
 		Tags:          tags,
 		Logs:          td.getLogs(zSpan.Annotations),
 	}}
@@ -188,6 +190,30 @@ func (td toDomain) getFlags(zSpan *zipkincore.Span) model.Flags {
 	return f
 }
 
+// Get a correct start time to use for the span if it's not set directly
+func (td toDomain) getStartTimeAndDuration(zSpan *zipkincore.Span) (int64, int64) {
+	timestamp := zSpan.GetTimestamp()
+	duration := zSpan.GetDuration()
+	if timestamp == 0 {
+		cs := td.findAnnotation(zSpan, zipkincore.CLIENT_SEND)
+		sr := td.findAnnotation(zSpan, zipkincore.SERVER_RECV)
+		if cs != nil {
+			timestamp = cs.Timestamp
+			cr := td.findAnnotation(zSpan, zipkincore.CLIENT_RECV)
+			if cr != nil && duration == 0 {
+				duration = cr.Timestamp - cs.Timestamp
+			}
+		} else if sr != nil {
+			timestamp = sr.Timestamp
+			ss := td.findAnnotation(zSpan, zipkincore.SERVER_SEND)
+			if ss != nil && duration == 0 {
+				duration = ss.Timestamp - sr.Timestamp
+			}
+		}
+	}
+	return timestamp, duration
+}
+
 // generateProcess takes a Zipkin Span and produces a model.Process.
 // An optional error may also be returned, but it is not fatal.
 func (td toDomain) generateProcess(zSpan *zipkincore.Span) (*model.Process, error) {

model/converter/thrift/zipkin/to_domain_test.go

@@ -91,6 +91,28 @@ func TestToDomainServiceNameInBinAnnotation(t *testing.T) {
 	assert.Equal(t, "bar", trace.Spans[0].Process.ServiceName)
 }
 
+func TestToDomainWithDurationFromServerAnnotations(t *testing.T) {
+	zSpans := getZipkinSpans(t, `[{ "trace_id": -1, "id": 31, "annotations": [
+	{"value": "sr", "timestamp": 1, "host": {"service_name": "bar", "ipv4": 23456}},
+	{"value": "ss", "timestamp": 10, "host": {"service_name": "bar", "ipv4": 23456}}
+	]}]`)
+	trace, err := ToDomain(zSpans)
+	require.Nil(t, err)
+	assert.Equal(t, 1000, int(trace.Spans[0].StartTime.Nanosecond()))
+	assert.Equal(t, 9000, int(trace.Spans[0].Duration))
+}
+
+func TestToDomainWithDurationFromClientAnnotations(t *testing.T) {
+	zSpans := getZipkinSpans(t, `[{ "trace_id": -1, "id": 31, "annotations": [
+	{"value": "cs", "timestamp": 1, "host": {"service_name": "bar", "ipv4": 23456}},
+	{"value": "cr", "timestamp": 10, "host": {"service_name": "bar", "ipv4": 23456}}
+	]}]`)
+	trace, err := ToDomain(zSpans)
+	require.Nil(t, err)
+	assert.Equal(t, 1000, int(trace.Spans[0].StartTime.Nanosecond()))
+	assert.Equal(t, 9000, int(trace.Spans[0].Duration))
+}
+
 func TestToDomainMultipleSpanKinds(t *testing.T) {
 	tests := []struct {
 		json      string