From 190fbd0f457e555000e0df51cd961c17c2bc763f Mon Sep 17 00:00:00 2001 From: Andreas Gerstmayr Date: Fri, 24 Jan 2025 17:15:06 +0100 Subject: [PATCH] [BREAKING CHANGE] Switch protocol for internal traces from Jaeger to OTLP/HTTP Exporting internal traces (traces generated by Tempo and the gateway) via the Jaeger/Thrift protocol and the `.spec.observability.tracing.jaeger_agent_endpoint` setting is deprecated. Please migrate to the OTLP/HTTP protocol and the new `.spec.observability.tracing.otlp_http_endpoint` setting. Signed-off-by: Andreas Gerstmayr --- .chloggen/internal_tracing_protocol.yaml | 18 ++++++++ api/tempo/v1alpha1/tempostack_types.go | 10 ++++- .../tempo-operator.clusterserviceversion.yaml | 8 ++-- .../tempo.grafana.com_tempostacks.yaml | 6 +-- .../tempo-operator.clusterserviceversion.yaml | 8 ++-- .../tempo.grafana.com_tempostacks.yaml | 6 +-- .../bases/tempo.grafana.com_tempostacks.yaml | 7 ++- .../tempo-operator.clusterserviceversion.yaml | 6 +-- .../tempo-operator.clusterserviceversion.yaml | 6 +-- docs/spec/tempo.grafana.com_tempostacks.yaml | 2 +- internal/manifests/gateway/gateway.go | 7 ++- internal/manifests/gateway/gateway_test.go | 21 ++++----- internal/manifests/manifestutils/tracing.go | 21 ++++----- .../manifests/manifestutils/tracing_test.go | 32 +++++++------ internal/webhooks/tempostack_webhook.go | 45 ++++++++++--------- internal/webhooks/tempostack_webhook_test.go | 20 +++++---- 16 files changed, 127 insertions(+), 96 deletions(-) create mode 100755 .chloggen/internal_tracing_protocol.yaml diff --git a/.chloggen/internal_tracing_protocol.yaml b/.chloggen/internal_tracing_protocol.yaml new file mode 100755 index 000000000..e063c1095 --- /dev/null +++ b/.chloggen/internal_tracing_protocol.yaml @@ -0,0 +1,18 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. tempostack, tempomonolithic, github action) +component: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Switch protocol for internal traces from Jaeger to OTLP/HTTP + +# One or more tracking issues related to the change +issues: [] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + Exporting internal traces (traces generated by Tempo and the gateway) via the Jaeger/Thrift protocol and the `.spec.observability.tracing.jaeger_agent_endpoint` setting is deprecated. + Please migrate to the OTLP/HTTP protocol and the new `.spec.observability.tracing.otlp_http_endpoint` setting. diff --git a/api/tempo/v1alpha1/tempostack_types.go b/api/tempo/v1alpha1/tempostack_types.go index 7270e9b87..6a5424de9 100644 --- a/api/tempo/v1alpha1/tempostack_types.go +++ b/api/tempo/v1alpha1/tempostack_types.go @@ -183,7 +183,15 @@ type TracingConfigSpec struct { // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Sampling Fraction" SamplingFraction string `json:"sampling_fraction,omitempty"` - // JaegerAgentEndpoint defines the jaeger endpoint data gets send to. + // OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. + // + // +optional + // +kubebuilder:validation:Optional + // +kubebuilder:default:="localhost:4318" + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="OTLP/HTTP Endpoint" + OTLPHTTPEndpoint string `json:"otlp_http_endpoint,omitempty"` + + // DEPRECATED. JaegerAgentEndpoint defines the jaeger endpoint data gets send to. // // +optional // +kubebuilder:validation:Optional diff --git a/bundle/community/manifests/tempo-operator.clusterserviceversion.yaml b/bundle/community/manifests/tempo-operator.clusterserviceversion.yaml index 35fe907e4..6ab4e372b 100644 --- a/bundle/community/manifests/tempo-operator.clusterserviceversion.yaml +++ b/bundle/community/manifests/tempo-operator.clusterserviceversion.yaml @@ -74,7 +74,7 @@ metadata: capabilities: Deep Insights categories: Logging & Tracing,Monitoring containerImage: ghcr.io/grafana/tempo-operator/tempo-operator:v0.14.2 - createdAt: "2025-01-16T09:58:41Z" + createdAt: "2025-01-24T15:21:14Z" description: Create and manage deployments of Tempo, a high-scale distributed tracing backend. operatorframework.io/cluster-monitoring: "true" @@ -757,10 +757,10 @@ spec: - description: Tracing defines a config for operands. displayName: Tracing Config path: observability.tracing - - description: JaegerAgentEndpoint defines the jaeger endpoint data gets send + - description: OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. - displayName: Jaeger-Agent-Endpoint - path: observability.tracing.jaeger_agent_endpoint + displayName: OTLP/HTTP Endpoint + path: observability.tracing.otlp_http_endpoint - description: SamplingFraction defines the sampling ratio. Valid values are 0 to 1. displayName: Sampling Fraction diff --git a/bundle/community/manifests/tempo.grafana.com_tempostacks.yaml b/bundle/community/manifests/tempo.grafana.com_tempostacks.yaml index 0f779524b..fda158891 100644 --- a/bundle/community/manifests/tempo.grafana.com_tempostacks.yaml +++ b/bundle/community/manifests/tempo.grafana.com_tempostacks.yaml @@ -291,9 +291,9 @@ spec: tracing: description: Tracing defines a config for operands. properties: - jaeger_agent_endpoint: - default: localhost:6831 - description: JaegerAgentEndpoint defines the jaeger endpoint + otlp_http_endpoint: + default: localhost:4318 + description: OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. type: string sampling_fraction: diff --git a/bundle/openshift/manifests/tempo-operator.clusterserviceversion.yaml b/bundle/openshift/manifests/tempo-operator.clusterserviceversion.yaml index 3d4945346..f2b7dd938 100644 --- a/bundle/openshift/manifests/tempo-operator.clusterserviceversion.yaml +++ b/bundle/openshift/manifests/tempo-operator.clusterserviceversion.yaml @@ -74,7 +74,7 @@ metadata: capabilities: Deep Insights categories: Logging & Tracing,Monitoring containerImage: ghcr.io/grafana/tempo-operator/tempo-operator:v0.14.2 - createdAt: "2025-01-16T09:58:39Z" + createdAt: "2025-01-24T15:21:13Z" description: Create and manage deployments of Tempo, a high-scale distributed tracing backend. operatorframework.io/cluster-monitoring: "true" @@ -757,10 +757,10 @@ spec: - description: Tracing defines a config for operands. displayName: Tracing Config path: observability.tracing - - description: JaegerAgentEndpoint defines the jaeger endpoint data gets send + - description: OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. - displayName: Jaeger-Agent-Endpoint - path: observability.tracing.jaeger_agent_endpoint + displayName: OTLP/HTTP Endpoint + path: observability.tracing.otlp_http_endpoint - description: SamplingFraction defines the sampling ratio. Valid values are 0 to 1. displayName: Sampling Fraction diff --git a/bundle/openshift/manifests/tempo.grafana.com_tempostacks.yaml b/bundle/openshift/manifests/tempo.grafana.com_tempostacks.yaml index 0f779524b..fda158891 100644 --- a/bundle/openshift/manifests/tempo.grafana.com_tempostacks.yaml +++ b/bundle/openshift/manifests/tempo.grafana.com_tempostacks.yaml @@ -291,9 +291,9 @@ spec: tracing: description: Tracing defines a config for operands. properties: - jaeger_agent_endpoint: - default: localhost:6831 - description: JaegerAgentEndpoint defines the jaeger endpoint + otlp_http_endpoint: + default: localhost:4318 + description: OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. type: string sampling_fraction: diff --git a/config/crd/bases/tempo.grafana.com_tempostacks.yaml b/config/crd/bases/tempo.grafana.com_tempostacks.yaml index 8401b4e1f..cb286e178 100644 --- a/config/crd/bases/tempo.grafana.com_tempostacks.yaml +++ b/config/crd/bases/tempo.grafana.com_tempostacks.yaml @@ -289,7 +289,12 @@ spec: properties: jaeger_agent_endpoint: default: localhost:6831 - description: JaegerAgentEndpoint defines the jaeger endpoint + description: DEPRECATED. JaegerAgentEndpoint defines the jaeger + endpoint data gets send to. + type: string + otlp_http_endpoint: + default: localhost:4318 + description: OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. type: string sampling_fraction: diff --git a/config/manifests/community/bases/tempo-operator.clusterserviceversion.yaml b/config/manifests/community/bases/tempo-operator.clusterserviceversion.yaml index 9bfe350e5..a55c48ca1 100644 --- a/config/manifests/community/bases/tempo-operator.clusterserviceversion.yaml +++ b/config/manifests/community/bases/tempo-operator.clusterserviceversion.yaml @@ -686,10 +686,10 @@ spec: - description: Tracing defines a config for operands. displayName: Tracing Config path: observability.tracing - - description: JaegerAgentEndpoint defines the jaeger endpoint data gets send + - description: OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. - displayName: Jaeger-Agent-Endpoint - path: observability.tracing.jaeger_agent_endpoint + displayName: OTLP/HTTP Endpoint + path: observability.tracing.otlp_http_endpoint - description: SamplingFraction defines the sampling ratio. Valid values are 0 to 1. displayName: Sampling Fraction diff --git a/config/manifests/openshift/bases/tempo-operator.clusterserviceversion.yaml b/config/manifests/openshift/bases/tempo-operator.clusterserviceversion.yaml index 4ad774573..35050f794 100644 --- a/config/manifests/openshift/bases/tempo-operator.clusterserviceversion.yaml +++ b/config/manifests/openshift/bases/tempo-operator.clusterserviceversion.yaml @@ -686,10 +686,10 @@ spec: - description: Tracing defines a config for operands. displayName: Tracing Config path: observability.tracing - - description: JaegerAgentEndpoint defines the jaeger endpoint data gets send + - description: OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. - displayName: Jaeger-Agent-Endpoint - path: observability.tracing.jaeger_agent_endpoint + displayName: OTLP/HTTP Endpoint + path: observability.tracing.otlp_http_endpoint - description: SamplingFraction defines the sampling ratio. Valid values are 0 to 1. displayName: Sampling Fraction diff --git a/docs/spec/tempo.grafana.com_tempostacks.yaml b/docs/spec/tempo.grafana.com_tempostacks.yaml index 4c68246b2..a7d5f06a7 100644 --- a/docs/spec/tempo.grafana.com_tempostacks.yaml +++ b/docs/spec/tempo.grafana.com_tempostacks.yaml @@ -53,7 +53,7 @@ spec: # TempoStackSpec defines the desired st createPrometheusRules: false # CreatePrometheusRules specifies if Prometheus rules for alerts should be created for Tempo components. createServiceMonitors: false # CreateServiceMonitors specifies if ServiceMonitors should be created for Tempo components. tracing: # Tracing defines a config for operands. - jaeger_agent_endpoint: "localhost:6831" # JaegerAgentEndpoint defines the jaeger endpoint data gets send to. + otlp_http_endpoint: "localhost:4318" # OTLPHTTPEndpoint defines the OTLP/HTTP endpoint data gets send to. sampling_fraction: "" # SamplingFraction defines the sampling ratio. Valid values are 0 to 1. replicationFactor: 0 # The replication factor is a configuration setting that determines how many ingesters need to acknowledge the data from the distributors before accepting a span. retention: # Retention period defined by dataset. User can specify how long data should be stored. diff --git a/internal/manifests/gateway/gateway.go b/internal/manifests/gateway/gateway.go index c79213cda..e12660cdd 100644 --- a/internal/manifests/gateway/gateway.go +++ b/internal/manifests/gateway/gateway.go @@ -4,7 +4,7 @@ import ( "crypto/sha256" "encoding/hex" "fmt" - "net" + "net/url" "path" "github.com/imdario/mergo" @@ -362,15 +362,14 @@ func patchTracing(tempo v1alpha1.TempoStack, pod corev1.PodTemplateSpec) (corev1 return pod, nil } - host, port, err := net.SplitHostPort(tempo.Spec.Observability.Tracing.JaegerAgentEndpoint) + _, err := url.ParseRequestURI(tempo.Spec.Observability.Tracing.OTLPHTTPEndpoint) if err != nil { return corev1.PodTemplateSpec{}, err } container := corev1.Container{ Args: []string{ - fmt.Sprintf("--internal.tracing.endpoint=%s:%s", host, port), - "--internal.tracing.endpoint-type=agent", + fmt.Sprintf("--internal.tracing.otlp-http-endpoint=%s", tempo.Spec.Observability.Tracing.OTLPHTTPEndpoint), fmt.Sprintf("--internal.tracing.sampling-fraction=%s", tempo.Spec.Observability.Tracing.SamplingFraction), }, } diff --git a/internal/manifests/gateway/gateway_test.go b/internal/manifests/gateway/gateway_test.go index 722c59a6c..b37e634d7 100644 --- a/internal/manifests/gateway/gateway_test.go +++ b/internal/manifests/gateway/gateway_test.go @@ -1,8 +1,9 @@ package gateway import ( + "errors" "fmt" - "net" + "net/url" "reflect" "testing" @@ -283,8 +284,8 @@ func TestPatchTracing(t *testing.T) { Spec: v1alpha1.TempoStackSpec{ Observability: v1alpha1.ObservabilitySpec{ Tracing: v1alpha1.TracingConfigSpec{ - SamplingFraction: "1.0", - JaegerAgentEndpoint: "agent:1234", + SamplingFraction: "1.0", + OTLPHTTPEndpoint: "http://collector:4318", }, }, }, @@ -319,8 +320,7 @@ func TestPatchTracing(t *testing.T) { Name: containerNameTempoGateway, Args: []string{ "--abc", - "--internal.tracing.endpoint=agent:1234", - "--internal.tracing.endpoint-type=agent", + "--internal.tracing.otlp-http-endpoint=http://collector:4318", "--internal.tracing.sampling-fraction=1.0", }, }, @@ -374,17 +374,18 @@ func TestPatchTracing(t *testing.T) { Spec: v1alpha1.TempoStackSpec{ Observability: v1alpha1.ObservabilitySpec{ Tracing: v1alpha1.TracingConfigSpec{ - SamplingFraction: "0.5", - JaegerAgentEndpoint: "---invalid----", + SamplingFraction: "0.5", + OTLPHTTPEndpoint: "---invalid----", }, }, }, }, inputPod: corev1.PodTemplateSpec{}, expectPod: corev1.PodTemplateSpec{}, - expectErr: &net.AddrError{ - Addr: "---invalid----", - Err: "missing port in address", + expectErr: &url.Error{ + Op: "parse", + URL: "---invalid----", + Err: errors.New("invalid URI for request"), }, }, } diff --git a/internal/manifests/manifestutils/tracing.go b/internal/manifests/manifestutils/tracing.go index a89688a86..18341e934 100644 --- a/internal/manifests/manifestutils/tracing.go +++ b/internal/manifests/manifestutils/tracing.go @@ -1,7 +1,7 @@ package manifestutils import ( - "net" + "net/url" "github.com/imdario/mergo" corev1 "k8s.io/api/core/v1" @@ -9,13 +9,14 @@ import ( "github.com/grafana/tempo-operator/api/tempo/v1alpha1" ) -// PatchTracingJaegerEnv adds configures jaeger-sdk via environment variables if +// PatchTracingJaegerEnv adds configures OTEL SDK via environment variables if // operand observability settings exist. func PatchTracingJaegerEnv(tempo v1alpha1.TempoStack, pod corev1.PodTemplateSpec) (corev1.PodTemplateSpec, error) { if tempo.Spec.Observability.Tracing.SamplingFraction == "" { return pod, nil } - host, port, err := net.SplitHostPort(tempo.Spec.Observability.Tracing.JaegerAgentEndpoint) + + _, err := url.ParseRequestURI(tempo.Spec.Observability.Tracing.OTLPHTTPEndpoint) if err != nil { return corev1.PodTemplateSpec{}, err } @@ -23,19 +24,15 @@ func PatchTracingJaegerEnv(tempo v1alpha1.TempoStack, pod corev1.PodTemplateSpec container := corev1.Container{ Env: []corev1.EnvVar{ { - Name: "JAEGER_AGENT_HOST", - Value: host, - }, - { - Name: "JAEGER_AGENT_PORT", - Value: port, + Name: "OTEL_EXPORTER_OTLP_ENDPOINT", + Value: tempo.Spec.Observability.Tracing.OTLPHTTPEndpoint, }, { - Name: "JAEGER_SAMPLER_TYPE", - Value: "const", + Name: "OTEL_TRACES_SAMPLER", + Value: "parentbased_traceidratio", }, { - Name: "JAEGER_SAMPLER_PARAM", + Name: "OTEL_TRACES_SAMPLER_ARG", Value: tempo.Spec.Observability.Tracing.SamplingFraction, }, }, diff --git a/internal/manifests/manifestutils/tracing_test.go b/internal/manifests/manifestutils/tracing_test.go index 15024e118..715898cce 100644 --- a/internal/manifests/manifestutils/tracing_test.go +++ b/internal/manifests/manifestutils/tracing_test.go @@ -1,7 +1,8 @@ package manifestutils import ( - "net" + "errors" + "net/url" "testing" "github.com/stretchr/testify/assert" @@ -26,8 +27,8 @@ func Test_PatchTracingJaegerEnv(t *testing.T) { Spec: v1alpha1.TempoStackSpec{ Observability: v1alpha1.ObservabilitySpec{ Tracing: v1alpha1.TracingConfigSpec{ - SamplingFraction: "1.0", - JaegerAgentEndpoint: "agent:1234", + SamplingFraction: "1.0", + OTLPHTTPEndpoint: "http://collector:4318", }, }, }, @@ -69,19 +70,15 @@ func Test_PatchTracingJaegerEnv(t *testing.T) { Value: "1234", }, { - Name: "JAEGER_AGENT_HOST", - Value: "agent", + Name: "OTEL_EXPORTER_OTLP_ENDPOINT", + Value: "http://collector:4318", }, { - Name: "JAEGER_AGENT_PORT", - Value: "1234", - }, - { - Name: "JAEGER_SAMPLER_TYPE", - Value: "const", + Name: "OTEL_TRACES_SAMPLER", + Value: "parentbased_traceidratio", }, { - Name: "JAEGER_SAMPLER_PARAM", + Name: "OTEL_TRACES_SAMPLER_ARG", Value: "1.0", }, }, @@ -148,17 +145,18 @@ func Test_PatchTracingJaegerEnv(t *testing.T) { Spec: v1alpha1.TempoStackSpec{ Observability: v1alpha1.ObservabilitySpec{ Tracing: v1alpha1.TracingConfigSpec{ - SamplingFraction: "0.5", - JaegerAgentEndpoint: "---invalid----", + SamplingFraction: "0.5", + OTLPHTTPEndpoint: "---invalid----", }, }, }, }, inputPod: corev1.PodTemplateSpec{}, expectPod: corev1.PodTemplateSpec{}, - expectErr: &net.AddrError{ - Addr: "---invalid----", - Err: "missing port in address", + expectErr: &url.Error{ + Op: "parse", + URL: "---invalid----", + Err: errors.New("invalid URI for request"), }, }, } diff --git a/internal/webhooks/tempostack_webhook.go b/internal/webhooks/tempostack_webhook.go index 12970ec8c..3e9b467ef 100644 --- a/internal/webhooks/tempostack_webhook.go +++ b/internal/webhooks/tempostack_webhook.go @@ -4,7 +4,7 @@ import ( "context" "fmt" "math" - "net" + "net/url" "strconv" "strings" "time" @@ -319,26 +319,26 @@ func (v *validator) validateGateway(tempo v1alpha1.TempoStack) field.ErrorList { return nil } -func (v *validator) validateObservability(tempo v1alpha1.TempoStack) field.ErrorList { +func (v *validator) validateObservability(tempo v1alpha1.TempoStack) (admission.Warnings, field.ErrorList) { observabilityBase := field.NewPath("spec").Child("observability") metricsBase := observabilityBase.Child("metrics") if tempo.Spec.Observability.Metrics.CreateServiceMonitors && !v.ctrlConfig.Gates.PrometheusOperator { - return field.ErrorList{ + return nil, field.ErrorList{ field.Invalid(metricsBase.Child("createServiceMonitors"), tempo.Spec.Observability.Metrics.CreateServiceMonitors, "the prometheusOperator feature gate must be enabled to create ServiceMonitors for Tempo components", )} } if tempo.Spec.Observability.Metrics.CreatePrometheusRules && !v.ctrlConfig.Gates.PrometheusOperator { - return field.ErrorList{ + return nil, field.ErrorList{ field.Invalid(metricsBase.Child("createPrometheusRules"), tempo.Spec.Observability.Metrics.CreatePrometheusRules, "the prometheusOperator feature gate must be enabled to create PrometheusRules for Tempo components", )} } if tempo.Spec.Observability.Metrics.CreatePrometheusRules && !tempo.Spec.Observability.Metrics.CreateServiceMonitors { - return field.ErrorList{ + return nil, field.ErrorList{ field.Invalid(metricsBase.Child("createPrometheusRules"), tempo.Spec.Observability.Metrics.CreatePrometheusRules, "the Prometheus rules alert based on collected metrics, therefore the createServiceMonitors feature must be enabled when enabling the createPrometheusRules feature", )} @@ -347,7 +347,7 @@ func (v *validator) validateObservability(tempo v1alpha1.TempoStack) field.Error tracingBase := observabilityBase.Child("tracing") if tempo.Spec.Observability.Tracing.SamplingFraction != "" { if _, err := strconv.ParseFloat(tempo.Spec.Observability.Tracing.SamplingFraction, 64); err != nil { - return field.ErrorList{ + return nil, field.ErrorList{ field.Invalid( tracingBase.Child("sampling_fraction"), tempo.Spec.Observability.Tracing.SamplingFraction, @@ -357,12 +357,17 @@ func (v *validator) validateObservability(tempo v1alpha1.TempoStack) field.Error } if tempo.Spec.Observability.Tracing.JaegerAgentEndpoint != "" { - _, _, err := net.SplitHostPort(tempo.Spec.Observability.Tracing.JaegerAgentEndpoint) + return admission.Warnings{"The spec.observability.tracing.jaeger_agent_endpoint field is deprecated and ignored. " + + "Please migrate to the spec.observability.tracing.otlp_http_endpoint field."}, nil + } + + if tempo.Spec.Observability.Tracing.OTLPHTTPEndpoint != "" { + _, err := url.ParseRequestURI(tempo.Spec.Observability.Tracing.OTLPHTTPEndpoint) if err != nil { - return field.ErrorList{ + return nil, field.ErrorList{ field.Invalid( - tracingBase.Child("jaeger_agent_endpoint"), - tempo.Spec.Observability.Tracing.JaegerAgentEndpoint, + tracingBase.Child("otlp_http_endpoint"), + tempo.Spec.Observability.Tracing.OTLPHTTPEndpoint, err.Error(), )} } @@ -370,21 +375,21 @@ func (v *validator) validateObservability(tempo v1alpha1.TempoStack) field.Error grafanaBase := observabilityBase.Child("grafana") if tempo.Spec.Observability.Grafana.CreateDatasource && !v.ctrlConfig.Gates.GrafanaOperator { - return field.ErrorList{ + return nil, field.ErrorList{ field.Invalid(grafanaBase.Child("createDatasource"), tempo.Spec.Observability.Grafana.CreateDatasource, "the grafanaOperator feature gate must be enabled to create a Datasource for Tempo", )} } if tempo.Spec.Observability.Grafana.CreateDatasource && tempo.Spec.Template.Gateway.Enabled { - return field.ErrorList{field.Invalid( + return nil, field.ErrorList{field.Invalid( grafanaBase.Child("createDatasource"), tempo.Spec.Observability.Grafana.CreateDatasource, "creating a data source for Tempo is not support if the gateway is enabled", )} } - return nil + return nil, nil } func (v *validator) validateTenantConfigs(tempo v1alpha1.TempoStack) field.ErrorList { @@ -458,28 +463,26 @@ func (v *validator) validate(ctx context.Context, obj runtime.Object) (admission allWarnings := admission.Warnings{} allErrors := field.ErrorList{} - var warnings admission.Warnings - var errors field.ErrorList + addValidationResults := func(w admission.Warnings, e field.ErrorList) { + allWarnings = append(allWarnings, w...) + allErrors = append(allErrors, e...) + } allErrors = append(allErrors, validateName(tempo.Name)...) allErrors = append(allErrors, v.validateServiceAccount(ctx, *tempo)...) - - warnings, errors = v.validateStorage(ctx, *tempo) - allWarnings = append(allWarnings, warnings...) - allErrors = append(allErrors, errors...) + addValidationResults(v.validateStorage(ctx, *tempo)) if tempo.Spec.ExtraConfig != nil && len(tempo.Spec.ExtraConfig.Tempo.Raw) > 0 { allWarnings = append(allWarnings, admission.Warnings{ "override tempo configuration could potentially break the stack, use it carefully", }...) - } allErrors = append(allErrors, v.validateReplicationFactor(*tempo)...) allErrors = append(allErrors, v.validateQueryFrontend(*tempo)...) allErrors = append(allErrors, v.validateGateway(*tempo)...) allErrors = append(allErrors, v.validateTenantConfigs(*tempo)...) - allErrors = append(allErrors, v.validateObservability(*tempo)...) + addValidationResults(v.validateObservability(*tempo)) allErrors = append(allErrors, v.validateDeprecatedFields(*tempo)...) allErrors = append(allErrors, v.validateReceiverTLS(*tempo)...) allErrors = append(allErrors, v.validateConflictWithMonolithic(ctx, tempo)...) diff --git a/internal/webhooks/tempostack_webhook_test.go b/internal/webhooks/tempostack_webhook_test.go index 45aedb7f7..e6c402706 100644 --- a/internal/webhooks/tempostack_webhook_test.go +++ b/internal/webhooks/tempostack_webhook_test.go @@ -1712,22 +1712,22 @@ func TestValidatorObservabilityTracingConfig(t *testing.T) { }, }, { - name: "invalid jaeger agent address", + name: "invalid OTLP HTTP endpoint address", input: v1alpha1.TempoStack{ Spec: v1alpha1.TempoStackSpec{ Observability: v1alpha1.ObservabilitySpec{ Tracing: v1alpha1.TracingConfigSpec{ - SamplingFraction: "0.5", - JaegerAgentEndpoint: "--invalid--", + SamplingFraction: "0.5", + OTLPHTTPEndpoint: "--invalid--", }, }, }, }, expected: field.ErrorList{ field.Invalid( - tracingBase.Child("jaeger_agent_endpoint"), + tracingBase.Child("otlp_http_endpoint"), "--invalid--", - "address --invalid--: missing port in address", + "parse \"--invalid--\": invalid URI for request", ), }, }, @@ -1737,8 +1737,8 @@ func TestValidatorObservabilityTracingConfig(t *testing.T) { Spec: v1alpha1.TempoStackSpec{ Observability: v1alpha1.ObservabilitySpec{ Tracing: v1alpha1.TracingConfigSpec{ - SamplingFraction: "0.5", - JaegerAgentEndpoint: "agent:1234", + SamplingFraction: "0.5", + OTLPHTTPEndpoint: "agent:1234", }, }, }, @@ -1749,7 +1749,8 @@ func TestValidatorObservabilityTracingConfig(t *testing.T) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { v := &validator{ctrlConfig: tc.ctrlConfig} - assert.Equal(t, tc.expected, v.validateObservability(tc.input)) + _, errors := v.validateObservability(tc.input) + assert.Equal(t, tc.expected, errors) }) } } @@ -1844,7 +1845,8 @@ func TestValidatorObservabilityGrafana(t *testing.T) { for _, tc := range tt { t.Run(tc.name, func(t *testing.T) { v := &validator{ctrlConfig: tc.ctrlConfig} - assert.Equal(t, tc.expected, v.validateObservability(tc.input)) + _, errors := v.validateObservability(tc.input) + assert.Equal(t, tc.expected, errors) }) } }