return early when non-traced (#17)

* return early when non-traced Signed-off-by: zhongzc <[email protected]> * clean up logic Signed-off-by: zhongzc <[email protected]>
tikv · Nov 20, 2020 · 5931332 · 5931332
1 parent 9b8565f
commit 5931332
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 34 deletions.
diff --git a/jaeger/jaeger.go b/jaeger/jaeger.go
@@ -17,20 +17,6 @@ import (
 	"github.com/tikv/minitrace-go"
 )
 
-type ReferenceType int32
-
-const (
-	ChildOf    ReferenceType = 0
-	FollowFrom ReferenceType = 1
-)
-
-type SpanInfo struct {
-	SelfId        int64
-	ParentId      int64
-	Ref           ReferenceType
-	OperationName string
-}
-
 func ThriftCompactEncode(
 	buf *[]uint8,
 	serviceName string,
@@ -69,7 +55,7 @@ func ThriftCompactEncode(
 		encodeBytes(buf, []uint8(span.Event))
 
 		*buf = append(*buf, []uint8{0x19, 0x1c, 0x15}...)
-		encodeVarInt(buf, uint64(zigzagFromI32(int32(FollowFrom))))
+		encodeVarInt(buf, uint64(zigzagFromI32(int32(1 /* Follow from */))))
 		*buf = append(*buf, 0x16)
 		encodeVarInt(buf, zigzagFromI64(traceIdLow))
 		*buf = append(*buf, 0x16)

diff --git a/minitrace_test.go b/minitrace_test.go
@@ -75,15 +75,37 @@ func BenchmarkAppdashTrace(b *testing.B) {
 }
 
 func TestMiniTrace(t *testing.T) {
-	ctx, handle := StartRootSpan(context.Background(), "root", 9527, nil)
+	var traceId uint64 = 9527
+	ctx, handle := StartRootSpan(context.Background(), "root", traceId, nil)
 	var wg sync.WaitGroup
 
-	if _, traceId, ok := CurrentSpanId(ctx); !ok || traceId != 9527 {
-		t.Fatalf("Trace id should be %d", traceId)
+	if spanId1, traceId1, ok := CurrentSpanId(ctx); ok {
+		spanId := handle.spanContext.currentSpanId
+		if spanId != spanId1 {
+			t.Fatalf("unmatched span ID: expected %d got %d", spanId, spanId1)
+		}
+		if traceId != traceId1 {
+			t.Fatalf("unmatched trace ID: expected %d got %d", traceId, traceId1)
+		}
+	} else {
+		t.Fatalf("cannot get current span ID")
 	}
 
 	for i := 1; i < 5; i++ {
 		ctx, handle := StartSpanWithContext(ctx, strconv.Itoa(i))
+
+		if spanId1, traceId1, ok := CurrentSpanId(ctx); ok {
+			spanId := handle.spanContext.currentSpanId
+			if spanId != spanId1 {
+				t.Fatalf("unmatched span ID: expected %d got %d", spanId, spanId1)
+			}
+			if traceId != traceId1 {
+				t.Fatalf("unmatched trace ID: expected %d got %d", traceId, traceId1)
+			}
+		} else {
+			t.Fatalf("cannot get current span ID")
+		}
+
 		wg.Add(1)
 		go func(prefix int) {
 			ctx, handle := StartSpanWithContext(ctx, strconv.Itoa(prefix))

diff --git a/trace.go b/trace.go
@@ -74,7 +74,7 @@ func StartRootSpan(ctx context.Context, event string, traceId uint64, attachment
 
 func StartSpanWithContext(ctx context.Context, event string) (context.Context, SpanHandle) {
 	handle := StartSpan(ctx, event)
-	if handle.durationNs != nil {
+	if !handle.finished {
 		return handle.spanContext, handle
 	}
 	return ctx, handle
@@ -93,6 +93,7 @@ func StartSpan(ctx context.Context, event string) (res SpanHandle) {
 		res.spanContext.createMonoTimeNs = s.createMonoTimeNs
 		res.spanContext.createUnixTimeNs = s.createUnixTimeNs
 	} else {
+		res.finished = true
 		return
 	}
 
@@ -136,7 +137,9 @@ func StartSpan(ctx context.Context, event string) (res SpanHandle) {
 }
 
 func CurrentSpanId(ctx context.Context) (spanId uint32, traceId uint64, ok bool) {
-	if s, ok := ctx.Value(activeTracingKey).(spanContext); ok {
+	if s, ok := ctx.(spanContext); ok {
+		return s.currentSpanId, s.tracingContext.traceId, ok
+	} else if s, ok := ctx.Value(activeTracingKey).(spanContext); ok {
 		return s.currentSpanId, s.tracingContext.traceId, ok
 	}
 
@@ -152,10 +155,18 @@ type SpanHandle struct {
 }
 
 func (hd *SpanHandle) AddProperty(key, value string) {
+	if hd.finished {
+		return
+	}
+
 	*hd.properties = append(*hd.properties, Property{Key: key, Value: value})
 }
 
 func (hd *SpanHandle) AccessAttachment(fn func(attachment interface{})) {
+	if hd.finished {
+		return
+	}
+
 	hd.spanContext.tracingContext.mu.Lock()
 	if !hd.spanContext.tracingContext.collected {
 		fn(hd.spanContext.tracingContext.attachment)
@@ -166,23 +177,21 @@ func (hd *SpanHandle) AccessAttachment(fn func(attachment interface{})) {
 func (hd *SpanHandle) Finish() {
 	if hd.finished {
 		return
-	} else {
-		hd.finished = true
 	}
 
-	if hd.durationNs != nil {
-		// For now, `beginUnixTimeNs` is a monotonic time. Here to correct its value to satisfy the semantic.
-		*hd.durationNs = monotimeNs() - *hd.beginUnixTimeNs
-		*hd.beginUnixTimeNs = (*hd.beginUnixTimeNs - hd.spanContext.createMonoTimeNs) + hd.spanContext.createUnixTimeNs
-
-		hd.spanContext.tracedSpans.refCount -= 1
-		if hd.spanContext.tracedSpans.refCount == 0 {
-			hd.spanContext.tracingContext.mu.Lock()
-			if !hd.spanContext.tracingContext.collected {
-				hd.spanContext.tracingContext.collectedSpans = append(hd.spanContext.tracingContext.collectedSpans, hd.spanContext.tracedSpans.spans.collect()...)
-			}
-			hd.spanContext.tracingContext.mu.Unlock()
+	hd.finished = true
+
+	// For now, `beginUnixTimeNs` is a monotonic time. Here to correct its value to satisfy the semantic.
+	*hd.durationNs = monotimeNs() - *hd.beginUnixTimeNs
+	*hd.beginUnixTimeNs = (*hd.beginUnixTimeNs - hd.spanContext.createMonoTimeNs) + hd.spanContext.createUnixTimeNs
+
+	hd.spanContext.tracedSpans.refCount -= 1
+	if hd.spanContext.tracedSpans.refCount == 0 {
+		hd.spanContext.tracingContext.mu.Lock()
+		if !hd.spanContext.tracingContext.collected {
+			hd.spanContext.tracingContext.collectedSpans = append(hd.spanContext.tracingContext.collectedSpans, hd.spanContext.tracedSpans.spans.collect()...)
 		}
+		hd.spanContext.tracingContext.mu.Unlock()
 	}
 }