Merge branch 'master' into feat/upgrade-otel-1.26.0

CHANGELOG.md

@@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## [Unreleased]
 
+### Changed
+
+- `WithFilter` option now support multiple filter functions, just like in [otelmux](https://github.com/open-telemetry/opentelemetry-go-contrib/blob/v1.24.0/instrumentation/github.com/gorilla/mux/otelmux/config.go#L106-L110). ([#47])
+
 ## [0.8.0] - 2024-04-29
 
 ### ⚠️ Notice ⚠️
@@ -161,6 +165,7 @@ It contains instrumentation for trace and depends on:
 - Example code for a basic usage.
 - Apache-2.0 license.
 
+[#47]: https://github.com/riandyrn/otelchi/pull/47
 [#43]: https://github.com/riandyrn/otelchi/pull/43
 [#42]: https://github.com/riandyrn/otelchi/pull/42
 [#41]: https://github.com/riandyrn/otelchi/pull/41

config.go

@@ -16,7 +16,7 @@ type config struct {
 	Propagators             propagation.TextMapPropagator
 	ChiRoutes               chi.Routes
 	RequestMethodInSpanName bool
-	Filter                  func(r *http.Request) bool
+	Filters                 []Filter
 	TraceResponseHeaderKey  string
 	PublicEndpointFn        func(r *http.Request) bool
 }
@@ -32,6 +32,10 @@ func (o optionFunc) apply(c *config) {
 	o(c)
 }
 
+// Filter is a predicate used to determine whether a given http.request should
+// be traced. A Filter must return true if the request should be traced.
+type Filter func(*http.Request) bool
+
 // WithPropagators specifies propagators to use for extracting
 // information from the HTTP requests. If none are specified, global
 // ones will be used.
@@ -76,12 +80,15 @@ func WithRequestMethodInSpanName(isActive bool) Option {
 	})
 }
 
-// WithFilter is used for filtering request that should not be traced.
-// This is useful for filtering health check request, etc.
-// A Filter must return true if the request should be traced.
-func WithFilter(filter func(r *http.Request) bool) Option {
+// WithFilter adds a filter to the list of filters used by the handler.
+// If any filter indicates to exclude a request then the request will not be
+// traced. All filters must allow a request to be traced for a Span to be created.
+// If no filters are provided then all requests are traced.
+// Filters will be invoked for each processed request, it is advised to make them
+// simple and fast.
+func WithFilter(filter Filter) Option {
 	return optionFunc(func(cfg *config) {
-		cfg.Filter = filter
+		cfg.Filters = append(cfg.Filters, filter)
 	})
 }
 

middleware.go

@@ -46,7 +46,7 @@ func Middleware(serverName string, opts ...Option) func(next http.Handler) http.
 			handler:                handler,
 			chiRoutes:              cfg.ChiRoutes,
 			reqMethodInSpanName:    cfg.RequestMethodInSpanName,
-			filter:                 cfg.Filter,
+			filters:                cfg.Filters,
 			traceResponseHeaderKey: cfg.TraceResponseHeaderKey,
 			publicEndpointFn:       cfg.PublicEndpointFn,
 		}
@@ -60,7 +60,7 @@ type traceware struct {
 	handler                http.Handler
 	chiRoutes              chi.Routes
 	reqMethodInSpanName    bool
-	filter                 func(r *http.Request) bool
+	filters                []Filter
 	traceResponseHeaderKey string
 	publicEndpointFn       func(r *http.Request) bool
 }
@@ -111,10 +111,14 @@ func putRRW(rrw *recordingResponseWriter) {
 // ServeHTTP implements the http.Handler interface. It does the actual
 // tracing of the request.
 func (tw traceware) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	// skip if filter returns false
-	if tw.filter != nil && !tw.filter(r) {
-		tw.handler.ServeHTTP(w, r)
-		return
+	// go through all filters if any
+	for _, filter := range tw.filters {
+		// if there is a filter that returns false, we skip tracing
+		// and execute next handler
+		if !filter(r) {
+			tw.handler.ServeHTTP(w, r)
+			return
+		}
 	}
 
 	// extract tracing header using propagator

test/cases/sdk_test.go

@@ -63,59 +63,107 @@ func TestSDKIntegration(t *testing.T) {
 	})
 }
 
-func TestSDKIntegrationWithFilters(t *testing.T) {
-	// prepare router and span recorder
-	router, sr := newSDKTestRouter("foobar", false, otelchi.WithFilter(func(r *http.Request) bool {
-		// if client access /live or /ready, there should be no span
-		if r.URL.Path == "/live" || r.URL.Path == "/ready" {
-			return false
-		}
-
-		// otherwise always return the span
-		return true
-	}))
-
-	// define router
-	router.HandleFunc("/user/{id:[0-9]+}", ok)
-	router.HandleFunc("/book/{title}", ok)
-	router.HandleFunc("/health", ok)
-	router.HandleFunc("/ready", ok)
-
-	// execute requests
-	executeRequests(router, []*http.Request{
-		httptest.NewRequest("GET", "/user/123", nil),
-		httptest.NewRequest("GET", "/book/foo", nil),
-		httptest.NewRequest("GET", "/live", nil),
-		httptest.NewRequest("GET", "/ready", nil),
-	})
-
-	// get recorded spans and ensure the length is 2
-	recordedSpans := sr.Ended()
-	require.Len(t, recordedSpans, 2)
-
-	// ensure span values
-	checkSpans(t, recordedSpans, []spanValueCheck{
+func TestSDKIntegrationWithFilter(t *testing.T) {
+	// prepare test cases
+	serviceName := "foobar"
+	testCases := []struct {
+		Name               string
+		FilterFn           []otelchi.Filter
+		LenSpans           int
+		ExpectedRouteNames []string
+	}{
 		{
-			Name: "/user/{id:[0-9]+}",
-			Kind: trace.SpanKindServer,
-			Attributes: getSemanticAttributes(
-				"foobar",
-				http.StatusOK,
-				"GET",
-				"/user/{id:[0-9]+}",
-			),
+			Name: "One WithFilter",
+			FilterFn: []otelchi.Filter{
+				func(r *http.Request) bool {
+					return r.URL.Path != "/live" && r.URL.Path != "/ready"
+				},
+			},
+			LenSpans:           2,
+			ExpectedRouteNames: []string{"/user/{id:[0-9]+}", "/book/{title}"},
 		},
 		{
-			Name: "/book/{title}",
-			Kind: trace.SpanKindServer,
-			Attributes: getSemanticAttributes(
-				"foobar",
-				http.StatusOK,
-				"GET",
-				"/book/{title}",
-			),
+			Name: "Multiple WithFilter",
+			FilterFn: []otelchi.Filter{
+				func(r *http.Request) bool {
+					return r.URL.Path != "/ready"
+				},
+				func(r *http.Request) bool {
+					return r.URL.Path != "/live"
+				},
+			},
+			LenSpans:           2,
+			ExpectedRouteNames: []string{"/user/{id:[0-9]+}", "/book/{title}"},
 		},
-	})
+		{
+			Name: "All Routes are traced",
+			FilterFn: []otelchi.Filter{
+				func(r *http.Request) bool {
+					return true
+				},
+			},
+			LenSpans:           4,
+			ExpectedRouteNames: []string{"/user/{id:[0-9]+}", "/book/{title}", "/live", "/ready"},
+		},
+		{
+			Name: "All Routes are not traced",
+			FilterFn: []otelchi.Filter{
+				func(r *http.Request) bool {
+					return false
+				},
+			},
+			LenSpans:           0,
+			ExpectedRouteNames: []string{},
+		},
+	}
+
+	// execute test cases
+	for _, testCase := range testCases {
+		t.Run(testCase.Name, func(t *testing.T) {
+			// prepare router and span recorder
+			filters := []otelchi.Option{}
+			for _, filter := range testCase.FilterFn {
+				filters = append(filters, otelchi.WithFilter(filter))
+			}
+			router, sr := newSDKTestRouter(serviceName, false, filters...)
+
+			// define router
+			router.HandleFunc("/user/{id:[0-9]+}", ok)
+			router.HandleFunc("/book/{title}", ok)
+			router.HandleFunc("/health", ok)
+			router.HandleFunc("/live", ok)
+			router.HandleFunc("/ready", ok)
+
+			// execute requests
+			executeRequests(router, []*http.Request{
+				httptest.NewRequest("GET", "/user/123", nil),
+				httptest.NewRequest("GET", "/book/foo", nil),
+				httptest.NewRequest("GET", "/live", nil),
+				httptest.NewRequest("GET", "/ready", nil),
+			})
+
+			// check recorded spans
+			recordedSpans := sr.Ended()
+			require.Len(t, recordedSpans, testCase.LenSpans)
+
+			// ensure span values
+			spanValues := []spanValueCheck{}
+			for _, routeName := range testCase.ExpectedRouteNames {
+				spanValues = append(spanValues, spanValueCheck{
+					Name: routeName,
+					Kind: trace.SpanKindServer,
+					Attributes: getSemanticAttributes(
+						serviceName,
+						http.StatusOK,
+						"GET",
+						routeName,
+					),
+				})
+			}
+			checkSpans(t, recordedSpans, spanValues)
+		})
+	}
+
 }
 
 func TestSDKIntegrationWithChiRoutes(t *testing.T) {