Remove routes > ignored_patterns configuration option (#33)

* Remove routes > ignored_patterns configuration option

* Fix integration test

Author: mariomac

pkg/export/debug/debug_test.go

@@ -66,8 +66,6 @@ func traceFuncHelper(t *testing.T, tracePrinter TracePrinter) string {
 		Statement:      "statement",
 	}
 
-	fakeSpan.SetIgnoreMetrics()
-
 	// redirect the TracePrinter function stdout to a pipe so that we can
 	// capture and return its output
 	r, w, err := os.Pipe()
@@ -115,7 +113,7 @@ func TestTracePrinterResolve_PrinterCounter(t *testing.T) {
 func TestTracePrinterResolve_PrinterJSON(t *testing.T) {
 	// test as separate chunks to exclude timestamps (start, handlerStart, end)
 
-	prefix := `[{"type":"HTTP","ignoreSpan":"Metrics","peer":"peer","peerPort":"1234",` +
+	prefix := `[{"type":"HTTP","peer":"peer","peerPort":"1234",` +
 		`"host":"host","hostPort":"5678","traceID":"01020300000000000000000000000000",` +
 		`"spanID":"0102030000000000","parentSpanID":"0102040000000000","flags":"1",` +
 		`"peerName":"peername","hostName":"hostname","kind":"SPAN_KIND_SERVER","`
@@ -136,7 +134,6 @@ func TestTracePrinterResolve_PrinterJSONIndent(t *testing.T) {
 	prefix := `[
  {
   "type": "HTTP",
-  "ignoreSpan": "Metrics",
   "peer": "peer",
   "peerPort": "1234",
   "host": "host",

pkg/export/otel/metrics.go

@@ -1040,10 +1040,6 @@ func (mr *MetricsReporter) reportMetrics(_ context.Context) {
 			if s.InternalSignal() {
 				continue
 			}
-			// If we are ignoring this span because of route patterns, don't do anything
-			if s.IgnoreMetrics() {
-				continue
-			}
 			reporter, err := mr.reporters.For(&s.Service)
 			if err != nil {
 				mlog().Error("unexpected error creating OTEL resource. Ignoring metric",

pkg/export/otel/traces.go

@@ -204,7 +204,7 @@ func (tr *tracesOTELReceiver) getConstantAttributes() (map[attr.Name]struct{}, e
 }
 
 func (tr *tracesOTELReceiver) spanDiscarded(span *request.Span) bool {
-	return span.IgnoreTraces() || span.Service.ExportsOTelTraces() || !tr.acceptSpan(span)
+	return span.Service.ExportsOTelTraces() || !tr.acceptSpan(span)
 }
 
 func (tr *tracesOTELReceiver) processSpans(ctx context.Context, exp exporter.Traces, spans []request.Span, traceAttrs map[attr.Name]struct{}, sampler trace.Sampler) {

pkg/export/prom/prom.go

@@ -733,7 +733,7 @@ func (r *metricsReporter) otelSpanObserved(span *request.Span) bool {
 }
 
 func (r *metricsReporter) otelSpanFiltered(span *request.Span) bool {
-	return span.InternalSignal() || span.IgnoreMetrics()
+	return span.InternalSignal()
 }
 
 //nolint:cyclop

pkg/export/prom/prom_test.go

@@ -314,45 +314,32 @@ func TestSpanMetricsDiscarded(t *testing.T) {
 	svcExportTraces := svc.Attrs{}
 	svcExportTraces.SetExportsOTelTraces()
 
-	ignoredSpan := request.Span{Service: svcExportTraces, Type: request.EventTypeHTTPClient, Method: "GET", Route: "/v1/traces", RequestStart: 100, End: 200}
-	ignoredSpan.SetIgnoreMetrics()
-
 	tests := []struct {
 		name      string
 		span      request.Span
 		discarded bool
-		filtered  bool
 	}{
 		{
 			name:      "Foo span is not filtered",
 			span:      request.Span{Service: svcNoExport, Type: request.EventTypeHTTPClient, Method: "GET", Route: "/foo", RequestStart: 100, End: 200},
 			discarded: false,
-			filtered:  false,
 		},
 		{
 			name:      "/v1/metrics span is filtered",
 			span:      request.Span{Service: svcExportMetrics, Type: request.EventTypeHTTPClient, Method: "GET", Route: "/v1/metrics", RequestStart: 100, End: 200},
 			discarded: true,
-			filtered:  false,
-		},
-		{
-			name:      "/v1/traces span is filtered because we ignore this span",
-			span:      ignoredSpan,
-			discarded: false,
-			filtered:  true,
 		},
 		{
 			name:      "/v1/traces span is not filtered",
 			span:      request.Span{Service: svcExportTraces, Type: request.EventTypeHTTPClient, Method: "GET", Route: "/v1/traces", RequestStart: 100, End: 200},
 			discarded: false,
-			filtered:  false,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			assert.Equal(t, tt.discarded, !(mr.otelSpanObserved(&tt.span)), tt.name)
-			assert.Equal(t, tt.filtered, mr.otelSpanFiltered(&tt.span), tt.name)
+			assert.False(t, mr.otelSpanFiltered(&tt.span), tt.name)
 		})
 	}
 }

pkg/internal/request/span.go

@@ -92,30 +92,6 @@ func (t EventType) MarshalText() ([]byte, error) {
 	return []byte(t.String()), nil
 }
 
-type ignoreMode uint8
-
-const (
-	ignoreMetrics ignoreMode = 0x1
-	ignoreTraces  ignoreMode = 0x2
-)
-
-func (m ignoreMode) String() string {
-	result := ""
-
-	if (m & ignoreMetrics) == ignoreMetrics {
-		result += "Metrics"
-	}
-	if (m & ignoreTraces) == ignoreTraces {
-		result += "Traces"
-	}
-
-	return result
-}
-
-func (m ignoreMode) MarshalText() ([]byte, error) {
-	return []byte(m.String()), nil
-}
-
 const (
 	MessagingPublish = "publish"
 	MessagingProcess = "process"
@@ -145,7 +121,6 @@ type PidInfo struct {
 // SpanPromGetters and getDefinitions in pkg/export/attributes/attr_defs.go
 type Span struct {
 	Type           EventType      `json:"type"`
-	IgnoreSpan     ignoreMode     `json:"ignoreSpan"`
 	Method         string         `json:"-"`
 	Path           string         `json:"-"`
 	Route          string         `json:"-"`
@@ -341,30 +316,6 @@ func (s *Span) IsClientSpan() bool {
 	return false
 }
 
-func (s *Span) setIgnoreFlag(flag ignoreMode) {
-	s.IgnoreSpan |= flag
-}
-
-func (s *Span) isIgnored(flag ignoreMode) bool {
-	return (s.IgnoreSpan & flag) == flag
-}
-
-func (s *Span) SetIgnoreMetrics() {
-	s.setIgnoreFlag(ignoreMetrics)
-}
-
-func (s *Span) SetIgnoreTraces() {
-	s.setIgnoreFlag(ignoreTraces)
-}
-
-func (s *Span) IgnoreMetrics() bool {
-	return s.isIgnored(ignoreMetrics)
-}
-
-func (s *Span) IgnoreTraces() bool {
-	return s.isIgnored(ignoreTraces)
-}
-
 const (
 	StatusCodeUnset = "STATUS_CODE_UNSET"
 	StatusCodeError = "STATUS_CODE_ERROR"

pkg/internal/request/span_test.go

@@ -46,19 +46,6 @@ func TestEventTypeString(t *testing.T) {
 	}
 }
 
-func TestIgnoreModeString(t *testing.T) {
-	modeStringMap := map[ignoreMode]string{
-		ignoreMetrics:                "Metrics",
-		ignoreTraces:                 "Traces",
-		ignoreMode(0):                "",
-		ignoreTraces | ignoreMetrics: "MetricsTraces",
-	}
-
-	for mode, str := range modeStringMap {
-		assert.Equal(t, mode.String(), str)
-	}
-}
-
 func TestKindString(t *testing.T) {
 	m := map[*Span]string{
 		{Type: EventTypeHTTP}:                                  "SPAN_KIND_SERVER",
@@ -181,7 +168,6 @@ func TestSerializeJSONSpans(t *testing.T) {
 	test := func(t *testing.T, tData *testData) {
 		span := Span{
 			Type:           tData.eventType,
-			IgnoreSpan:     ignoreMetrics,
 			Method:         "method",
 			Path:           "path",
 			Route:          "route",
@@ -216,7 +202,6 @@ func TestSerializeJSONSpans(t *testing.T) {
 		assert.Equal(t, map[string]any{
 			"type":                tData.eventType.String(),
 			"kind":                span.ServiceGraphKind(),
-			"ignoreSpan":          "Metrics",
 			"peer":                "peer",
 			"peerPort":            "1234",
 			"host":                "host",

pkg/transform/routes.go

@@ -47,9 +47,7 @@ type RoutesConfig struct {
 	// Unmatch specifies what to do when a route pattern is not matched
 	Unmatch UnmatchType `yaml:"unmatched"`
 	// Patterns of the paths that will match to a route
-	Patterns       []string   `yaml:"patterns"`
-	IgnorePatterns []string   `yaml:"ignored_patterns"`
-	IgnoredEvents  IgnoreMode `yaml:"ignore_mode"`
+	Patterns []string `yaml:"patterns"`
 	// Character that will be used to replace route segments
 	WildcardChar string `yaml:"wildcard_char,omitempty"`
 }
@@ -80,14 +78,7 @@ func (rn *routerNode) provideRoutes(_ context.Context) (swarm.RunFunc, error) {
 		return nil, err
 	}
 	matcher := route.NewMatcher(rc.Patterns)
-	discarder := route.NewMatcher(rc.IgnorePatterns)
 	routesEnabled := len(rc.Patterns) > 0
-	ignoreEnabled := len(rc.IgnorePatterns) > 0
-
-	ignoreMode := rc.IgnoredEvents
-	if ignoreMode == "" {
-		ignoreMode = IgnoreDefault
-	}
 
 	in := rn.input.Subscribe()
 	out := rn.output
@@ -98,16 +89,6 @@ func (rn *routerNode) provideRoutes(_ context.Context) (swarm.RunFunc, error) {
 		for spans := range in {
 			for i := range spans {
 				s := &spans[i]
-				if ignoreEnabled {
-					if discarder.Find(s.Path) != "" {
-						if ignoreMode == IgnoreAll {
-							s.SetIgnoreMetrics()
-							s.SetIgnoreTraces()
-						}
-						// we can't discard it here, ignoring is selective (metrics | traces)
-						setSpanIgnoreMode(ignoreMode, s)
-					}
-				}
 				if routesEnabled {
 					s.Route = matcher.Find(s.Path)
 				}
@@ -174,12 +155,3 @@ func classifyFromPath(rc *RoutesConfig, s *request.Span) {
 		s.Route = route.ClusterPath(s.Path, rc.WildcardChar[0])
 	}
 }
-
-func setSpanIgnoreMode(mode IgnoreMode, s *request.Span) {
-	switch mode {
-	case IgnoreMetrics:
-		s.SetIgnoreMetrics()
-	case IgnoreTraces:
-		s.SetIgnoreTraces()
-	}
-}

pkg/transform/routes_test.go

@@ -118,43 +118,6 @@ func TestUnmatchedAuto(t *testing.T) {
 	}
 }
 
-func TestIgnoreRoutes(t *testing.T) {
-	input := msg.NewQueue[[]request.Span](msg.ChannelBufferLen(10))
-	output := msg.NewQueue[[]request.Span](msg.ChannelBufferLen(10))
-	router, err := RoutesProvider(&RoutesConfig{
-		Unmatch: UnmatchPath, Patterns: []string{"/user/:id", "/v1/metrics"},
-		IgnorePatterns: []string{"/v1/metrics/*", "/v1/traces/*", "/exact"},
-	},
-		input, output)(t.Context())
-	require.NoError(t, err)
-	out := output.Subscribe()
-	defer input.Close()
-	go router(t.Context())
-	input.Send([]request.Span{{Path: "/user/1234"}})
-	input.Send([]request.Span{{Path: "/v1/metrics"}}) // this is in routes and ignore, ignore takes precedence
-	input.Send([]request.Span{{Path: "/v1/traces/1234/test"}})
-	input.Send([]request.Span{{Path: "/v1/metrics/1234/test"}}) // this is in routes and ignore, ignore takes precedence
-	input.Send([]request.Span{{Path: "/v1/traces"}})
-	input.Send([]request.Span{{Path: "/exact"}})
-	input.Send([]request.Span{{Path: "/some/path"}})
-	assert.Equal(t, []request.Span{{
-		Path:  "/user/1234",
-		Route: "/user/:id",
-	}}, filterIgnored(func() []request.Span { return testutil.ReadChannel(t, out, testTimeout) }))
-	assert.Equal(t, []request.Span{{
-		Path:  "/some/path",
-		Route: "/some/path",
-	}}, filterIgnored(func() []request.Span { return testutil.ReadChannel(t, out, testTimeout) }))
-}
-
-func TestIgnoreMode(t *testing.T) {
-	s := request.Span{Path: "/user/1234"}
-	setSpanIgnoreMode(IgnoreTraces, &s)
-	assert.True(t, s.IgnoreTraces())
-	setSpanIgnoreMode(IgnoreMetrics, &s)
-	assert.True(t, s.IgnoreMetrics())
-}
-
 func BenchmarkRoutesProvider_Wildcard(b *testing.B) {
 	benchProvider(b, UnmatchWildcard)
 }
@@ -188,27 +151,3 @@ func benchProvider(b *testing.B, unmatch UnmatchType) {
 		<-outCh
 	}
 }
-
-func filterIgnored(reader func() []request.Span) []request.Span {
-	for {
-		input := reader()
-		output := make([]request.Span, 0, len(input))
-		for i := range input {
-			s := &input[i]
-
-			if s.IgnoreMetrics() {
-				continue
-			}
-
-			if s.IgnoreTraces() {
-				continue
-			}
-
-			output = append(output, *s)
-		}
-
-		if len(output) > 0 {
-			return output
-		}
-	}
-}

test/integration/configs/instrumenter-config-grpc-export.yml

@@ -1,10 +1,11 @@
 routes:
   patterns:
     - /basic/:rnd
-  ignored_patterns:
-    - /metrics
-  ignore_mode: traces
   unmatched: path
+filter:
+  application:
+    url_path:
+      not_match: /metrics
 otel_metrics_export:
   endpoint: http://otelcol:4317
   protocol: grpc