From 66c957e352e3ac213500acc9efd5fa809948f82f Mon Sep 17 00:00:00 2001 From: Paul Salaberria Date: Thu, 31 Mar 2022 11:53:36 +0200 Subject: [PATCH 1/2] Add propagation_modes for the Lightstep Tracer The Lightstep tracer supports multiple propagation modes. See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/trace/v3/lightstep.proto#enum-config-trace-v3-lightstepconfig-propagationmode With this PR it is now possible to set a list of propagation modes in the config field of the TracingService. Signed-off-by: Paul Salaberria --- CHANGELOG.md | 5 + docs/releaseNotes.yml | 10 ++ manifests/emissary/emissary-crds.yaml.in | 9 ++ pkg/api/getambassador.io/crds.yaml | 9 ++ .../v2/zz_generated.conversion.go | 72 +++++++++-- .../v3alpha1/crd_tracingservice.go | 14 ++- .../v3alpha1/zz_generated.deepcopy.go | 5 + python/tests/integration/manifests/crds.yaml | 9 ++ python/tests/unit/test_tracing.py | 117 ++++++++++++++++++ 9 files changed, 238 insertions(+), 12 deletions(-) create mode 100644 python/tests/unit/test_tracing.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b5952a72d3..01f1da7939 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,11 @@ Please see the [Envoy documentation](https://www.envoyproxy.io/docs/envoy/latest - Security: Completely remove gdbm, pip, smtplib, and sqlite packages, as they are unused. +- Feature: It is now possible to set `propagation_modes` in the `TracingService` config when using + lightstep as the driver. (Thanks to Paul!) ([#4179]) + +[#4179]: https://github.com/emissary-ingress/emissary/pull/4179 + ## [2.2.1] February 22, 2022 [2.2.1]: https://github.com/emissary-ingress/emissary/compare/v2.2.0...v2.2.1 diff --git a/docs/releaseNotes.yml b/docs/releaseNotes.yml index 8bc747f00e..26b1f7dead 100644 --- a/docs/releaseNotes.yml +++ b/docs/releaseNotes.yml @@ -39,6 +39,16 @@ items: type: security body: >- Completely remove gdbm, pip, smtplib, and sqlite packages, as they are unused. + - title: Allow setting propagation modes for Lightstep tracing + type: feature + body: >- + It is now possible to set propagation_modes in the + TracingService config when using lightstep as the driver. + (Thanks to Paul!) + github: + - title: "#4179" + link: https://github.com/emissary-ingress/emissary/pull/4179 + - version: 2.2.1 date: '2022-02-22' diff --git a/manifests/emissary/emissary-crds.yaml.in b/manifests/emissary/emissary-crds.yaml.in index 8b917b3bfc..4dd952717c 100644 --- a/manifests/emissary/emissary-crds.yaml.in +++ b/manifests/emissary/emissary-crds.yaml.in @@ -3773,6 +3773,15 @@ spec: type: string collector_hostname: type: string + propagation_modes: + items: + enum: + - ENVOY + - LIGHTSTEP + - B3 + - TRACE_CONTEXT + type: string + type: array service_name: type: string shared_span_context: diff --git a/pkg/api/getambassador.io/crds.yaml b/pkg/api/getambassador.io/crds.yaml index ceb7d739c5..67350ff643 100644 --- a/pkg/api/getambassador.io/crds.yaml +++ b/pkg/api/getambassador.io/crds.yaml @@ -3831,6 +3831,15 @@ spec: type: string collector_hostname: type: string + propagation_modes: + items: + enum: + - ENVOY + - LIGHTSTEP + - B3 + - TRACE_CONTEXT + type: string + type: array service_name: type: string shared_span_context: diff --git a/pkg/api/getambassador.io/v2/zz_generated.conversion.go b/pkg/api/getambassador.io/v2/zz_generated.conversion.go index 059e20fe5d..c42ad2385d 100644 --- a/pkg/api/getambassador.io/v2/zz_generated.conversion.go +++ b/pkg/api/getambassador.io/v2/zz_generated.conversion.go @@ -5345,7 +5345,38 @@ func Convert_v3alpha1_TLSContextSpec_To_v2_TLSContextSpec(in *v3alpha1.TLSContex } func autoConvert_v2_TraceConfig_To_v3alpha1_TraceConfig(in *TraceConfig, out *v3alpha1.TraceConfig, s conversion.Scope) error { - *out = v3alpha1.TraceConfig(*in) + if true { + in, out := &in.AccessTokenFile, &out.AccessTokenFile + *out = *in + } + if true { + in, out := &in.CollectorCluster, &out.CollectorCluster + *out = *in + } + if true { + in, out := &in.CollectorEndpoint, &out.CollectorEndpoint + *out = *in + } + if true { + in, out := &in.CollectorEndpointVersion, &out.CollectorEndpointVersion + *out = *in + } + if true { + in, out := &in.CollectorHostname, &out.CollectorHostname + *out = *in + } + if true { + in, out := &in.TraceID128Bit, &out.TraceID128Bit + *out = *in + } + if true { + in, out := &in.SharedSpanContext, &out.SharedSpanContext + *out = *in + } + if true { + in, out := &in.ServiceName, &out.ServiceName + *out = *in + } return nil } @@ -5355,15 +5386,42 @@ func Convert_v2_TraceConfig_To_v3alpha1_TraceConfig(in *TraceConfig, out *v3alph } func autoConvert_v3alpha1_TraceConfig_To_v2_TraceConfig(in *v3alpha1.TraceConfig, out *TraceConfig, s conversion.Scope) error { - *out = TraceConfig(*in) + if true { + in, out := &in.AccessTokenFile, &out.AccessTokenFile + *out = *in + } + if true { + in, out := &in.CollectorCluster, &out.CollectorCluster + *out = *in + } + if true { + in, out := &in.CollectorEndpoint, &out.CollectorEndpoint + *out = *in + } + if true { + in, out := &in.CollectorEndpointVersion, &out.CollectorEndpointVersion + *out = *in + } + if true { + in, out := &in.CollectorHostname, &out.CollectorHostname + *out = *in + } + // WARNING: in.PropagationModes requires manual conversion: does not exist in peer-type + if true { + in, out := &in.TraceID128Bit, &out.TraceID128Bit + *out = *in + } + if true { + in, out := &in.SharedSpanContext, &out.SharedSpanContext + *out = *in + } + if true { + in, out := &in.ServiceName, &out.ServiceName + *out = *in + } return nil } -// Convert_v3alpha1_TraceConfig_To_v2_TraceConfig is an autogenerated conversion function. -func Convert_v3alpha1_TraceConfig_To_v2_TraceConfig(in *v3alpha1.TraceConfig, out *TraceConfig, s conversion.Scope) error { - return autoConvert_v3alpha1_TraceConfig_To_v2_TraceConfig(in, out, s) -} - func autoConvert_v2_TraceSampling_To_v3alpha1_TraceSampling(in *TraceSampling, out *v3alpha1.TraceSampling, s conversion.Scope) error { *out = v3alpha1.TraceSampling(*in) return nil diff --git a/pkg/api/getambassador.io/v3alpha1/crd_tracingservice.go b/pkg/api/getambassador.io/v3alpha1/crd_tracingservice.go index 09cc9dbcdd..09389ee4b5 100644 --- a/pkg/api/getambassador.io/v3alpha1/crd_tracingservice.go +++ b/pkg/api/getambassador.io/v3alpha1/crd_tracingservice.go @@ -29,16 +29,20 @@ type TraceSampling struct { Overall *int `json:"overall,omitempty"` } +// +kubebuilder:validation:Enum=ENVOY;LIGHTSTEP;B3;TRACE_CONTEXT +type PropagationMode string + type TraceConfig struct { AccessTokenFile string `json:"access_token_file,omitempty"` CollectorCluster string `json:"collector_cluster,omitempty"` CollectorEndpoint string `json:"collector_endpoint,omitempty"` // +kubebuilder:validation:Enum=HTTP_JSON_V1;HTTP_JSON;HTTP_PROTO - CollectorEndpointVersion string `json:"collector_endpoint_version,omitempty"` - CollectorHostname string `json:"collector_hostname,omitempty"` - TraceID128Bit *bool `json:"trace_id_128bit,omitempty"` - SharedSpanContext *bool `json:"shared_span_context,omitempty"` - ServiceName string `json:"service_name,omitempty"` + CollectorEndpointVersion string `json:"collector_endpoint_version,omitempty"` + CollectorHostname string `json:"collector_hostname,omitempty"` + PropagationModes []PropagationMode `json:"propagation_modes,omitempty"` + TraceID128Bit *bool `json:"trace_id_128bit,omitempty"` + SharedSpanContext *bool `json:"shared_span_context,omitempty"` + ServiceName string `json:"service_name,omitempty"` } // TracingServiceSpec defines the desired state of TracingService diff --git a/pkg/api/getambassador.io/v3alpha1/zz_generated.deepcopy.go b/pkg/api/getambassador.io/v3alpha1/zz_generated.deepcopy.go index 62c4b46764..aec9d33d8f 100644 --- a/pkg/api/getambassador.io/v3alpha1/zz_generated.deepcopy.go +++ b/pkg/api/getambassador.io/v3alpha1/zz_generated.deepcopy.go @@ -2646,6 +2646,11 @@ func (in *TLSContextSpec) DeepCopy() *TLSContextSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TraceConfig) DeepCopyInto(out *TraceConfig) { *out = *in + if in.PropagationModes != nil { + in, out := &in.PropagationModes, &out.PropagationModes + *out = make([]PropagationMode, len(*in)) + copy(*out, *in) + } if in.TraceID128Bit != nil { in, out := &in.TraceID128Bit, &out.TraceID128Bit *out = new(bool) diff --git a/python/tests/integration/manifests/crds.yaml b/python/tests/integration/manifests/crds.yaml index 121df311d5..92549653a0 100644 --- a/python/tests/integration/manifests/crds.yaml +++ b/python/tests/integration/manifests/crds.yaml @@ -3682,6 +3682,15 @@ spec: type: string collector_hostname: type: string + propagation_modes: + items: + enum: + - ENVOY + - LIGHTSTEP + - B3 + - TRACE_CONTEXT + type: string + type: array service_name: type: string shared_span_context: diff --git a/python/tests/unit/test_tracing.py b/python/tests/unit/test_tracing.py new file mode 100644 index 0000000000..417530800f --- /dev/null +++ b/python/tests/unit/test_tracing.py @@ -0,0 +1,117 @@ +from typing import Optional, TYPE_CHECKING + +import logging +import pytest + +from tests.selfsigned import TLSCerts +from tests.utils import assert_valid_envoy_config, module_and_mapping_manifests + +logging.basicConfig( + level=logging.INFO, + format="%(asctime)s test %(levelname)s: %(message)s", + datefmt='%Y-%m-%d %H:%M:%S' +) + +logger = logging.getLogger("ambassador") + +from ambassador import Config, IR +from ambassador.envoy import EnvoyConfig +from ambassador.fetch import ResourceFetcher +from ambassador.utils import SecretHandler, SecretInfo + +if TYPE_CHECKING: + from ambassador.ir.irresource import IRResource # pragma: no cover + +class MockSecretHandler(SecretHandler): + def load_secret(self, resource: 'IRResource', secret_name: str, namespace: str) -> Optional[SecretInfo]: + return SecretInfo('fallback-self-signed-cert', 'ambassador', "mocked-fallback-secret", + TLSCerts["acook"].pubcert, TLSCerts["acook"].privkey, decode_b64=False) + + +def lightstep_tracing_service_manifest(): + return """ +--- +apiVersion: getambassador.io/v3alpha1 +kind: TracingService +metadata: + name: tracing + namespace: ambassador +spec: + service: lightstep:80 + driver: lightstep + config: + access_token_file: /lightstep-credentials/access-token + propagation_modes: ["ENVOY", "TRACE_CONTEXT"] +""" + +@pytest.mark.compilertest +def test_tracing_config_v3(): + aconf = Config() + + yaml = module_and_mapping_manifests(None, []) + "\n" + lightstep_tracing_service_manifest() + fetcher = ResourceFetcher(logger, aconf) + fetcher.parse_yaml(yaml, k8s=True) + + aconf.load_all(fetcher.sorted()) + + secret_handler = MockSecretHandler(logger, "mockery", "/tmp/ambassador/snapshots", "v1") + ir = IR(aconf, file_checker=lambda path: True, secret_handler=secret_handler) + + assert ir + + econf = EnvoyConfig.generate(ir, "V3") + + bootstrap_config, ads_config, _ = econf.split_config() + assert "tracing" in bootstrap_config + assert bootstrap_config["tracing"] == { + "http": { + "name": "envoy.lightstep", + "typed_config": { + "@type": "type.googleapis.com/envoy.config.trace.v3.LightstepConfig", + "access_token_file": "/lightstep-credentials/access-token", + "collector_cluster": "cluster_tracing_lightstep_80_ambassador", + "propagation_modes": ["ENVOY", "TRACE_CONTEXT"] + } + } + } + + ads_config.pop('@type', None) + assert_valid_envoy_config(ads_config) + assert_valid_envoy_config(bootstrap_config) + + +@pytest.mark.compilertest +def test_tracing_config_v2(): + aconf = Config() + + yaml = module_and_mapping_manifests(None, []) + "\n" + lightstep_tracing_service_manifest() + fetcher = ResourceFetcher(logger, aconf) + fetcher.parse_yaml(yaml, k8s=True) + + aconf.load_all(fetcher.sorted()) + + secret_handler = MockSecretHandler(logger, "mockery", "/tmp/ambassador/snapshots", "v1") + ir = IR(aconf, file_checker=lambda path: True, secret_handler=secret_handler) + + assert ir + + econf = EnvoyConfig.generate(ir, "V2") + + bootstrap_config, ads_config, _ = econf.split_config() + assert "tracing" in bootstrap_config + assert bootstrap_config["tracing"] == { + "http": { + "name": "envoy.lightstep", + "typed_config": { + "@type": "type.googleapis.com/envoy.config.trace.v2.LightstepConfig", + "access_token_file": "/lightstep-credentials/access-token", + "collector_cluster": "cluster_tracing_lightstep_80_ambassador", + "propagation_modes": ["ENVOY", "TRACE_CONTEXT"] + } + } + } + + ads_config.pop('@type', None) + assert_valid_envoy_config(ads_config, v2=True) + assert_valid_envoy_config(bootstrap_config, v2=True) + From f36a9c30a4f2e4dd69694f6faf7354c8699318d0 Mon Sep 17 00:00:00 2001 From: Paul Salaberria Date: Fri, 15 Apr 2022 17:13:58 +0200 Subject: [PATCH 2/2] Add field in v2 for backwards compatibility Signed-off-by: Paul Salaberria --- manifests/emissary/emissary-crds.yaml.in | 9 +++++++++ pkg/api/getambassador.io/crds.yaml | 9 +++++++++ pkg/api/getambassador.io/v2/crd_tracingservice.go | 4 ++++ .../getambassador.io/v2/zz_generated.conversion.go | 14 +++++++++++++- .../getambassador.io/v2/zz_generated.deepcopy.go | 6 ++++++ python/tests/integration/manifests/crds.yaml | 9 +++++++++ 6 files changed, 50 insertions(+), 1 deletion(-) diff --git a/manifests/emissary/emissary-crds.yaml.in b/manifests/emissary/emissary-crds.yaml.in index 4dd952717c..7697f43118 100644 --- a/manifests/emissary/emissary-crds.yaml.in +++ b/manifests/emissary/emissary-crds.yaml.in @@ -3696,6 +3696,15 @@ spec: type: boolean trace_id_128bit: type: boolean + v3PropagationModes: + items: + enum: + - ENVOY + - LIGHTSTEP + - B3 + - TRACE_CONTEXT + type: string + type: array type: object driver: enum: diff --git a/pkg/api/getambassador.io/crds.yaml b/pkg/api/getambassador.io/crds.yaml index 67350ff643..c6e06981d9 100644 --- a/pkg/api/getambassador.io/crds.yaml +++ b/pkg/api/getambassador.io/crds.yaml @@ -3755,6 +3755,15 @@ spec: type: boolean trace_id_128bit: type: boolean + v3PropagationModes: + items: + enum: + - ENVOY + - LIGHTSTEP + - B3 + - TRACE_CONTEXT + type: string + type: array type: object driver: enum: diff --git a/pkg/api/getambassador.io/v2/crd_tracingservice.go b/pkg/api/getambassador.io/v2/crd_tracingservice.go index ebda6862dc..c26d6f1eeb 100644 --- a/pkg/api/getambassador.io/v2/crd_tracingservice.go +++ b/pkg/api/getambassador.io/v2/crd_tracingservice.go @@ -20,6 +20,7 @@ package v2 import ( + "github.com/datawire/ambassador/v2/pkg/api/getambassador.io/v3alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -39,6 +40,9 @@ type TraceConfig struct { TraceID128Bit *bool `json:"trace_id_128bit,omitempty"` SharedSpanContext *bool `json:"shared_span_context,omitempty"` ServiceName string `json:"service_name,omitempty"` + + // +k8s:conversion-gen:rename=PropagationModes + V3PropagationModes []v3alpha1.PropagationMode `json:"v3PropagationModes,omitempty"` } // TracingServiceSpec defines the desired state of TracingService diff --git a/pkg/api/getambassador.io/v2/zz_generated.conversion.go b/pkg/api/getambassador.io/v2/zz_generated.conversion.go index c42ad2385d..0a629a6881 100644 --- a/pkg/api/getambassador.io/v2/zz_generated.conversion.go +++ b/pkg/api/getambassador.io/v2/zz_generated.conversion.go @@ -5377,6 +5377,10 @@ func autoConvert_v2_TraceConfig_To_v3alpha1_TraceConfig(in *TraceConfig, out *v3 in, out := &in.ServiceName, &out.ServiceName *out = *in } + if true { + in, out := &in.V3PropagationModes, &out.PropagationModes + *out = *in + } return nil } @@ -5406,7 +5410,10 @@ func autoConvert_v3alpha1_TraceConfig_To_v2_TraceConfig(in *v3alpha1.TraceConfig in, out := &in.CollectorHostname, &out.CollectorHostname *out = *in } - // WARNING: in.PropagationModes requires manual conversion: does not exist in peer-type + if true { + in, out := &in.PropagationModes, &out.V3PropagationModes + *out = *in + } if true { in, out := &in.TraceID128Bit, &out.TraceID128Bit *out = *in @@ -5422,6 +5429,11 @@ func autoConvert_v3alpha1_TraceConfig_To_v2_TraceConfig(in *v3alpha1.TraceConfig return nil } +// Convert_v3alpha1_TraceConfig_To_v2_TraceConfig is an autogenerated conversion function. +func Convert_v3alpha1_TraceConfig_To_v2_TraceConfig(in *v3alpha1.TraceConfig, out *TraceConfig, s conversion.Scope) error { + return autoConvert_v3alpha1_TraceConfig_To_v2_TraceConfig(in, out, s) +} + func autoConvert_v2_TraceSampling_To_v3alpha1_TraceSampling(in *TraceSampling, out *v3alpha1.TraceSampling, s conversion.Scope) error { *out = v3alpha1.TraceSampling(*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 ad7154bc6c..bc55ecb8ec 100644 --- a/pkg/api/getambassador.io/v2/zz_generated.deepcopy.go +++ b/pkg/api/getambassador.io/v2/zz_generated.deepcopy.go @@ -21,6 +21,7 @@ package v2 import ( "encoding/json" + "github.com/datawire/ambassador/v2/pkg/api/getambassador.io/v3alpha1" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -2492,6 +2493,11 @@ func (in *TraceConfig) DeepCopyInto(out *TraceConfig) { *out = new(bool) **out = **in } + if in.V3PropagationModes != nil { + in, out := &in.V3PropagationModes, &out.V3PropagationModes + *out = make([]v3alpha1.PropagationMode, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TraceConfig. diff --git a/python/tests/integration/manifests/crds.yaml b/python/tests/integration/manifests/crds.yaml index 92549653a0..6855dfff8e 100644 --- a/python/tests/integration/manifests/crds.yaml +++ b/python/tests/integration/manifests/crds.yaml @@ -3605,6 +3605,15 @@ spec: type: boolean trace_id_128bit: type: boolean + v3PropagationModes: + items: + enum: + - ENVOY + - LIGHTSTEP + - B3 + - TRACE_CONTEXT + type: string + type: array type: object driver: enum: