From 97ee0c7a8ee225984c4f2d4a43c9c10b152129d1 Mon Sep 17 00:00:00 2001 From: Paul Salaberria Date: Fri, 1 Apr 2022 15:00:10 +0200 Subject: [PATCH] Allow setting custom_tags for traces It allows setting trace tags based on literal values, environment variables, or request headers. Based on https://www.envoyproxy.io/docs/envoy/latest/api-v3/type/tracing/v3/custom_tag.proto#custom-tag Signed-off-by: Paul Salaberria --- CHANGELOG.md | 5 + docs/releaseNotes.yml | 10 ++ manifests/emissary/emissary-crds.yaml.in | 89 ++++++++++++ pkg/api/getambassador.io/conversion_test.go | 132 +++++++++++++++++- pkg/api/getambassador.io/crds.yaml | 89 ++++++++++++ .../getambassador.io/v2/crd_tracingservice.go | 2 + .../v2/handwritten.conversion.go | 53 +++++++ .../v2/zz_generated.conversion.go | 42 +++--- .../v2/zz_generated.deepcopy.go | 7 + .../v3alpha1/crd_tracingservice.go | 47 ++++++- .../v3alpha1/testdata/tracingsvc.yaml | 6 + .../v3alpha1/zz_generated.deepcopy.go | 96 ++++++++++++- python/ambassador/envoy/v3/v3listener.py | 23 +-- python/ambassador/ir/irtracing.py | 2 + python/tests/integration/manifests/crds.yaml | 89 ++++++++++++ python/tests/kat/t_tracing.py | 22 ++- python/tests/unit/test_tracing.py | 22 ++- 17 files changed, 689 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 884e78e04e..6aec35aaf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,7 +97,12 @@ Please see the [Envoy documentation](https://www.envoyproxy.io/docs/envoy/latest - Feature: It is now possible to set `propagation_modes` in the `TracingService` config when using lightstep as the driver. (Thanks to Paul!) ([#4179]) +- Feature: It is now possible to set `custom_tags` in the `TracingService`. Trace tags can be set + based on literal values, environment variables, or request headers. (Thanks to Paul!) ([#4181]) + [#4179]: https://github.com/emissary-ingress/emissary/pull/4179 +[#4181]: https://github.com/emissary-ingress/emissary/pull/4181 ## [2.2.2] TBD [2.2.2]: https://github.com/emissary-ingress/emissary/compare/v2.2.1...v2.2.2 diff --git a/docs/releaseNotes.yml b/docs/releaseNotes.yml index 2770fbd6cd..3909a1c9bd 100644 --- a/docs/releaseNotes.yml +++ b/docs/releaseNotes.yml @@ -48,6 +48,16 @@ items: github: - title: "#4179" link: https://github.com/emissary-ingress/emissary/pull/4179 + - title: Allow setting custom_tags for traces + type: feature + body: >- + It is now possible to set custom_tags in the + TracingService. Trace tags can be set based on + literal values, environment variables, or request headers. + (Thanks to Paul!) + github: + - title: "#4181" + link: https://github.com/emissary-ingress/emissary/pull/4181 - version: 2.2.2 date: 'TBD' diff --git a/manifests/emissary/emissary-crds.yaml.in b/manifests/emissary/emissary-crds.yaml.in index 7697f43118..9aa89c54a1 100644 --- a/manifests/emissary/emissary-crds.yaml.in +++ b/manifests/emissary/emissary-crds.yaml.in @@ -3727,6 +3727,49 @@ spec: items: type: string type: array + v3CustomTags: + items: + properties: + environment: + description: Environment explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + literal: + description: Literal explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + value: + type: string + required: + - value + type: object + request_header: + description: Header explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + tag: + type: string + required: + - tag + type: object + type: array v3StatsName: type: string required: @@ -3798,6 +3841,49 @@ spec: trace_id_128bit: type: boolean type: object + custom_tags: + items: + properties: + environment: + description: Environment explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + literal: + description: Literal explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + value: + type: string + required: + - value + type: object + request_header: + description: Header explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + tag: + type: string + required: + - tag + type: object + type: array driver: enum: - lightstep @@ -3818,6 +3904,9 @@ spec: stats_name: type: string tag_headers: + description: 'tag_headers is deprecated. Use custom_tags instead. + `tag_headers: ["header"]` can be defined as `custom_tags: [{"request_header": + {"name": "header"}}]`.' items: type: string type: array diff --git a/pkg/api/getambassador.io/conversion_test.go b/pkg/api/getambassador.io/conversion_test.go index 314e84c187..becba42b1b 100644 --- a/pkg/api/getambassador.io/conversion_test.go +++ b/pkg/api/getambassador.io/conversion_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" - "github.com/datawire/ambassador/v2/pkg/api/getambassador.io/v2" + v2 "github.com/datawire/ambassador/v2/pkg/api/getambassador.io/v2" "github.com/datawire/ambassador/v2/pkg/api/getambassador.io/v3alpha1" ) @@ -146,3 +146,133 @@ func TestConvert(t *testing.T) { } }) } + +func TestConvertTracingService(t *testing.T) { + + scheme := BuildScheme() + + // v3alpha1 to v2 + + // only custom_tags set + o := &v2.TracingServiceSpec{} + scheme.Convert(&v3alpha1.TracingServiceSpec{ + AmbassadorID: v3alpha1.AmbassadorID{}, + CustomTags: []v3alpha1.TracingCustomTag{ + { + Tag: "hola", + Header: &v3alpha1.TracingCustomTagTypeRequestHeader{ + Name: "hola", + }, + }, + }, + }, o, nil) + if len(o.V3CustomTags) != 1 { + t.Errorf("got %d; want 1", len(o.V3CustomTags)) + } + if len(o.TagHeaders) != 0 { + t.Errorf("got %d; want 0", len(o.TagHeaders)) + } + + // both custom_tags and tag_headers set + o2 := &v2.TracingServiceSpec{} + scheme.Convert(&v3alpha1.TracingServiceSpec{ + AmbassadorID: v3alpha1.AmbassadorID{}, + DeprecatedTagHeaders: []string{"hello"}, + CustomTags: []v3alpha1.TracingCustomTag{ + { + Tag: "hola", + Header: &v3alpha1.TracingCustomTagTypeRequestHeader{ + Name: "hola", + }, + }, + { + Tag: "env", + Environment: &v3alpha1.TracingCustomTagTypeEnvironment{ + Name: "env", + }, + }, + }, + }, o2, nil) + if len(o2.V3CustomTags) != 2 { + t.Errorf("got %d; want 2", len(o2.V3CustomTags)) + } + if len(o2.TagHeaders) != 0 { + t.Errorf("got %d; want 0", len(o2.TagHeaders)) + } + + // only tag_headers set + o3 := &v2.TracingServiceSpec{} + scheme.Convert(&v3alpha1.TracingServiceSpec{ + AmbassadorID: v3alpha1.AmbassadorID{}, + DeprecatedTagHeaders: []string{"hello"}, + }, o3, nil) + if len(o3.V3CustomTags) != 1 { + t.Errorf("got %d; want 1", len(o3.V3CustomTags)) + } + if len(o3.TagHeaders) != 0 { + t.Errorf("got %d; want 0", len(o3.TagHeaders)) + } + + // v2 to v3alpha1 + + // only tag_headers set + out := &v3alpha1.TracingServiceSpec{} + scheme.Convert(&v2.TracingServiceSpec{ + AmbassadorID: v2.AmbassadorID{}, + TagHeaders: []string{"hola"}, + }, out, nil) + if len(out.CustomTags) != 1 { + t.Errorf("got %d; want 1", len(out.CustomTags)) + } + if len(out.DeprecatedTagHeaders) != 0 { + t.Errorf("got %d; want 0", len(out.DeprecatedTagHeaders)) + } + + // only v3CustomTags set + out2 := &v3alpha1.TracingServiceSpec{} + scheme.Convert(&v2.TracingServiceSpec{ + AmbassadorID: v2.AmbassadorID{}, + V3CustomTags: []v3alpha1.TracingCustomTag{ + { + Tag: "hello", + Header: &v3alpha1.TracingCustomTagTypeRequestHeader{ + Name: "hello", + }, + }, + }, + }, out2, nil) + if len(out2.CustomTags) != 1 { + t.Errorf("got %d; want 1", len(out2.CustomTags)) + } + if len(out2.DeprecatedTagHeaders) != 0 { + t.Errorf("got %d; want 0", len(out2.DeprecatedTagHeaders)) + } + + // both custom_tags and tag_headers set + out3 := &v3alpha1.TracingServiceSpec{} + scheme.Convert(&v2.TracingServiceSpec{ + AmbassadorID: v2.AmbassadorID{}, + TagHeaders: []string{"hola"}, + V3CustomTags: []v3alpha1.TracingCustomTag{ + { + Tag: "hello", + Header: &v3alpha1.TracingCustomTagTypeRequestHeader{ + Name: "hello", + }, + }, + { + Tag: "hello2", + Environment: &v3alpha1.TracingCustomTagTypeEnvironment{ + Name: "hello2", + }, + }, + }, + }, out3, nil) + if len(out3.CustomTags) != 2 { + t.Errorf("got %d; want 2", len(out3.CustomTags)) + } + if len(out3.DeprecatedTagHeaders) != 0 { + t.Errorf("got %d; want 0", len(out3.DeprecatedTagHeaders)) + } + +} diff --git a/pkg/api/getambassador.io/crds.yaml b/pkg/api/getambassador.io/crds.yaml index c6e06981d9..5ea14473f6 100644 --- a/pkg/api/getambassador.io/crds.yaml +++ b/pkg/api/getambassador.io/crds.yaml @@ -3786,6 +3786,49 @@ spec: items: type: string type: array + v3CustomTags: + items: + properties: + environment: + description: Environment explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + literal: + description: Literal explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + value: + type: string + required: + - value + type: object + request_header: + description: Header explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + tag: + type: string + required: + - tag + type: object + type: array v3StatsName: type: string required: @@ -3856,6 +3899,49 @@ spec: trace_id_128bit: type: boolean type: object + custom_tags: + items: + properties: + environment: + description: Environment explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + literal: + description: Literal explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + value: + type: string + required: + - value + type: object + request_header: + description: Header explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + tag: + type: string + required: + - tag + type: object + type: array driver: enum: - lightstep @@ -3876,6 +3962,9 @@ spec: stats_name: type: string tag_headers: + description: 'tag_headers is deprecated. Use custom_tags instead. + `tag_headers: ["header"]` can be defined as `custom_tags: [{"request_header": + {"name": "header"}}]`.' items: type: string type: array diff --git a/pkg/api/getambassador.io/v2/crd_tracingservice.go b/pkg/api/getambassador.io/v2/crd_tracingservice.go index c26d6f1eeb..536d0a2c7a 100644 --- a/pkg/api/getambassador.io/v2/crd_tracingservice.go +++ b/pkg/api/getambassador.io/v2/crd_tracingservice.go @@ -60,6 +60,8 @@ type TracingServiceSpec struct { // +k8s:conversion-gen:rename=StatsName V3StatsName string `json:"v3StatsName,omitempty"` + // +k8s:conversion-gen:rename=CustomTags + V3CustomTags []v3alpha1.TracingCustomTag `json:"v3CustomTags,omitempty"` } // TracingService is the Schema for the tracingservices API diff --git a/pkg/api/getambassador.io/v2/handwritten.conversion.go b/pkg/api/getambassador.io/v2/handwritten.conversion.go index 140c8dc6f9..a88946f9a0 100644 --- a/pkg/api/getambassador.io/v2/handwritten.conversion.go +++ b/pkg/api/getambassador.io/v2/handwritten.conversion.go @@ -9,6 +9,7 @@ package v2 import ( + "fmt" "strings" "k8s.io/apimachinery/pkg/conversion" @@ -658,3 +659,55 @@ func Convert_v3alpha1_TCPMappingSpec_To_v2_TCPMappingSpec(in *v3alpha1.TCPMappin return nil } + +func Convert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec(in *TracingServiceSpec, out *v3alpha1.TracingServiceSpec, s conversion.Scope) error { + if err := autoConvert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec(in, out, s); err != nil { + return err + } + // WARNING: in.TagHeaders requires manual conversion: does not exist in peer-type + // if only tag_headers are set, translate to custom_tags. + // if both are set, ignore tag_headers. + if in.TagHeaders != nil { + if in.V3CustomTags == nil { + out.CustomTags = []v3alpha1.TracingCustomTag{} + for _, tag := range in.TagHeaders { + out.CustomTags = append(out.CustomTags, v3alpha1.TracingCustomTag{ + Tag: tag, + Header: &v3alpha1.TracingCustomTagTypeRequestHeader{ + Name: tag, + }, + }) + } + } + } + return nil +} + +func Convert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(in *v3alpha1.TracingServiceSpec, out *TracingServiceSpec, s conversion.Scope) error { + in = in.DeepCopy() + // if only tag_headers are set, translate to custom_tags. + // if both are set, log a warning and ignore tag_headers. + if in.DeprecatedTagHeaders != nil { + if in.CustomTags == nil { + in.CustomTags = []v3alpha1.TracingCustomTag{} + for _, tag := range in.DeprecatedTagHeaders { + in.CustomTags = append(in.CustomTags, v3alpha1.TracingCustomTag{ + Tag: tag, + Header: &v3alpha1.TracingCustomTagTypeRequestHeader{ + Name: tag, + }, + }) + } + } else { + // TODO: Use dlog logger + fmt.Printf("CustomTags and TagHeaders cannot be set at the same time in a TracingService. ignoring TagHeaders since it is deprecated.") + } + } + + if err := autoConvert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(in, out, s); err != nil { + return err + } + // WARNING: in.DeprecatedTagHeaders requires manual conversion: does not exist in peer-type + // see above + return nil +} diff --git a/pkg/api/getambassador.io/v2/zz_generated.conversion.go b/pkg/api/getambassador.io/v2/zz_generated.conversion.go index 0a629a6881..91f248f46e 100644 --- a/pkg/api/getambassador.io/v2/zz_generated.conversion.go +++ b/pkg/api/getambassador.io/v2/zz_generated.conversion.go @@ -722,16 +722,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*TracingServiceSpec)(nil), (*v3alpha1.TracingServiceSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec(a.(*TracingServiceSpec), b.(*v3alpha1.TracingServiceSpec), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v3alpha1.TracingServiceSpec)(nil), (*TracingServiceSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(a.(*v3alpha1.TracingServiceSpec), b.(*TracingServiceSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*UntypedDict)(nil), (*v3alpha1.UntypedDict)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v2_UntypedDict_To_v3alpha1_UntypedDict(a.(*UntypedDict), b.(*v3alpha1.UntypedDict), scope) }); err != nil { @@ -797,6 +787,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*TracingServiceSpec)(nil), (*v3alpha1.TracingServiceSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec(a.(*TracingServiceSpec), b.(*v3alpha1.TracingServiceSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v3alpha1.AddedHeader)(nil), (*AddedHeader)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v3alpha1_AddedHeader_To_v2_AddedHeader(a.(*v3alpha1.AddedHeader), b.(*AddedHeader), scope) }); err != nil { @@ -842,6 +837,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v3alpha1.TracingServiceSpec)(nil), (*TracingServiceSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(a.(*v3alpha1.TracingServiceSpec), b.(*TracingServiceSpec), scope) + }); err != nil { + return err + } return nil } @@ -5573,10 +5573,7 @@ func autoConvert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec(in *Tracin } } } - if true { - in, out := &in.TagHeaders, &out.TagHeaders - *out = *in - } + // WARNING: in.TagHeaders requires manual conversion: does not exist in peer-type if true { in, out := &in.Config, &out.Config if *in == nil { @@ -5593,14 +5590,13 @@ func autoConvert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec(in *Tracin in, out := &in.V3StatsName, &out.StatsName *out = *in } + if true { + in, out := &in.V3CustomTags, &out.CustomTags + *out = *in + } return nil } -// Convert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec is an autogenerated conversion function. -func Convert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec(in *TracingServiceSpec, out *v3alpha1.TracingServiceSpec, s conversion.Scope) error { - return autoConvert_v2_TracingServiceSpec_To_v3alpha1_TracingServiceSpec(in, out, s) -} - func autoConvert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(in *v3alpha1.TracingServiceSpec, out *TracingServiceSpec, s conversion.Scope) error { if true { in, out := &in.AmbassadorID, &out.AmbassadorID @@ -5628,8 +5624,9 @@ func autoConvert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(in *v3alph } } } + // WARNING: in.DeprecatedTagHeaders requires manual conversion: does not exist in peer-type if true { - in, out := &in.TagHeaders, &out.TagHeaders + in, out := &in.CustomTags, &out.V3CustomTags *out = *in } if true { @@ -5651,11 +5648,6 @@ func autoConvert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(in *v3alph return nil } -// Convert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec is an autogenerated conversion function. -func Convert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(in *v3alpha1.TracingServiceSpec, out *TracingServiceSpec, s conversion.Scope) error { - return autoConvert_v3alpha1_TracingServiceSpec_To_v2_TracingServiceSpec(in, out, s) -} - func autoConvert_v2_UntypedDict_To_v3alpha1_UntypedDict(in *UntypedDict, out *v3alpha1.UntypedDict, s conversion.Scope) error { *out = v3alpha1.UntypedDict(*in) return nil diff --git a/pkg/api/getambassador.io/v2/zz_generated.deepcopy.go b/pkg/api/getambassador.io/v2/zz_generated.deepcopy.go index bc55ecb8ec..2b3706b137 100644 --- a/pkg/api/getambassador.io/v2/zz_generated.deepcopy.go +++ b/pkg/api/getambassador.io/v2/zz_generated.deepcopy.go @@ -2621,6 +2621,13 @@ func (in *TracingServiceSpec) DeepCopyInto(out *TracingServiceSpec) { *out = new(TraceConfig) (*in).DeepCopyInto(*out) } + if in.V3CustomTags != nil { + in, out := &in.V3CustomTags, &out.V3CustomTags + *out = make([]v3alpha1.TracingCustomTag, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TracingServiceSpec. diff --git a/pkg/api/getambassador.io/v3alpha1/crd_tracingservice.go b/pkg/api/getambassador.io/v3alpha1/crd_tracingservice.go index 09389ee4b5..2af003f250 100644 --- a/pkg/api/getambassador.io/v3alpha1/crd_tracingservice.go +++ b/pkg/api/getambassador.io/v3alpha1/crd_tracingservice.go @@ -45,6 +45,40 @@ type TraceConfig struct { ServiceName string `json:"service_name,omitempty"` } +type TracingCustomTagTypeLiteral struct { + // +kubebuilder:validation:Required + Value string `json:"value"` +} + +type TracingCustomTagTypeEnvironment struct { + // +kubebuilder:validation:Required + Name string `json:"name"` + DefaultValue *string `json:"default_value,omitempty"` +} + +type TracingCustomTagTypeRequestHeader struct { + // +kubebuilder:validation:Required + Name string `json:"name"` + DefaultValue *string `json:"default_value,omitempty"` +} + +type TracingCustomTag struct { + // +kubebuilder:validation:Required + Tag string `json:"tag"` + + // There is no oneOf support in kubebuilder https://github.com/kubernetes-sigs/controller-tools/issues/461 + + // Literal explicitly specifies the protocol stack to set up. Exactly one of Literal, + // Environment or Header must be supplied. + Literal *TracingCustomTagTypeLiteral `json:"literal,omitempty"` + // Environment explicitly specifies the protocol stack to set up. Exactly one of Literal, + // Environment or Header must be supplied. + Environment *TracingCustomTagTypeEnvironment `json:"environment,omitempty"` + // Header explicitly specifies the protocol stack to set up. Exactly one of Literal, + // Environment or Header must be supplied. + Header *TracingCustomTagTypeRequestHeader `json:"request_header,omitempty"` +} + // TracingServiceSpec defines the desired state of TracingService type TracingServiceSpec struct { AmbassadorID AmbassadorID `json:"ambassador_id,omitempty"` @@ -53,11 +87,14 @@ type TracingServiceSpec struct { // +kubebuilder:validation:Required Driver string `json:"driver,omitempty"` // +kubebuilder:validation:Required - Service string `json:"service,omitempty"` - Sampling *TraceSampling `json:"sampling,omitempty"` - TagHeaders []string `json:"tag_headers,omitempty"` - Config *TraceConfig `json:"config,omitempty"` - StatsName string `json:"stats_name,omitempty"` + Service string `json:"service,omitempty"` + Sampling *TraceSampling `json:"sampling,omitempty"` + // tag_headers is deprecated. Use custom_tags instead. + // `tag_headers: ["header"]` can be defined as `custom_tags: [{"request_header": {"name": "header"}}]`. + DeprecatedTagHeaders []string `json:"tag_headers,omitempty"` + CustomTags []TracingCustomTag `json:"custom_tags,omitempty"` + Config *TraceConfig `json:"config,omitempty"` + StatsName string `json:"stats_name,omitempty"` } // TracingService is the Schema for the tracingservices API diff --git a/pkg/api/getambassador.io/v3alpha1/testdata/tracingsvc.yaml b/pkg/api/getambassador.io/v3alpha1/testdata/tracingsvc.yaml index 57bbcb00ed..e25658eb59 100644 --- a/pkg/api/getambassador.io/v3alpha1/testdata/tracingsvc.yaml +++ b/pkg/api/getambassador.io/v3alpha1/testdata/tracingsvc.yaml @@ -96,3 +96,9 @@ overall: 0 random: 0 service: "zipkin-65:9411" + tag_headers: + - hello + custom_tags: + - request_header: + name: hola + tag: hola diff --git a/pkg/api/getambassador.io/v3alpha1/zz_generated.deepcopy.go b/pkg/api/getambassador.io/v3alpha1/zz_generated.deepcopy.go index aec9d33d8f..7ed448f57b 100644 --- a/pkg/api/getambassador.io/v3alpha1/zz_generated.deepcopy.go +++ b/pkg/api/getambassador.io/v3alpha1/zz_generated.deepcopy.go @@ -2703,6 +2703,91 @@ func (in *TraceSampling) DeepCopy() *TraceSampling { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TracingCustomTag) DeepCopyInto(out *TracingCustomTag) { + *out = *in + if in.Literal != nil { + in, out := &in.Literal, &out.Literal + *out = new(TracingCustomTagTypeLiteral) + **out = **in + } + if in.Environment != nil { + in, out := &in.Environment, &out.Environment + *out = new(TracingCustomTagTypeEnvironment) + (*in).DeepCopyInto(*out) + } + if in.Header != nil { + in, out := &in.Header, &out.Header + *out = new(TracingCustomTagTypeRequestHeader) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TracingCustomTag. +func (in *TracingCustomTag) DeepCopy() *TracingCustomTag { + if in == nil { + return nil + } + out := new(TracingCustomTag) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TracingCustomTagTypeEnvironment) DeepCopyInto(out *TracingCustomTagTypeEnvironment) { + *out = *in + if in.DefaultValue != nil { + in, out := &in.DefaultValue, &out.DefaultValue + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TracingCustomTagTypeEnvironment. +func (in *TracingCustomTagTypeEnvironment) DeepCopy() *TracingCustomTagTypeEnvironment { + if in == nil { + return nil + } + out := new(TracingCustomTagTypeEnvironment) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TracingCustomTagTypeLiteral) DeepCopyInto(out *TracingCustomTagTypeLiteral) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TracingCustomTagTypeLiteral. +func (in *TracingCustomTagTypeLiteral) DeepCopy() *TracingCustomTagTypeLiteral { + if in == nil { + return nil + } + out := new(TracingCustomTagTypeLiteral) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TracingCustomTagTypeRequestHeader) DeepCopyInto(out *TracingCustomTagTypeRequestHeader) { + *out = *in + if in.DefaultValue != nil { + in, out := &in.DefaultValue, &out.DefaultValue + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TracingCustomTagTypeRequestHeader. +func (in *TracingCustomTagTypeRequestHeader) DeepCopy() *TracingCustomTagTypeRequestHeader { + if in == nil { + return nil + } + out := new(TracingCustomTagTypeRequestHeader) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TracingService) DeepCopyInto(out *TracingService) { *out = *in @@ -2774,11 +2859,18 @@ func (in *TracingServiceSpec) DeepCopyInto(out *TracingServiceSpec) { *out = new(TraceSampling) (*in).DeepCopyInto(*out) } - if in.TagHeaders != nil { - in, out := &in.TagHeaders, &out.TagHeaders + if in.DeprecatedTagHeaders != nil { + in, out := &in.DeprecatedTagHeaders, &out.DeprecatedTagHeaders *out = make([]string, len(*in)) copy(*out, *in) } + if in.CustomTags != nil { + in, out := &in.CustomTags, &out.CustomTags + *out = make([]TracingCustomTag, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Config != nil { in, out := &in.Config, &out.Config *out = new(TraceConfig) diff --git a/python/ambassador/envoy/v3/v3listener.py b/python/ambassador/envoy/v3/v3listener.py index b73f9cd859..6820799488 100644 --- a/python/ambassador/envoy/v3/v3listener.py +++ b/python/ambassador/envoy/v3/v3listener.py @@ -447,18 +447,21 @@ def base_http_config(self) -> Dict[str, Any]: base_http_config["tracing"] = {} self.traffic_direction = "OUTBOUND" + finalCustomTags = [] + custom_tags = self.config.ir.tracing.get('custom_tags', []) req_hdrs = self.config.ir.tracing.get('tag_headers', []) - - if req_hdrs: - base_http_config["tracing"]["custom_tags"] = [] - for hdr in req_hdrs: - custom_tag = { - "request_header": { - "name": hdr, + if custom_tags or req_hdrs: + finalCustomTags += custom_tags + if req_hdrs: + for hdr in req_hdrs: + custom_tag = { + "request_header": { + "name": hdr, }, - "tag": hdr, - } - base_http_config["tracing"]["custom_tags"].append(custom_tag) + "tag": hdr, + } + finalCustomTags.append(custom_tag) + base_http_config["tracing"]["custom_tags"] = finalCustomTags sampling = self.config.ir.tracing.get('sampling', {}) diff --git a/python/ambassador/ir/irtracing.py b/python/ambassador/ir/irtracing.py index 849ae4a182..84bbe3493d 100644 --- a/python/ambassador/ir/irtracing.py +++ b/python/ambassador/ir/irtracing.py @@ -15,6 +15,7 @@ class IRTracing(IRResource): driver: str driver_config: dict tag_headers: list + custom_tags: list host_rewrite: Optional[str] sampling: dict @@ -89,6 +90,7 @@ def setup(self, ir: 'IR', aconf: Config) -> bool: self.cluster = None self.driver_config = driver_config self.tag_headers = config.get('tag_headers', []) + self.custom_tags = config.get('custom_tags', []) self.sampling = config.get('sampling', {}) # XXX host_rewrite actually isn't in the schema right now. diff --git a/python/tests/integration/manifests/crds.yaml b/python/tests/integration/manifests/crds.yaml index 6855dfff8e..8856f54ba0 100644 --- a/python/tests/integration/manifests/crds.yaml +++ b/python/tests/integration/manifests/crds.yaml @@ -3636,6 +3636,49 @@ spec: items: type: string type: array + v3CustomTags: + items: + properties: + environment: + description: Environment explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + literal: + description: Literal explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + value: + type: string + required: + - value + type: object + request_header: + description: Header explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + tag: + type: string + required: + - tag + type: object + type: array v3StatsName: type: string required: @@ -3707,6 +3750,49 @@ spec: trace_id_128bit: type: boolean type: object + custom_tags: + items: + properties: + environment: + description: Environment explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + literal: + description: Literal explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + value: + type: string + required: + - value + type: object + request_header: + description: Header explicitly specifies the protocol stack + to set up. Exactly one of Literal, Environment or Header must + be supplied. + properties: + default_value: + type: string + name: + type: string + required: + - name + type: object + tag: + type: string + required: + - tag + type: object + type: array driver: enum: - lightstep @@ -3727,6 +3813,9 @@ spec: stats_name: type: string tag_headers: + description: 'tag_headers is deprecated. Use custom_tags instead. + `tag_headers: ["header"]` can be defined as `custom_tags: [{"request_header": + {"name": "header"}}]`.' items: type: string type: array diff --git a/python/tests/kat/t_tracing.py b/python/tests/kat/t_tracing.py index 5e5c99c32f..2dbd947ef4 100644 --- a/python/tests/kat/t_tracing.py +++ b/python/tests/kat/t_tracing.py @@ -84,6 +84,18 @@ def config(self) -> Generator[Union[str, Tuple[Node, str]], None, None]: driver: zipkin tag_headers: - "x-watsup" +custom_tags: + - tag: ltag + literal: + value: lvalue + - tag: etag + environment: + name: UNKNOWN_ENV_VAR + default_value: efallback + - tag: htag + request_header: + name: x-something + default_value: hfallback """) def requirements(self): @@ -94,7 +106,7 @@ def queries(self): # Speak through each Ambassador to the traced service... for i in range(100): - yield Query(self.url("target/"), headers={'x-watsup':'nothin'}, phase=1) + yield Query(self.url("target/"), headers={'x-watsup':'nothin','x-something':'something'}, phase=1) # ...then ask the Zipkin for services and spans. Including debug=True in these queries @@ -131,8 +143,12 @@ def check(self): assert len(traceId) == 32 for t in self.results[102].json[0]: if t.get('tags', {}).get('node_id') == 'test-id': - assert 'x-watsup' in t['tags'] - assert t['tags']['x-watsup'] == 'nothin' + assert 'ltag' in t['tags'] + assert t['tags']['ltag'] == 'lvalue' + assert 'etag' in t['tags'] + assert t['tags']['etag'] == 'efallback' + assert 'htag' in t['tags'] + assert t['tags']['htag'] == 'something' class TracingTestLongClusterName(AmbassadorTest): diff --git a/python/tests/unit/test_tracing.py b/python/tests/unit/test_tracing.py index 417530800f..6f8fb7792e 100644 --- a/python/tests/unit/test_tracing.py +++ b/python/tests/unit/test_tracing.py @@ -39,6 +39,18 @@ def lightstep_tracing_service_manifest(): spec: service: lightstep:80 driver: lightstep + custom_tags: + - tag: ltag + literal: + value: avalue + - tag: etag + environment: + name: UNKNOWN_ENV_VAR + default_value: efallback + - tag: htag + request_header: + name: x-does-not-exist + default_value: hfallback config: access_token_file: /lightstep-credentials/access-token propagation_modes: ["ENVOY", "TRACE_CONTEXT"] @@ -61,6 +73,15 @@ def test_tracing_config_v3(): econf = EnvoyConfig.generate(ir, "V3") + # check if custom_tags are added + assert econf.as_dict()['static_resources']['listeners'][0]['filter_chains'][0]['filters'][0]['typed_config']['tracing'] == { + "custom_tags": [ + {'literal': {'value': 'avalue'}, 'tag': 'ltag'}, + {'environment': {'default_value': 'efallback', 'name': 'UNKNOWN_ENV_VAR'}, 'tag': 'etag'}, + {'request_header': {'default_value': 'hfallback', 'name': 'x-does-not-exist'}, 'tag': 'htag'}, + ] + } + bootstrap_config, ads_config, _ = econf.split_config() assert "tracing" in bootstrap_config assert bootstrap_config["tracing"] == { @@ -114,4 +135,3 @@ def test_tracing_config_v2(): ads_config.pop('@type', None) assert_valid_envoy_config(ads_config, v2=True) assert_valid_envoy_config(bootstrap_config, v2=True) -