allow tagFilter to use response data (#2607)

This change introduces a new filter tracingTagFromResponse that works just like the tracingTag filter, but is applied only after ther request has been processed. This allows using properties of the response (a header value) in the tag.

The use case for this is that users might want to consider an operation as failed even if it technically succeeds, e.g. because fallbacks were used. With this change, a response header can be sent, e.g. `"X-Fallback": "true"`, and then the filter `tracingTagFromResponse("error", "${response.header.X-Fallback}")` would result in the span being marked as error.

Another use cases could be that metrics should be captured by use case, but the use case is not apparent from the request and instead only defined by the result of the request processing.

Signed-off-by: lukas-c-wilhelm <[email protected]>

docs/reference/filters.md

@@ -1696,8 +1696,8 @@ Given following example ID token:
   "email": "[email protected]",
   "groups": [
     "CD-xyz",
-    "appX-Test-Users"
-    "Purchasing-Department",
+    "appX-Test-Users",
+    "Purchasing-Department"
   ],
   "name": "Some One"
 }
@@ -3013,6 +3013,10 @@ Example: Set tag from request header
 tracingTag("http.flow_id", "${request.header.X-Flow-Id}")
 ```
 
+### tracingTagFromResponse
+
+This filter works just like [tracingTag](#tracingTag), but is applied after the request was processed. In particular, [template placeholders](#template-placeholders) referencing the response can be used in the parameters. 
+
 ### tracingSpanName
 
 This filter sets the name of the outgoing (client span) in opentracing. The default name is "proxy". Example:

filters/builtin/builtin.go

@@ -207,6 +207,7 @@ func Filters() []filters.Spec {
 		tracing.NewSpanName(),
 		tracing.NewBaggageToTagFilter(),
 		tracing.NewTag(),
+		tracing.NewTagFromResponse(),
 		tracing.NewStateBagToTag(),
 		//lint:ignore SA1019 due to backward compatibility
 		accesslog.NewAccessLogDisabled(),

filters/filters.go

@@ -335,6 +335,7 @@ const (
 	TracingBaggageToTagName                    = "tracingBaggageToTag"
 	StateBagToTagName                          = "stateBagToTag"
 	TracingTagName                             = "tracingTag"
+	TracingTagFromResponseName                 = "tracingTagFromResponse"
 	TracingSpanNameName                        = "tracingSpanName"
 	OriginMarkerName                           = "originMarker"
 	FadeInName                                 = "fadeIn"

filters/tracing/tag.go

@@ -7,23 +7,31 @@ import (
 )
 
 type tagSpec struct {
+	typ string
 }
 
 type tagFilter struct {
+	tagFromResponse bool
+
 	tagName  string
 	tagValue *eskip.Template
 }
 
 // NewTag creates a filter specification for the tracingTag filter.
 func NewTag() filters.Spec {
-	return tagSpec{}
+	return &tagSpec{typ: filters.TracingTagName}
+}
+
+// NewTagFromResponse creates a filter similar to NewTag, but applies tags after the request has been processed.
+func NewTagFromResponse() filters.Spec {
+	return &tagSpec{typ: filters.TracingTagFromResponseName}
 }
 
-func (s tagSpec) Name() string {
-	return filters.TracingTagName
+func (s *tagSpec) Name() string {
+	return s.typ
 }
 
-func (s tagSpec) CreateFilter(args []interface{}) (filters.Filter, error) {
+func (s *tagSpec) CreateFilter(args []interface{}) (filters.Filter, error) {
 	if len(args) != 2 {
 		return nil, filters.ErrInvalidFilterParameters
 	}
@@ -38,15 +46,28 @@ func (s tagSpec) CreateFilter(args []interface{}) (filters.Filter, error) {
 		return nil, filters.ErrInvalidFilterParameters
 	}
 
-	return tagFilter{
+	return &tagFilter{
+		tagFromResponse: s.typ == filters.TracingTagFromResponseName,
+
 		tagName:  tagName,
 		tagValue: eskip.NewTemplate(tagValue),
 	}, nil
 }
 
-func (f tagFilter) Request(ctx filters.FilterContext) {
-	req := ctx.Request()
-	span := opentracing.SpanFromContext(req.Context())
+func (f *tagFilter) Request(ctx filters.FilterContext) {
+	if !f.tagFromResponse {
+		f.setTag(ctx)
+	}
+}
+
+func (f *tagFilter) Response(ctx filters.FilterContext) {
+	if f.tagFromResponse {
+		f.setTag(ctx)
+	}
+}
+
+func (f *tagFilter) setTag(ctx filters.FilterContext) {
+	span := opentracing.SpanFromContext(ctx.Request().Context())
 	if span == nil {
 		return
 	}
@@ -55,5 +76,3 @@ func (f tagFilter) Request(ctx filters.FilterContext) {
 		span.SetTag(f.tagName, v)
 	}
 }
-
-func (f tagFilter) Response(filters.FilterContext) {}

filters/tracing/tag_test.go

@@ -31,10 +31,14 @@ func TestTracingTagNil(t *testing.T) {
 }
 
 func TestTagName(t *testing.T) {
-	if (tagSpec{}).Name() != filters.TracingTagName {
+	if NewTag().Name() != filters.TracingTagName {
+		t.Error("Wrong tag spec name")
+	}
+	if NewTagFromResponse().Name() != filters.TracingTagFromResponseName {
 		t.Error("Wrong tag spec name")
 	}
 }
+
 func TestTagCreateFilter(t *testing.T) {
 	spec := tagSpec{}
 	if _, err := spec.CreateFilter(nil); err != filters.ErrInvalidFilterParameters {
@@ -55,18 +59,21 @@ func TestTracingTag(t *testing.T) {
 
 	for _, ti := range []struct {
 		name     string
+		spec     filters.Spec
 		value    string
 		context  *filtertest.Context
 		expected interface{}
 	}{{
 		"plain key value",
+		NewTag(),
 		"test_value",
 		&filtertest.Context{
 			FRequest: &http.Request{},
 		},
 		"test_value",
 	}, {
 		"tag from header",
+		NewTag(),
 		"${request.header.X-Flow-Id}",
 		&filtertest.Context{
 			FRequest: &http.Request{
@@ -76,34 +83,64 @@ func TestTracingTag(t *testing.T) {
 			},
 		},
 		"foo",
+	}, {
+		"tag from response",
+		NewTagFromResponse(),
+		"${response.header.X-Fallback}",
+		&filtertest.Context{
+			FRequest: &http.Request{},
+			FResponse: &http.Response{
+				Header: http.Header{
+					"X-Fallback": []string{"true"},
+				},
+			},
+		},
+		"true",
 	}, {
 		"tag from missing header",
+		NewTag(),
 		"${request.header.missing}",
 		&filtertest.Context{
 			FRequest: &http.Request{},
 		},
 		nil,
+	}, {
+		"tracingTag is not processed on response",
+		NewTag(),
+		"${response.header.X-Fallback}",
+		&filtertest.Context{
+			FRequest: &http.Request{},
+			FResponse: &http.Response{
+				Header: http.Header{
+					"X-Fallback": []string{"true"},
+				},
+			},
+		},
+		nil,
 	},
 	} {
 		t.Run(ti.name, func(t *testing.T) {
 			span := tracer.StartSpan("proxy").(*mocktracer.MockSpan)
 			defer span.Finish()
 
-			ti.context.FRequest = ti.context.FRequest.WithContext(opentracing.ContextWithSpan(ti.context.FRequest.Context(), span))
+			requestContext := &filtertest.Context{
+				FRequest: ti.context.FRequest.WithContext(opentracing.ContextWithSpan(ti.context.FRequest.Context(), span)),
+			}
 
-			s := NewTag()
-			f, err := s.CreateFilter([]interface{}{"test_tag", ti.value})
+			f, err := ti.spec.CreateFilter([]interface{}{"test_tag", ti.value})
 			if err != nil {
 				t.Fatal(err)
 			}
 
-			f.Request(ti.context)
+			f.Request(requestContext)
+
+			requestContext.FResponse = ti.context.FResponse
+
+			f.Response(requestContext)
 
 			if got := span.Tag("test_tag"); got != ti.expected {
 				t.Errorf("unexpected tag value '%v' != '%v'", got, ti.expected)
 			}
-
-			f.Response(ti.context)
 		})
 	}
 }