Repository: zeromicro/go-zero
Pull request #2913 (2023-02-22 12:16:09)

fix: fixed the bug that old trace instances may be fetched

codecov[bot]: # [Codecov](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#2913](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (09b23e3) into [master](https://codecov.io/gh/zeromicro/go-zero/commit/133c40ac1cbc357788d7936351ce2d514642855a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (133c40a) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.


| [Impacted Files](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [core/stores/mon/trace.go](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9zdG9yZXMvbW9uL3RyYWNlLmdv) | `100.00% <100.00%> (ø)` | |
| [core/stores/redis/hook.go](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9zdG9yZXMvcmVkaXMvaG9vay5nbw==) | `88.49% <100.00%> (+0.20%)` | :arrow_up: |
| [core/stores/sqlx/trace.go](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9zdG9yZXMvc3FseC90cmFjZS5nbw==) | `78.94% <100.00%> (+1.16%)` | :arrow_up: |
| [core/trace/utils.go](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS90cmFjZS91dGlscy5nbw==) | `100.00% <100.00%> (ø)` | |
| [rest/httpc/requests.go](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cmVzdC9odHRwYy9yZXF1ZXN0cy5nbw==) | `100.00% <100.00%> (ø)` | |
| [core/stores/sqlx/tx.go](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9zdG9yZXMvc3FseC90eC5nbw==) | `19.41% <0.00%> (-3.89%)` | :arrow_down: |
| [core/logx/rotatelogger.go](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9sb2d4L3JvdGF0ZWxvZ2dlci5nbw==) | `72.69% <0.00%> (+1.10%)` | :arrow_up: |
| [core/stores/sqlx/sqlconn.go](https://codecov.io/gh/zeromicro/go-zero/pull/2913?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29yZS9zdG9yZXMvc3FseC9zcWxjb25uLmdv) | `57.84% <0.00%> (+1.47%)` | :arrow_up: |


Patch:
diff --git a/core/stores/mon/trace.go b/core/stores/mon/trace.go
index 0d4cad01..8fd8212d 100644
--- a/core/stores/mon/trace.go
+++ b/core/stores/mon/trace.go
@@ -5,7 +5,6 @@ import (
 
 	"github.com/zeromicro/go-zero/core/trace"
 	"go.mongodb.org/mongo-driver/mongo"
-	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	oteltrace "go.opentelemetry.io/otel/trace"
@@ -14,7 +13,8 @@ import (
 var mongoCmdAttributeKey = attribute.Key("mongo.cmd")
 
 func startSpan(ctx context.Context, cmd string) (context.Context, oteltrace.Span) {
-	tracer := otel.Tracer(trace.TraceName)
+	tracer := trace.TracerFromContext(ctx)
+
 	ctx, span := tracer.Start(ctx,
 		spanName,
 		oteltrace.WithSpanKind(oteltrace.SpanKindClient),
diff --git a/core/stores/redis/hook.go b/core/stores/redis/hook.go
index d7bac1f8..ba0d79ef 100644
--- a/core/stores/redis/hook.go
+++ b/core/stores/redis/hook.go
@@ -14,7 +14,7 @@ import (
 	"github.com/zeromicro/go-zero/core/mapping"
 	"github.com/zeromicro/go-zero/core/timex"
 	"github.com/zeromicro/go-zero/core/trace"
-	"go.opentelemetry.io/otel"
+
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	oteltrace "go.opentelemetry.io/otel/trace"
@@ -25,15 +25,13 @@ const spanName = "redis"
 
 var (
 	startTimeKey          = contextKey("startTime")
-	durationHook          = hook{tracer: otel.Tracer(trace.TraceName)}
+	durationHook          = hook{}
 	redisCmdsAttributeKey = attribute.Key("redis.cmds")
 )
 
 type (
 	contextKey string
-	hook       struct {
-		tracer oteltrace.Tracer
-	}
+	hook       struct{}
 )
 
 func (h hook) BeforeProcess(ctx context.Context, cmd red.Cmder) (context.Context, error) {
@@ -155,7 +153,9 @@ func logDuration(ctx context.Context, cmds []red.Cmder, duration time.Duration)
 }
 
 func (h hook) startSpan(ctx context.Context, cmds ...red.Cmder) context.Context {
-	ctx, span := h.tracer.Start(ctx,
+	tracer := trace.TracerFromContext(ctx)
+
+	ctx, span := tracer.Start(ctx,
 		spanName,
 		oteltrace.WithSpanKind(oteltrace.SpanKindClient),
 	)
diff --git a/core/stores/sqlx/trace.go b/core/stores/sqlx/trace.go
index 2c5c5799..b256a6c6 100644
--- a/core/stores/sqlx/trace.go
+++ b/core/stores/sqlx/trace.go
@@ -5,7 +5,6 @@ import (
 	"database/sql"
 
 	"github.com/zeromicro/go-zero/core/trace"
-	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	oteltrace "go.opentelemetry.io/otel/trace"
@@ -14,7 +13,8 @@ import (
 var sqlAttributeKey = attribute.Key("sql.method")
 
 func startSpan(ctx context.Context, method string) (context.Context, oteltrace.Span) {
-	tracer := otel.Tracer(trace.TraceName)
+	tracer := trace.TracerFromContext(ctx)
+
 	start, span := tracer.Start(ctx,
 		spanName,
 		oteltrace.WithSpanKind(oteltrace.SpanKindClient),
diff --git a/core/trace/utils.go b/core/trace/utils.go
index ce83e863..3fad0cfc 100644
--- a/core/trace/utils.go
+++ b/core/trace/utils.go
@@ -6,8 +6,10 @@ import (
 	"strings"
 
 	ztrace "github.com/zeromicro/go-zero/internal/trace"
+	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
+	"go.opentelemetry.io/otel/trace"
 	"google.golang.org/grpc/peer"
 )
 
@@ -75,3 +77,14 @@ func PeerAttr(addr string) []attribute.KeyValue {
 		semconv.NetPeerPortKey.String(port),
 	}
 }
+
+// TracerFromContext returns a tracer in ctx, otherwise returns a global tracer.
+func TracerFromContext(ctx context.Context) (tracer trace.Tracer) {
+	if span := trace.SpanFromContext(ctx); span.SpanContext().IsValid() {
+		tracer = span.TracerProvider().Tracer(TraceName)
+	} else {
+		tracer = otel.Tracer(TraceName)
+	}
+
+	return
+}
diff --git a/rest/httpc/requests.go b/rest/httpc/requests.go
index 4b3fc60b..defe2599 100644
--- a/rest/httpc/requests.go
+++ b/rest/httpc/requests.go
@@ -156,12 +156,13 @@ func fillPath(u *nurl.URL, val map[string]any) error {
 }
 
 func request(r *http.Request, cli client) (*http.Response, error) {
-	tracer := otel.Tracer(trace.TraceName)
+	ctx := r.Context()
+	tracer := trace.TracerFromContext(ctx)
 	propagator := otel.GetTextMapPropagator()
 
 	spanName := r.URL.Path
 	ctx, span := tracer.Start(
-		r.Context(),
+		ctx,
 		spanName,
 		oteltrace.WithSpanKind(oteltrace.SpanKindClient),
 		oteltrace.WithAttributes(semconv.HTTPClientAttributesFromHTTPRequest(r)...),


Test patch:
diff --git a/core/trace/utils_test.go b/core/trace/utils_test.go
index abb67775..089479ec 100644
--- a/core/trace/utils_test.go
+++ b/core/trace/utils_test.go
@@ -6,8 +6,12 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/sdk/resource"
+	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 	semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
+	"go.opentelemetry.io/otel/trace"
 	"google.golang.org/grpc/peer"
 )
 
@@ -151,3 +155,50 @@ func TestPeerAttr(t *testing.T) {
 		})
 	}
 }
+
+func TestTracerFromContext(t *testing.T) {
+	traceFn := func(ctx context.Context, hasTraceId bool) {
+		spanContext := trace.SpanContextFromContext(ctx)
+		assert.Equal(t, spanContext.IsValid(), hasTraceId)
+		parentTraceId := spanContext.TraceID().String()
+
+		tracer := TracerFromContext(ctx)
+		_, span := tracer.Start(ctx, "b")
+		defer span.End()
+
+		spanContext = span.SpanContext()
+		assert.True(t, spanContext.IsValid())
+		if hasTraceId {
+			assert.Equal(t, parentTraceId, spanContext.TraceID().String())
+		}
+
+	}
+
+	t.Run("context", func(t *testing.T) {
+		opts := []sdktrace.TracerProviderOption{
+			// Set the sampling rate based on the parent span to 100%
+			sdktrace.WithSampler(sdktrace.ParentBased(sdktrace.TraceIDRatioBased(1))),
+			// Record information about this application in a Resource.
+			sdktrace.WithResource(resource.NewSchemaless(semconv.ServiceNameKey.String("test"))),
+		}
+		tp = sdktrace.NewTracerProvider(opts...)
+		otel.SetTracerProvider(tp)
+		ctx, span := tp.Tracer(TraceName).Start(context.Background(), "a")
+
+		defer span.End()
+		traceFn(ctx, true)
+	})
+
+	t.Run("global", func(t *testing.T) {
+		opts := []sdktrace.TracerProviderOption{
+			// Set the sampling rate based on the parent span to 100%
+			sdktrace.WithSampler(sdktrace.ParentBased(sdktrace.TraceIDRatioBased(1))),
+			// Record information about this application in a Resource.
+			sdktrace.WithResource(resource.NewSchemaless(semconv.ServiceNameKey.String("test"))),
+		}
+		tp = sdktrace.NewTracerProvider(opts...)
+		otel.SetTracerProvider(tp)
+
+		traceFn(context.Background(), false)
+	})
+}