Repository: zeromicro/go-zero
Pull request #2773 (2023-01-09 15:22:16)

refactor(rest): use static config for trace ignore paths.
BREAKING CHANGE: remove `handler.DontTraceSpan` function, please use `rest.RestConf.TraceIgnorePaths` instead.


Patch:
diff --git a/core/mapping/unmarshaler.go b/core/mapping/unmarshaler.go
index e6c7425e..1ec535d1 100644
--- a/core/mapping/unmarshaler.go
+++ b/core/mapping/unmarshaler.go
@@ -731,24 +731,41 @@ func (u *Unmarshaler) processNamedFieldWithValue(fieldType reflect.Type, value r
 		return u.processFieldNotFromString(fieldType, value, vp, opts, fullName)
 	default:
 		if u.opts.fromString || opts.fromString() {
-			valueKind := reflect.TypeOf(mapValue).Kind()
-			if valueKind != reflect.String {
-				return fmt.Errorf("error: the value in map is not string, but %s", valueKind)
-			}
+			return u.processNamedFieldWithValueFromString(fieldType, value, mapValue,
+				key, opts, fullName)
+		}
 
-			options := opts.options()
-			if len(options) > 0 {
-				if !stringx.Contains(options, mapValue.(string)) {
-					return fmt.Errorf(`error: value "%s" for field "%s" is not defined in options "%v"`,
-						mapValue, key, options)
-				}
-			}
+		return u.processFieldNotFromString(fieldType, value, vp, opts, fullName)
+	}
+}
 
-			return fillPrimitive(fieldType, value, mapValue, opts, fullName)
+func (u *Unmarshaler) processNamedFieldWithValueFromString(fieldType reflect.Type, value reflect.Value,
+	mapValue interface{}, key string, opts *fieldOptionsWithContext, fullName string) error {
+	valueKind := reflect.TypeOf(mapValue).Kind()
+	if valueKind != reflect.String {
+		return fmt.Errorf("the value in map is not string, but %s", valueKind)
+	}
+
+	options := opts.options()
+	if len(options) > 0 {
+		var checkValue string
+		switch mt := mapValue.(type) {
+		case string:
+			checkValue = mt
+		case fmt.Stringer:
+			checkValue = mt.String()
+		default:
+			return fmt.Errorf("the value in map is not string or json.Number, but %s",
+				valueKind.String())
 		}
 
-		return u.processFieldNotFromString(fieldType, value, vp, opts, fullName)
+		if !stringx.Contains(options, checkValue) {
+			return fmt.Errorf(`value "%s" for field "%s" is not defined in options "%v"`,
+				mapValue, key, options)
+		}
 	}
+
+	return fillPrimitive(fieldType, value, mapValue, opts, fullName)
 }
 
 func (u *Unmarshaler) processNamedFieldWithoutValue(fieldType reflect.Type, value reflect.Value,
diff --git a/rest/config.go b/rest/config.go
index 48738001..ece29bfd 100644
--- a/rest/config.go
+++ b/rest/config.go
@@ -57,5 +57,7 @@ type (
 		Signature    SignatureConf `json:",optional"`
 		// There are default values for all the items in Middlewares.
 		Middlewares MiddlewaresConf
+		// TraceIgnorePaths is paths blacklist for trace middleware.
+		TraceIgnorePaths []string `json:",optional"`
 	}
 )
diff --git a/rest/engine.go b/rest/engine.go
index 8beb5ad0..6f12ee23 100644
--- a/rest/engine.go
+++ b/rest/engine.go
@@ -118,7 +118,9 @@ func (ng *engine) buildChainWithNativeMiddlewares(fr featuredRoutes, route Route
 	chn := chain.New()
 
 	if ng.conf.Middlewares.Trace {
-		chn = chn.Append(handler.TracingHandler(ng.conf.Name, route.Path))
+		chn = chn.Append(handler.TracingHandler(ng.conf.Name,
+			route.Path,
+			handler.WithTraceIgnorePaths(ng.conf.TraceIgnorePaths)))
 	}
 	if ng.conf.Middlewares.Log {
 		chn = chn.Append(ng.getLogHandler())
@@ -202,7 +204,9 @@ func (ng *engine) getShedder(priority bool) load.Shedder {
 func (ng *engine) notFoundHandler(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		chn := chain.New(
-			handler.TracingHandler(ng.conf.Name, ""),
+			handler.TracingHandler(ng.conf.Name,
+				"",
+				handler.WithTraceIgnorePaths(ng.conf.TraceIgnorePaths)),
 			ng.getLogHandler(),
 		)
 
diff --git a/rest/handler/tracinghandler.go b/rest/handler/tracinghandler.go
index d3151569..a664cd78 100644
--- a/rest/handler/tracinghandler.go
+++ b/rest/handler/tracinghandler.go
@@ -2,9 +2,8 @@ package handler
 
 import (
 	"net/http"
-	"sync"
 
-	"github.com/zeromicro/go-zero/core/lang"
+	"github.com/zeromicro/go-zero/core/collection"
 	"github.com/zeromicro/go-zero/core/trace"
 	"github.com/zeromicro/go-zero/rest/internal/response"
 	"go.opentelemetry.io/otel"
@@ -13,15 +12,26 @@ import (
 	oteltrace "go.opentelemetry.io/otel/trace"
 )
 
-var notTracingSpans sync.Map
+type (
+	// TracingOption defines the method to customize an tracingOptions.
+	TracingOption func(options *tracingOptions)
 
-// DontTraceSpan disable tracing for the specified span name.
-func DontTraceSpan(spanName string) {
-	notTracingSpans.Store(spanName, lang.Placeholder)
-}
+	// tracingOptions is TracingHandler options.
+	tracingOptions struct {
+		traceIgnorePaths []string
+	}
+)
 
 // TracingHandler return a middleware that process the opentelemetry.
-func TracingHandler(serviceName, path string) func(http.Handler) http.Handler {
+func TracingHandler(serviceName, path string, opts ...TracingOption) func(http.Handler) http.Handler {
+	var tracingOpts tracingOptions
+	for _, opt := range opts {
+		opt(&tracingOpts)
+	}
+
+	ignorePaths := collection.NewSet()
+	ignorePaths.AddStr(tracingOpts.traceIgnorePaths...)
+
 	return func(next http.Handler) http.Handler {
 		propagator := otel.GetTextMapPropagator()
 		tracer := otel.GetTracerProvider().Tracer(trace.TraceName)
@@ -32,7 +42,7 @@ func TracingHandler(serviceName, path string) func(http.Handler) http.Handler {
 				spanName = r.URL.Path
 			}
 
-			if _, ok := notTracingSpans.Load(spanName); ok {
+			if ignorePaths.Contains(spanName) {
 				next.ServeHTTP(w, r)
 				return
 			}
@@ -58,3 +68,10 @@ func TracingHandler(serviceName, path string) func(http.Handler) http.Handler {
 		})
 	}
 }
+
+// WithTraceIgnorePaths specifies the traceIgnorePaths option for TracingHandler.
+func WithTraceIgnorePaths(traceIgnorePaths []string) TracingOption {
+	return func(options *tracingOptions) {
+		options.traceIgnorePaths = traceIgnorePaths
+	}
+}


Test patch:
diff --git a/core/mapping/unmarshaler_test.go b/core/mapping/unmarshaler_test.go
index 8916e183..99a0984d 100644
--- a/core/mapping/unmarshaler_test.go
+++ b/core/mapping/unmarshaler_test.go
@@ -250,6 +250,62 @@ func TestUnmarshalIntWithDefault(t *testing.T) {
 	assert.Equal(t, 1, in.Int)
 }
 
+func TestUnmarshalIntWithString(t *testing.T) {
+	t.Run("int without options", func(t *testing.T) {
+		type inner struct {
+			Int int64 `key:"int,string"`
+		}
+		m := map[string]interface{}{
+			"int": json.Number("0"),
+		}
+
+		var in inner
+		assert.NoError(t, UnmarshalKey(m, &in))
+		assert.Equal(t, int64(0), in.Int)
+	})
+
+	t.Run("int with options", func(t *testing.T) {
+		type inner struct {
+			Int int64 `key:"int,string,options=[0,1]"`
+		}
+		m := map[string]interface{}{
+			"int": json.Number("0"),
+		}
+
+		var in inner
+		assert.NoError(t, UnmarshalKey(m, &in))
+		assert.Equal(t, int64(0), in.Int)
+	})
+
+	t.Run("int with options", func(t *testing.T) {
+		type inner struct {
+			Int int64 `key:"int,string,options=[0,1]"`
+		}
+		m := map[string]interface{}{
+			"int": nil,
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+
+	t.Run("int with options", func(t *testing.T) {
+		type (
+			StrType string
+
+			inner struct {
+				Int int64 `key:"int,string,options=[0,1]"`
+			}
+		)
+		m := map[string]interface{}{
+			"int": StrType("0"),
+		}
+
+		var in inner
+		assert.Error(t, UnmarshalKey(m, &in))
+	})
+}
+
 func TestUnmarshalBoolSliceRequired(t *testing.T) {
 	type inner struct {
 		Bools []bool `key:"bools"`
diff --git a/rest/handler/tracinghandler_test.go b/rest/handler/tracinghandler_test.go
index 79a94c4d..0e110010 100644
--- a/rest/handler/tracinghandler_test.go
+++ b/rest/handler/tracinghandler_test.go
@@ -63,12 +63,9 @@ func TestDontTracingSpan(t *testing.T) {
 	})
 	defer ztrace.StopAgent()
 
-	DontTraceSpan("bar")
-	defer notTracingSpans.Delete("bar")
-
 	for _, test := range []string{"", "bar", "foo"} {
 		t.Run(test, func(t *testing.T) {
-			h := chain.New(TracingHandler("foo", test)).Then(
+			h := chain.New(TracingHandler("foo", test, WithTraceIgnorePaths([]string{"bar"}))).Then(
 				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 					span := trace.SpanFromContext(r.Context())
 					spanCtx := span.SpanContext()