From 2d0320a58b1563397d9e2a1be2790e2eccf4aa8f Mon Sep 17 00:00:00 2001 From: Ilia Petrov Date: Wed, 22 Apr 2026 10:27:36 +0300 Subject: [PATCH 1/6] feat: move metrics from apis to internal package Signed-off-by: Ilia Petrov --- {apis/v1beta1 => internal/metrics}/metrics.go | 26 ++-- .../metrics}/metrics_test.go | 112 +++++++++--------- internal/webhook/collector_webhook.go | 7 +- main.go | 7 +- 4 files changed, 79 insertions(+), 73 deletions(-) rename {apis/v1beta1 => internal/metrics}/metrics.go (85%) rename {apis/v1beta1 => internal/metrics}/metrics_test.go (86%) diff --git a/apis/v1beta1/metrics.go b/internal/metrics/metrics.go similarity index 85% rename from apis/v1beta1/metrics.go rename to internal/metrics/metrics.go index a9cef0f5a0..9471ef82fa 100644 --- a/apis/v1beta1/metrics.go +++ b/internal/metrics/metrics.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package v1beta1 +package metrics import ( "context" @@ -14,6 +14,8 @@ import ( sdkmetric "go.opentelemetry.io/otel/sdk/metric" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/metrics" + + otelv1beta1 "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" ) const ( @@ -52,8 +54,8 @@ type Metrics struct { connectorsCounter metric.Int64UpDownCounter } -// BootstrapMetrics configures the OpenTelemetry meter provider with the Prometheus exporter. -func BootstrapMetrics() (metric.MeterProvider, error) { +// Bootstrap configures the OpenTelemetry meter provider with the Prometheus exporter. +func Bootstrap() (metric.MeterProvider, error) { exporter, err := prometheus.New(prometheus.WithRegisterer(metrics.Registry)) if err != nil { return nil, err @@ -61,7 +63,7 @@ func BootstrapMetrics() (metric.MeterProvider, error) { return sdkmetric.NewMeterProvider(sdkmetric.WithReader(exporter)), err } -func NewMetrics(prv metric.MeterProvider, ctx context.Context, cl client.Reader) (*Metrics, error) { //nolint:revive //context-as-argument +func New(prv metric.MeterProvider, ctx context.Context, cl client.Reader) (*Metrics, error) { meter := prv.Meter(meterName) modeCounter, err := meter.Int64UpDownCounter(modeMetricName) if err != nil { @@ -110,7 +112,7 @@ func NewMetrics(prv metric.MeterProvider, ctx context.Context, cl client.Reader) // Init metrics from the first time the operator starts. func (m *Metrics) init(ctx context.Context, cl client.Reader) error { - list := &OpenTelemetryCollectorList{} + list := &otelv1beta1.OpenTelemetryCollectorList{} if err := cl.List(ctx, list); err != nil { return fmt.Errorf("failed to list: %w", err) } @@ -121,22 +123,22 @@ func (m *Metrics) init(ctx context.Context, cl client.Reader) error { return nil } -func (m *Metrics) Create(ctx context.Context, collector *OpenTelemetryCollector) { +func (m *Metrics) Create(ctx context.Context, collector *otelv1beta1.OpenTelemetryCollector) { m.updateComponentCounters(ctx, collector, true) m.updateGeneralCRMetricsComponents(ctx, collector, true) } -func (m *Metrics) Delete(ctx context.Context, collector *OpenTelemetryCollector) { +func (m *Metrics) Delete(ctx context.Context, collector *otelv1beta1.OpenTelemetryCollector) { m.updateComponentCounters(ctx, collector, false) m.updateGeneralCRMetricsComponents(ctx, collector, false) } -func (m *Metrics) Update(ctx context.Context, oldCollector, newCollector *OpenTelemetryCollector) { +func (m *Metrics) Update(ctx context.Context, oldCollector, newCollector *otelv1beta1.OpenTelemetryCollector) { m.Delete(ctx, oldCollector) m.Create(ctx, newCollector) } -func (m *Metrics) updateGeneralCRMetricsComponents(ctx context.Context, collector *OpenTelemetryCollector, up bool) { +func (m *Metrics) updateGeneralCRMetricsComponents(ctx context.Context, collector *otelv1beta1.OpenTelemetryCollector, up bool) { inc := 1 if !up { inc = -1 @@ -148,7 +150,7 @@ func (m *Metrics) updateGeneralCRMetricsComponents(ctx context.Context, collecto )) } -func (m *Metrics) updateComponentCounters(ctx context.Context, collector *OpenTelemetryCollector, up bool) { +func (m *Metrics) updateComponentCounters(ctx context.Context, collector *otelv1beta1.OpenTelemetryCollector, up bool) { components := getComponentsFromConfig(collector.Spec.Config) moveCounter(ctx, collector, components.receivers, m.receiversCounter, up) moveCounter(ctx, collector, components.exporters, m.exporterCounter, up) @@ -176,7 +178,7 @@ func extractElements(elements map[string]any) []string { return items } -func getComponentsFromConfig(yamlContent Config) *componentDefinitions { +func getComponentsFromConfig(yamlContent otelv1beta1.Config) *componentDefinitions { info := &componentDefinitions{ receivers: extractElements(yamlContent.Receivers.Object), exporters: extractElements(yamlContent.Exporters.Object), @@ -198,7 +200,7 @@ func getComponentsFromConfig(yamlContent Config) *componentDefinitions { } func moveCounter( - ctx context.Context, collector *OpenTelemetryCollector, types []string, upDown metric.Int64UpDownCounter, up bool, + ctx context.Context, collector *otelv1beta1.OpenTelemetryCollector, types []string, upDown metric.Int64UpDownCounter, up bool, ) { for _, exporter := range types { inc := 1 diff --git a/apis/v1beta1/metrics_test.go b/internal/metrics/metrics_test.go similarity index 86% rename from apis/v1beta1/metrics_test.go rename to internal/metrics/metrics_test.go index 1de8af5005..d96a779436 100644 --- a/apis/v1beta1/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package v1beta1 +package metrics import ( "context" @@ -18,6 +18,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" + + otelv1beta1 "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" ) var wantInstrumentationScope = instrumentation.Scope{ @@ -25,21 +27,21 @@ var wantInstrumentationScope = instrumentation.Scope{ } func TestOTELCollectorCRDMetrics(t *testing.T) { - otelcollector1 := &OpenTelemetryCollector{ + otelcollector1 := &otelv1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Name: "collector1", Namespace: "test1", }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - Config: Config{ - Processors: &AnyConfig{ + Spec: otelv1beta1.OpenTelemetryCollectorSpec{ + Mode: otelv1beta1.ModeDeployment, + Config: otelv1beta1.Config{ + Processors: &otelv1beta1.AnyConfig{ Object: map[string]any{ "batch": nil, "foo": nil, }, }, - Extensions: &AnyConfig{ + Extensions: &otelv1beta1.AnyConfig{ Object: map[string]any{ "extfoo": nil, }, @@ -48,26 +50,26 @@ func TestOTELCollectorCRDMetrics(t *testing.T) { }, } - otelcollector2 := &OpenTelemetryCollector{ + otelcollector2 := &otelv1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Name: "collector2", Namespace: "test2", }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - Config: Config{ - Processors: &AnyConfig{ + Spec: otelv1beta1.OpenTelemetryCollectorSpec{ + Mode: otelv1beta1.ModeSidecar, + Config: otelv1beta1.Config{ + Processors: &otelv1beta1.AnyConfig{ Object: map[string]any{ "x": nil, "y": nil, }, }, - Extensions: &AnyConfig{ + Extensions: &otelv1beta1.AnyConfig{ Object: map[string]any{ "z/r": nil, }, }, - Exporters: AnyConfig{ + Exporters: otelv1beta1.AnyConfig{ Object: map[string]any{ "w": nil, }, @@ -76,26 +78,26 @@ func TestOTELCollectorCRDMetrics(t *testing.T) { }, } - updatedCollector1 := &OpenTelemetryCollector{ + updatedCollector1 := &otelv1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Name: "collector1", Namespace: "test1", }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - Config: Config{ - Processors: &AnyConfig{ + Spec: otelv1beta1.OpenTelemetryCollectorSpec{ + Mode: otelv1beta1.ModeSidecar, + Config: otelv1beta1.Config{ + Processors: &otelv1beta1.AnyConfig{ Object: map[string]any{ "foo": nil, "y": nil, }, }, - Extensions: &AnyConfig{ + Extensions: &otelv1beta1.AnyConfig{ Object: map[string]any{ "z/r": nil, }, }, - Exporters: AnyConfig{ + Exporters: otelv1beta1.AnyConfig{ Object: map[string]any{ "w": nil, }, @@ -106,7 +108,7 @@ func TestOTELCollectorCRDMetrics(t *testing.T) { tests := []struct { name string - testFunction func(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, reader metric.Reader) + testFunction func(t *testing.T, m *Metrics, collectors []*otelv1beta1.OpenTelemetryCollector, reader metric.Reader) }{ { name: "Create", @@ -122,8 +124,8 @@ func TestOTELCollectorCRDMetrics(t *testing.T) { }, } schemeBuilder := runtime.NewSchemeBuilder(func(s *runtime.Scheme) error { - s.AddKnownTypes(GroupVersion, &OpenTelemetryCollector{}, &OpenTelemetryCollectorList{}) - metav1.AddToGroupVersion(s, GroupVersion) + s.AddKnownTypes(otelv1beta1.GroupVersion, &otelv1beta1.OpenTelemetryCollector{}, &otelv1beta1.OpenTelemetryCollectorList{}) + metav1.AddToGroupVersion(s, otelv1beta1.GroupVersion) return nil }) scheme := runtime.NewScheme() @@ -132,33 +134,33 @@ func TestOTELCollectorCRDMetrics(t *testing.T) { reader := metric.NewManualReader() provider := metric.NewMeterProvider(metric.WithReader(reader)) cl := fake.NewClientBuilder().WithScheme(scheme).Build() - crdMetrics, err := NewMetrics(provider, context.Background(), cl) + crdMetrics, err := New(provider, context.Background(), cl) assert.NoError(t, err) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tt.testFunction(t, crdMetrics, []*OpenTelemetryCollector{otelcollector1, otelcollector2, updatedCollector1}, reader) + tt.testFunction(t, crdMetrics, []*otelv1beta1.OpenTelemetryCollector{otelcollector1, otelcollector2, updatedCollector1}, reader) }) } } func TestOTELCollectorInitMetrics(t *testing.T) { - otelcollector1 := OpenTelemetryCollector{ + otelcollector1 := otelv1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Name: "collector1", Namespace: "test1", Labels: map[string]string{"app.kubernetes.io/managed-by": "opentelemetry-operator"}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeDeployment, - Config: Config{ - Processors: &AnyConfig{ + Spec: otelv1beta1.OpenTelemetryCollectorSpec{ + Mode: otelv1beta1.ModeDeployment, + Config: otelv1beta1.Config{ + Processors: &otelv1beta1.AnyConfig{ Object: map[string]any{ "batch": nil, "foo": nil, }, }, - Extensions: &AnyConfig{ + Extensions: &otelv1beta1.AnyConfig{ Object: map[string]any{ "extfoo": nil, }, @@ -167,27 +169,27 @@ func TestOTELCollectorInitMetrics(t *testing.T) { }, } - otelcollector2 := OpenTelemetryCollector{ + otelcollector2 := otelv1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Name: "collector2", Namespace: "test2", Labels: map[string]string{"app.kubernetes.io/managed-by": "opentelemetry-operator"}, }, - Spec: OpenTelemetryCollectorSpec{ - Mode: ModeSidecar, - Config: Config{ - Processors: &AnyConfig{ + Spec: otelv1beta1.OpenTelemetryCollectorSpec{ + Mode: otelv1beta1.ModeSidecar, + Config: otelv1beta1.Config{ + Processors: &otelv1beta1.AnyConfig{ Object: map[string]any{ "x": nil, "y": nil, }, }, - Extensions: &AnyConfig{ + Extensions: &otelv1beta1.AnyConfig{ Object: map[string]any{ "z/r": nil, }, }, - Exporters: AnyConfig{ + Exporters: otelv1beta1.AnyConfig{ Object: map[string]any{ "w": nil, }, @@ -197,21 +199,21 @@ func TestOTELCollectorInitMetrics(t *testing.T) { } schemeBuilder := runtime.NewSchemeBuilder(func(s *runtime.Scheme) error { - s.AddKnownTypes(GroupVersion, &OpenTelemetryCollector{}, &OpenTelemetryCollectorList{}) - metav1.AddToGroupVersion(s, GroupVersion) + s.AddKnownTypes(otelv1beta1.GroupVersion, &otelv1beta1.OpenTelemetryCollector{}, &otelv1beta1.OpenTelemetryCollectorList{}) + metav1.AddToGroupVersion(s, otelv1beta1.GroupVersion) return nil }) scheme := runtime.NewScheme() err := schemeBuilder.AddToScheme(scheme) require.NoError(t, err) - list := &OpenTelemetryCollectorList{ - Items: []OpenTelemetryCollector{otelcollector1, otelcollector2}, + list := &otelv1beta1.OpenTelemetryCollectorList{ + Items: []otelv1beta1.OpenTelemetryCollector{otelcollector1, otelcollector2}, } require.NoError(t, err, "Should be able to add custom types") cl := fake.NewClientBuilder().WithLists(list).WithScheme(scheme).Build() reader := metric.NewManualReader() provider := metric.NewMeterProvider(metric.WithReader(reader)) - _, err = NewMetrics(provider, context.Background(), cl) + _, err = New(provider, context.Background(), cl) assert.NoError(t, err) rm := metricdata.ResourceMetrics{} @@ -238,7 +240,7 @@ func TestOTELCollectorInitMetrics(t *testing.T) { Attributes: attribute.NewSet( attribute.Key("collector_name").String("collector2"), attribute.Key("namespace").String("test2"), - attribute.Key("type").String(string(ModeSidecar)), + attribute.Key("type").String(string(otelv1beta1.ModeSidecar)), ), Value: 1, }, @@ -332,7 +334,7 @@ func TestOTELCollectorInitMetrics(t *testing.T) { metricdatatest.AssertEqual(t, want, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp()) } -func checkCreate(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, reader metric.Reader) { +func checkCreate(t *testing.T, m *Metrics, collectors []*otelv1beta1.OpenTelemetryCollector, reader metric.Reader) { provider := metric.NewMeterProvider(metric.WithReader(reader)) otel.SetMeterProvider(provider) @@ -431,7 +433,7 @@ func checkCreate(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, Attributes: attribute.NewSet( attribute.Key("collector_name").String("collector2"), attribute.Key("namespace").String("test2"), - attribute.Key("type").String(string(ModeSidecar)), + attribute.Key("type").String(string(otelv1beta1.ModeSidecar)), ), Value: 1, }, @@ -525,7 +527,7 @@ func checkCreate(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, metricdatatest.AssertEqual(t, want, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp()) } -func checkUpdate(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, reader metric.Reader) { +func checkUpdate(t *testing.T, m *Metrics, collectors []*otelv1beta1.OpenTelemetryCollector, reader metric.Reader) { m.Update(context.Background(), collectors[0], collectors[2]) rm := metricdata.ResourceMetrics{} @@ -544,7 +546,7 @@ func checkUpdate(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, Attributes: attribute.NewSet( attribute.Key("collector_name").String("collector1"), attribute.Key("namespace").String("test1"), - attribute.Key("type").String(string(ModeDeployment)), + attribute.Key("type").String(string(otelv1beta1.ModeDeployment)), ), Value: 0, }, @@ -552,7 +554,7 @@ func checkUpdate(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, Attributes: attribute.NewSet( attribute.Key("collector_name").String("collector1"), attribute.Key("namespace").String("test1"), - attribute.Key("type").String(string(ModeSidecar)), + attribute.Key("type").String(string(otelv1beta1.ModeSidecar)), ), Value: 1, }, @@ -560,7 +562,7 @@ func checkUpdate(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, Attributes: attribute.NewSet( attribute.Key("collector_name").String("collector2"), attribute.Key("namespace").String("test2"), - attribute.Key("type").String(string(ModeSidecar)), + attribute.Key("type").String(string(otelv1beta1.ModeSidecar)), ), Value: 1, }, @@ -677,7 +679,7 @@ func checkUpdate(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, metricdatatest.AssertEqual(t, want, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp()) } -func checkDelete(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, reader metric.Reader) { +func checkDelete(t *testing.T, m *Metrics, collectors []*otelv1beta1.OpenTelemetryCollector, reader metric.Reader) { m.Delete(context.Background(), collectors[1]) rm := metricdata.ResourceMetrics{} err := reader.Collect(context.Background(), &rm) @@ -694,7 +696,7 @@ func checkDelete(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, Attributes: attribute.NewSet( attribute.Key("collector_name").String("collector1"), attribute.Key("namespace").String("test1"), - attribute.Key("type").String(string(ModeDeployment)), + attribute.Key("type").String(string(otelv1beta1.ModeDeployment)), ), Value: 0, }, @@ -702,7 +704,7 @@ func checkDelete(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, Attributes: attribute.NewSet( attribute.Key("collector_name").String("collector1"), attribute.Key("namespace").String("test1"), - attribute.Key("type").String(string(ModeSidecar)), + attribute.Key("type").String(string(otelv1beta1.ModeSidecar)), ), Value: 1, }, @@ -710,7 +712,7 @@ func checkDelete(t *testing.T, m *Metrics, collectors []*OpenTelemetryCollector, Attributes: attribute.NewSet( attribute.Key("collector_name").String("collector2"), attribute.Key("namespace").String("test2"), - attribute.Key("type").String(string(ModeSidecar)), + attribute.Key("type").String(string(otelv1beta1.ModeSidecar)), ), Value: 0, }, diff --git a/internal/webhook/collector_webhook.go b/internal/webhook/collector_webhook.go index 1b599a685d..41393c2900 100644 --- a/internal/webhook/collector_webhook.go +++ b/internal/webhook/collector_webhook.go @@ -21,6 +21,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/fips" ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/metrics" "github.com/open-telemetry/opentelemetry-operator/internal/naming" "github.com/open-telemetry/opentelemetry-operator/internal/rbac" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" @@ -45,7 +46,7 @@ type CollectorWebhook struct { cfg config.Config scheme *runtime.Scheme reviewer *rbac.Reviewer - metrics *v1beta1.Metrics + metrics *metrics.Metrics bv BuildValidator fips fips.FIPSCheck recorder events.EventRecorder @@ -407,7 +408,7 @@ func NewCollectorWebhook( cfg config.Config, reviewer *rbac.Reviewer, recorder events.EventRecorder, - metrics *v1beta1.Metrics, + metrics *metrics.Metrics, bv BuildValidator, fips fips.FIPSCheck, ) *CollectorWebhook { @@ -423,7 +424,7 @@ func NewCollectorWebhook( } } -func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *v1beta1.Metrics, bv BuildValidator, fipsCheck fips.FIPSCheck) error { +func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *metrics.Metrics, bv BuildValidator, fipsCheck fips.FIPSCheck) error { cvw := NewCollectorWebhook(mgr.GetLogger().WithValues("handler", "CollectorWebhook", "version", "v1beta1"), mgr.GetScheme(), cfg, reviewer, mgr.GetEventRecorder("opentelemetry-operator"), metrics, bv, fipsCheck) return ctrl.NewWebhookManagedBy(mgr, &v1beta1.OpenTelemetryCollector{}). WithValidator(cvw). diff --git a/main.go b/main.go index 013b57aebf..9c97131301 100644 --- a/main.go +++ b/main.go @@ -60,6 +60,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/instrumentation" instrumentationupgrade "github.com/open-telemetry/opentelemetry-operator/internal/instrumentation/upgrade" collectorManifests "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/metrics" openshiftDashboards "github.com/open-telemetry/opentelemetry-operator/internal/openshift/dashboards" operatormetrics "github.com/open-telemetry/opentelemetry-operator/internal/operator-metrics" "github.com/open-telemetry/opentelemetry-operator/internal/operatornetworkpolicy" @@ -447,15 +448,15 @@ func main() { } if cfg.EnableWebhooks { - var crdMetrics *otelv1beta1.Metrics + var crdMetrics *metrics.Metrics if cfg.EnableCRMetrics { - meterProvider, metricsErr := otelv1beta1.BootstrapMetrics() + meterProvider, metricsErr := metrics.Bootstrap() if metricsErr != nil { setupLog.Error(metricsErr, "Error bootstrapping CRD metrics") } - crdMetrics, err = otelv1beta1.NewMetrics(meterProvider, ctx, mgr.GetAPIReader()) + crdMetrics, err = metrics.New(meterProvider, ctx, mgr.GetAPIReader()) if err != nil { setupLog.Error(err, "Error init CRD metrics") } From fac5f5c48c3938199e9c0d034e982e333a1d7ed9 Mon Sep 17 00:00:00 2001 From: Ilia Petrov Date: Thu, 23 Apr 2026 10:18:10 +0300 Subject: [PATCH 2/6] feat: move config parsing from apis to internal/otelconfig Signed-off-by: Ilia Petrov --- apis/v1alpha1/convert.go | 3 +- apis/v1beta1/config.go | 522 ------ apis/v1beta1/config_test.go | 1409 ---------------- .../manifests/collector/config_replace.go | 3 +- internal/manifests/collector/configmap.go | 3 +- internal/manifests/collector/container.go | 13 +- internal/manifests/collector/ingress.go | 3 +- internal/manifests/collector/podmonitor.go | 3 +- internal/manifests/collector/rbac.go | 7 +- internal/manifests/collector/service.go | 7 +- .../manifests/collector/servicemonitor.go | 3 +- internal/manifests/collector/volume.go | 3 +- .../manifests/targetallocator/configmap.go | 3 +- .../targetallocator/configmap_test.go | 3 +- internal/metrics/metrics.go | 2 +- internal/metrics/metrics_test.go | 4 +- internal/otelconfig/config.go | 531 ++++++ internal/otelconfig/config_test.go | 1425 +++++++++++++++++ .../otelconfig}/helpers.go | 7 +- .../otelconfig}/helpers_test.go | 2 +- internal/webhook/collector_webhook.go | 11 +- internal/webhook/collector_webhook_test.go | 3 +- main.go | 2 +- pkg/collector/upgrade/v0_111_0.go | 3 +- pkg/collector/upgrade/v0_122_0.go | 9 +- pkg/sidecar/pod_test.go | 5 +- 26 files changed, 2018 insertions(+), 1971 deletions(-) create mode 100644 internal/otelconfig/config.go create mode 100644 internal/otelconfig/config_test.go rename {apis/v1beta1 => internal/otelconfig}/helpers.go (97%) rename {apis/v1beta1 => internal/otelconfig}/helpers_test.go (99%) diff --git a/apis/v1alpha1/convert.go b/apis/v1alpha1/convert.go index b6575958e5..36b3e20de7 100644 --- a/apis/v1alpha1/convert.go +++ b/apis/v1alpha1/convert.go @@ -13,6 +13,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/conversion" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) var _ conversion.Convertible = &OpenTelemetryCollector{} @@ -301,7 +302,7 @@ func tov1alpha1Ports(in []v1beta1.PortsSpec) []PortsSpec { func tov1alpha1(in v1beta1.OpenTelemetryCollector) (*OpenTelemetryCollector, error) { c := in.DeepCopy() - configYaml, err := c.Spec.Config.Yaml() + configYaml, err := otelconfig.Yaml(&c.Spec.Config) if err != nil { return nil, err } diff --git a/apis/v1beta1/config.go b/apis/v1beta1/config.go index a37390ce60..0ca87838da 100644 --- a/apis/v1beta1/config.go +++ b/apis/v1beta1/config.go @@ -4,26 +4,10 @@ package v1beta1 import ( - "bytes" "encoding/json" - "fmt" "maps" - "math" - "slices" - "strings" - "dario.cat/mergo" - "github.com/go-logr/logr" - go_yaml "github.com/goccy/go-yaml" otelConfig "go.opentelemetry.io/contrib/otelconf/v0.3.0" - corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" - - "github.com/open-telemetry/opentelemetry-operator/internal/components" - "github.com/open-telemetry/opentelemetry-operator/internal/components/exporters" - "github.com/open-telemetry/opentelemetry-operator/internal/components/extensions" - "github.com/open-telemetry/opentelemetry-operator/internal/components/processors" - "github.com/open-telemetry/opentelemetry-operator/internal/components/receivers" ) type ComponentKind int @@ -130,347 +114,6 @@ type Config struct { Service Service `json:"service" yaml:"service"` } -// GetEnabledComponents constructs a list of enabled components by component type. -func (c *Config) GetEnabledComponents() map[ComponentKind]map[string]any { - toReturn := map[ComponentKind]map[string]any{ - KindReceiver: {}, - KindProcessor: {}, - KindExporter: {}, - KindExtension: {}, - } - for _, extension := range c.Service.Extensions { - toReturn[KindExtension][extension] = struct{}{} - } - - for _, pipeline := range c.Service.Pipelines { - if pipeline == nil { - continue - } - for _, componentId := range pipeline.Receivers { - toReturn[KindReceiver][componentId] = struct{}{} - } - for _, componentId := range pipeline.Exporters { - toReturn[KindExporter][componentId] = struct{}{} - } - for _, componentId := range pipeline.Processors { - toReturn[KindProcessor][componentId] = struct{}{} - } - } - for _, componentId := range c.Service.Extensions { - toReturn[KindExtension][componentId] = struct{}{} - } - return toReturn -} - -// getRbacRulesForComponentKinds gets the RBAC Rules for the given ComponentKind(s). -func (c *Config) getRbacRulesForComponentKinds(logger logr.Logger, componentKinds ...ComponentKind) ([]rbacv1.PolicyRule, error) { - var rules []rbacv1.PolicyRule - enabledComponents := c.GetEnabledComponents() - for _, componentKind := range componentKinds { - var retriever components.ParserRetriever - var cfg AnyConfig - switch componentKind { - case KindReceiver: - retriever = receivers.ReceiverFor - cfg = c.Receivers - case KindExporter: - retriever = exporters.ParserFor - cfg = c.Exporters - case KindProcessor: - retriever = processors.ProcessorFor - if c.Processors == nil { - cfg = AnyConfig{} - } else { - cfg = *c.Processors - } - case KindExtension: - retriever = extensions.ParserFor - if c.Extensions == nil { - cfg = AnyConfig{} - } else { - cfg = *c.Extensions - } - default: - logger.V(1).Info("unknown component kind", "kind", componentKind) - continue - } - for componentName := range enabledComponents[componentKind] { - // TODO: Clean up the naming here and make it simpler to use a retriever. - parser := retriever(componentName) - parsedRules, err := parser.GetRBACRules(logger, cfg.Object[componentName]) - if err != nil { - return nil, err - } - rules = append(rules, parsedRules...) - } - } - return rules, nil -} - -// getPortsForComponentKinds gets the ports for the given ComponentKind(s). -func (c *Config) getPortsForComponentKinds(logger logr.Logger, componentKinds ...ComponentKind) ([]corev1.ServicePort, error) { - var ports []corev1.ServicePort - enabledComponents := c.GetEnabledComponents() - for _, componentKind := range componentKinds { - var retriever components.ParserRetriever - var cfg AnyConfig - switch componentKind { - case KindReceiver: - retriever = receivers.ReceiverFor - cfg = c.Receivers - case KindExporter: - retriever = exporters.ParserFor - cfg = c.Exporters - case KindProcessor: - continue - case KindExtension: - retriever = extensions.ParserFor - if c.Extensions == nil { - cfg = AnyConfig{} - } else { - cfg = *c.Extensions - } - } - for componentName := range enabledComponents[componentKind] { - // TODO: Clean up the naming here and make it simpler to use a retriever. - parser := retriever(componentName) - parsedPorts, err := parser.Ports(logger, componentName, cfg.Object[componentName]) - if err != nil { - return nil, err - } - ports = append(ports, parsedPorts...) - } - } - - slices.SortFunc(ports, func(i, j corev1.ServicePort) int { - return strings.Compare(i.Name, j.Name) - }) - - return ports, nil -} - -// getEnvironmentVariablesForComponentKinds gets the environment variables for the given ComponentKind(s). -func (c *Config) getEnvironmentVariablesForComponentKinds(logger logr.Logger, componentKinds ...ComponentKind) ([]corev1.EnvVar, error) { - envVars := []corev1.EnvVar{} - enabledComponents := c.GetEnabledComponents() - for _, componentKind := range componentKinds { - var retriever components.ParserRetriever - var cfg AnyConfig - - switch componentKind { - case KindReceiver: - retriever = receivers.ReceiverFor - cfg = c.Receivers - case KindExporter, KindProcessor, KindExtension: - continue - } - for componentName := range enabledComponents[componentKind] { - parser := retriever(componentName) - parsedEnvVars, err := parser.GetEnvironmentVariables(logger, cfg.Object[componentName]) - if err != nil { - return nil, err - } - envVars = append(envVars, parsedEnvVars...) - } - } - - slices.SortFunc(envVars, func(i, j corev1.EnvVar) int { - return strings.Compare(i.Name, j.Name) - }) - - return envVars, nil -} - -// applyDefaultForComponentKinds applies defaults to the endpoints for the given ComponentKind(s). -// If defaultsCfg.TLSProfile is set, TLS defaults are also applied via the Parser.GetDefaultConfig method. -// Returns a list of events that should be recorded by the caller. -func (c *Config) applyDefaultForComponentKinds(logger logr.Logger, parserOpts []components.DefaultOption, componentKinds ...ComponentKind) ([]EventInfo, error) { - events, err := c.Service.ApplyDefaults(logger) - if err != nil { - return events, err - } - enabledComponents := c.GetEnabledComponents() - for _, componentKind := range componentKinds { - var retriever components.ParserRetriever - var cfg AnyConfig - switch componentKind { - case KindReceiver: - retriever = receivers.ReceiverFor - cfg = c.Receivers - case KindExporter, KindProcessor: - retriever = exporters.ParserFor - cfg = c.Exporters - case KindExtension: - if c.Extensions == nil { - continue - } - retriever = extensions.ParserFor - cfg = *c.Extensions - } - for componentName := range enabledComponents[componentKind] { - parser := retriever(componentName) - componentConf := cfg.Object[componentName] - newCfg, err := parser.GetDefaultConfig(logger, componentConf, parserOpts...) - if err != nil { - return events, err - } - - // We need to ensure we don't remove any fields in defaulting. - mappedCfg, ok := newCfg.(map[string]any) - if !ok || mappedCfg == nil { - logger.V(1).Info("returned default configuration invalid", - "warn", "could not apply component defaults", - "component", componentName, - ) - continue - } - - if componentConf == nil { - componentConf = map[string]any{} - } - if err := mergo.Merge(&mappedCfg, componentConf); err != nil { - return events, err - } - cfg.Object[componentName] = mappedCfg - } - } - - return events, nil -} - -func (c *Config) GetReceiverPorts(logger logr.Logger) ([]corev1.ServicePort, error) { - return c.getPortsForComponentKinds(logger, KindReceiver) -} - -func (c *Config) GetExporterPorts(logger logr.Logger) ([]corev1.ServicePort, error) { - return c.getPortsForComponentKinds(logger, KindExporter) -} - -func (c *Config) GetExtensionPorts(logger logr.Logger) ([]corev1.ServicePort, error) { - return c.getPortsForComponentKinds(logger, KindExtension) -} - -func (c *Config) GetReceiverAndExporterPorts(logger logr.Logger) ([]corev1.ServicePort, error) { - return c.getPortsForComponentKinds(logger, KindReceiver, KindExporter) -} - -func (c *Config) GetAllPorts(logger logr.Logger) ([]corev1.ServicePort, error) { - return c.getPortsForComponentKinds(logger, KindReceiver, KindExporter, KindExtension) -} - -func (c *Config) GetEnvironmentVariables(logger logr.Logger) ([]corev1.EnvVar, error) { - return c.getEnvironmentVariablesForComponentKinds(logger, KindReceiver) -} - -func (c *Config) GetAllRbacRules(logger logr.Logger) ([]rbacv1.PolicyRule, error) { - return c.getRbacRulesForComponentKinds(logger, KindReceiver, KindExporter, KindProcessor, KindExtension) -} - -// ApplyDefaults applies default configuration values to the collector config. -// Optional DefaultsOption arguments can be provided to customize behavior. -func (c *Config) ApplyDefaults(logger logr.Logger, opts ...components.DefaultOption) ([]EventInfo, error) { - return c.applyDefaultForComponentKinds(logger, opts, KindReceiver, KindExporter, KindExtension) -} - -// GetLivenessProbe gets the first enabled liveness probe. There should only ever be one extension enabled -// that provides the hinting for the liveness probe. -func (c *Config) GetLivenessProbe(logger logr.Logger) (*corev1.Probe, error) { - if c.Extensions == nil { - return nil, nil - } - - enabledComponents := c.GetEnabledComponents() - for componentName := range enabledComponents[KindExtension] { - // TODO: Clean up the naming here and make it simpler to use a retriever. - parser := extensions.ParserFor(componentName) - if probe, err := parser.GetLivenessProbe(logger, c.Extensions.Object[componentName]); err != nil { - return nil, err - } else if probe != nil { - return probe, nil - } - } - return nil, nil -} - -// GetReadinessProbe gets the first enabled readiness probe. There should only ever be one extension enabled -// that provides the hinting for the readiness probe. -func (c *Config) GetReadinessProbe(logger logr.Logger) (*corev1.Probe, error) { - if c.Extensions == nil { - return nil, nil - } - - enabledComponents := c.GetEnabledComponents() - for componentName := range enabledComponents[KindExtension] { - // TODO: Clean up the naming here and make it simpler to use a retriever. - parser := extensions.ParserFor(componentName) - if probe, err := parser.GetReadinessProbe(logger, c.Extensions.Object[componentName]); err != nil { - return nil, err - } else if probe != nil { - return probe, nil - } - } - return nil, nil -} - -// GetStartupProbe gets the first enabled startup probe. There should only ever be one extension enabled -// that provides the hinting for the startup probe. -func (c *Config) GetStartupProbe(logger logr.Logger) (*corev1.Probe, error) { - if c.Extensions == nil { - return nil, nil - } - - enabledComponents := c.GetEnabledComponents() - for componentName := range enabledComponents[KindExtension] { - // TODO: Clean up the naming here and make it simpler to use a retriever. - parser := extensions.ParserFor(componentName) - if probe, err := parser.GetStartupProbe(logger, c.Extensions.Object[componentName]); err != nil { - return nil, err - } else if probe != nil { - return probe, nil - } - } - return nil, nil -} - -// Yaml encodes the current object and returns it as a string. -func (c *Config) Yaml() (string, error) { - var buf bytes.Buffer - yamlEncoder := go_yaml.NewEncoder(&buf, go_yaml.IndentSequence(true), go_yaml.AutoInt()) - if err := yamlEncoder.Encode(&c); err != nil { - return "", err - } - return buf.String(), nil -} - -// NullObjects returns null objects in the config. -func (c *Config) NullObjects() []string { - var nullKeys []string - if nulls := getNullValuedKeys(c.Receivers.Object); len(nulls) > 0 { - nullKeys = append(nullKeys, addPrefix("receivers.", nulls)...) - } - if nulls := getNullValuedKeys(c.Exporters.Object); len(nulls) > 0 { - nullKeys = append(nullKeys, addPrefix("exporters.", nulls)...) - } - if c.Processors != nil { - if nulls := getNullValuedKeys(c.Processors.Object); len(nulls) > 0 { - nullKeys = append(nullKeys, addPrefix("processors.", nulls)...) - } - } - if c.Extensions != nil { - if nulls := getNullValuedKeys(c.Extensions.Object); len(nulls) > 0 { - nullKeys = append(nullKeys, addPrefix("extensions.", nulls)...) - } - } - if c.Connectors != nil { - if nulls := getNullValuedKeys(c.Connectors.Object); len(nulls) > 0 { - nullKeys = append(nullKeys, addPrefix("connectors.", nulls)...) - } - } - // Make the return deterministic. The config uses maps therefore processing order is non-deterministic. - slices.Sort(nullKeys) - return nullKeys -} - type Service struct { Extensions []string `json:"extensions,omitempty" yaml:"extensions,omitempty"` // +kubebuilder:pruning:PreserveUnknownFields @@ -479,107 +122,6 @@ type Service struct { Pipelines map[string]*Pipeline `json:"pipelines" yaml:"pipelines"` } -const ( - defaultServicePort int32 = 8888 - defaultServiceHost = "0.0.0.0" -) - -// MetricsEndpoint attempts gets the host and port number from the host address without doing any validation regarding the -// address itself. -// It works even before env var expansion happens, when a simple `net.SplitHostPort` would fail because of the extra colon -// from the env var, i.e. the address looks like "${env:POD_IP}:4317", "${env:POD_IP}", or "${POD_IP}". -// In cases which the port itself is a variable, i.e. "${env:POD_IP}:${env:PORT}", this returns an error. This happens -// because the port is used to generate Service objects and mappings. -func (s *Service) MetricsEndpoint(logger logr.Logger) (host string, port int32, err error) { - telemetry := s.GetTelemetry(logger) - if telemetry == nil { - return defaultServiceHost, defaultServicePort, nil - } - - if telemetry.Metrics.Address != "" && len(telemetry.Metrics.Readers) == 0 { - host, port, err := parseAddressEndpoint(telemetry.Metrics.Address) - if err != nil { - return "", 0, err - } - - return host, port, nil - } - - for _, r := range telemetry.Metrics.Readers { - if r.Pull == nil { - continue - } - prom := r.Pull.Exporter.Prometheus - if prom == nil { - continue - } - host := defaultServiceHost - if prom.Host != nil && *prom.Host != "" { - host = *prom.Host - } - port := defaultServicePort - if prom.Port != nil && *prom.Port != 0 { - if *prom.Port < 0 || *prom.Port > math.MaxUint16 { - return "", 0, fmt.Errorf("invalid prometheus metrics port: %d", *prom.Port) - } - port = int32(*prom.Port) - } - return host, port, nil - } - - return defaultServiceHost, defaultServicePort, nil -} - -// ApplyDefaults inserts configuration defaults if it has not been set. -// Returns a list of events that should be recorded by the caller. -func (s *Service) ApplyDefaults(logger logr.Logger) ([]EventInfo, error) { - var events []EventInfo - tel := s.GetTelemetry(logger) - - if tel == nil { - logger.V(2).Info("no telemetry configuration parsed, creating default") - tel = &Telemetry{} - s.Telemetry = &AnyConfig{ - Object: map[string]any{}, - } - } - - if tel.Metrics.Address != "" || len(tel.Metrics.Readers) != 0 { - // The user already set the address or the readers, so we don't need to do anything - logger.V(1).Info("telemetry configuration already provided by user, skipping defaults", - "metricsAddress", tel.Metrics.Address, - "readersCount", len(tel.Metrics.Readers)) - return events, nil - } - - logger.V(2).Info("no telemetry readers configuration found, applying default Prometheus endpoint") - - host, port, err := s.MetricsEndpoint(logger) - if err != nil { - logger.Error(err, "failed to determine metrics endpoint for default configuration") - return events, err - } - - reader := AddPrometheusMetricsEndpoint(host, port) - tel.Metrics.Readers = append(tel.Metrics.Readers, reader) - - events = append(events, EventInfo{ - Type: corev1.EventTypeNormal, - Reason: "Spec.Service.Telemetry.DefaultsApplied", - Message: fmt.Sprintf("Applied default Prometheus telemetry configuration (host: %s, port: %d)", host, port), - }) - - telConfig, err := tel.ToAnyConfig() - if err != nil { - return events, err - } - - if err := mergo.Merge(&s.Telemetry.Object, telConfig.Object); err != nil { - return events, err - } - return events, nil -} - // MetricsConfig comes from the collector. type MetricsConfig struct { // Level is the level of telemetry metrics, the possible values are: @@ -620,67 +162,3 @@ type Telemetry struct { // attribute must be specified in this map with null YAML value (nil string pointer). Resource map[string]*string `json:"resource,omitempty" yaml:"resource,omitempty"` } - -// ToAnyConfig converts the Telemetry struct to an AnyConfig struct. -func (t *Telemetry) ToAnyConfig() (*AnyConfig, error) { - data, err := json.Marshal(t) - if err != nil { - return nil, err - } - var result map[string]any - if err := json.Unmarshal(data, &result); err != nil { - return nil, err - } - - normalizeConfig(result) - - return &AnyConfig{ - Object: result, - }, nil -} - -func AddPrometheusMetricsEndpoint(host string, port int32) otelConfig.MetricReader { - portInt := int(port) - return otelConfig.MetricReader{ - Pull: &otelConfig.PullMetricReader{ - Exporter: otelConfig.PullMetricExporter{ - Prometheus: &otelConfig.Prometheus{ - Host: &host, - Port: &portInt, - }, - }, - }, - } -} - -// GetTelemetry serves as a helper function to access the fields we care about in the underlying telemetry struct. -// This exists to avoid needing to worry extra fields in the telemetry struct. -func (s *Service) GetTelemetry(logger logr.Logger) *Telemetry { - if s.Telemetry == nil { - logger.V(2).Info("no spec.service.telemetry configuration found") - return nil - } - - // Convert map to JSON bytes - jsonData, err := json.Marshal(s.Telemetry) - if err != nil { - logger.Error(err, "failed to marshal telemetry configuration to JSON", "telemetry", s.Telemetry.Object) - return nil - } - - logger.V(2).Info("marshaled telemetry configuration", "json", string(jsonData)) - - t := &Telemetry{} - // Unmarshal JSON into the provided struct - if err := json.Unmarshal(jsonData, t); err != nil { - logger.Error(err, "failed to unmarshal telemetry configuration, this may indicate invalid configuration", "json", string(jsonData), "originalConfig", s.Telemetry.Object) - return nil - } - - logger.V(2).Info("successfully parsed telemetry configuration", - "metricsLevel", t.Metrics.Level, - "metricsAddress", t.Metrics.Address, - "readersCount", len(t.Metrics.Readers)) - - return t -} diff --git a/apis/v1beta1/config_test.go b/apis/v1beta1/config_test.go index 3a45d24262..9b0866ede2 100644 --- a/apis/v1beta1/config_test.go +++ b/apis/v1beta1/config_test.go @@ -10,14 +10,9 @@ import ( "strings" "testing" - "github.com/go-logr/logr" go_yaml "github.com/goccy/go-yaml" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/utils/ptr" ) func TestConfigFiles(t *testing.T) { @@ -51,37 +46,6 @@ func TestConfigFiles(t *testing.T) { } } -func TestNullObjects(t *testing.T) { - collectorYaml, err := os.ReadFile("./testdata/otelcol-null-values.yaml") - require.NoError(t, err) - - collectorJson, err := go_yaml.YAMLToJSON(collectorYaml) - require.NoError(t, err) - - cfg := &Config{} - err = json.Unmarshal(collectorJson, cfg) - require.NoError(t, err) - - nullObjects := cfg.NullObjects() - assert.Equal(t, []string{"connectors.spanmetrics:", "exporters.otlp.endpoint:", "extensions.health_check:", "processors.batch:", "receivers.otlp.protocols.grpc:", "receivers.otlp.protocols.http:"}, nullObjects) -} - -func TestNullObjects_issue_3445(t *testing.T) { - collectorYaml, err := os.ReadFile("./testdata/issue-3452.yaml") - require.NoError(t, err) - - collectorJson, err := go_yaml.YAMLToJSON(collectorYaml) - require.NoError(t, err) - - cfg := &Config{} - err = json.Unmarshal(collectorJson, cfg) - require.NoError(t, err) - - _, err = cfg.ApplyDefaults(logr.Discard()) - require.NoError(t, err) - assert.Empty(t, cfg.NullObjects()) -} - func TestConfigFiles_go_yaml(t *testing.T) { files, err := os.ReadDir("./testdata") require.NoError(t, err) @@ -108,1379 +72,6 @@ func TestConfigFiles_go_yaml(t *testing.T) { } } -func TestNullObjects_go_yaml(t *testing.T) { - collectorYaml, err := os.ReadFile("./testdata/otelcol-null-values.yaml") - require.NoError(t, err) - - cfg := &Config{} - err = go_yaml.Unmarshal(collectorYaml, cfg) - require.NoError(t, err) - - nullObjects := cfg.NullObjects() - assert.Equal(t, []string{"connectors.spanmetrics:", "exporters.otlp.endpoint:", "extensions.health_check:", "processors.batch:", "receivers.otlp.protocols.grpc:", "receivers.otlp.protocols.http:"}, nullObjects) -} - -func TestConfigYaml(t *testing.T) { - cfg := &Config{ - Receivers: AnyConfig{ - Object: map[string]any{ - "otlp": nil, - }, - }, - Processors: &AnyConfig{ - Object: map[string]any{ - "modify_2000": "enabled", - }, - }, - Exporters: AnyConfig{ - Object: map[string]any{ - "otlp/exporter": nil, - }, - }, - Connectors: &AnyConfig{ - Object: map[string]any{ - "con": "magic", - }, - }, - Extensions: &AnyConfig{ - Object: map[string]any{ - "addon": "option1", - }, - }, - Service: Service{ - Extensions: []string{"addon"}, - Telemetry: &AnyConfig{ - Object: map[string]any{ - "insights": "yeah!", - }, - }, - Pipelines: map[string]*Pipeline{ - "traces": { - Receivers: []string{"otlp"}, - Processors: []string{"modify_2000"}, - Exporters: []string{"otlp/exporter", "con"}, - }, - }, - }, - } - yamlCollector, err := cfg.Yaml() - require.NoError(t, err) - - const expected = `receivers: - otlp: null -exporters: - otlp/exporter: null -processors: - modify_2000: enabled -connectors: - con: magic -extensions: - addon: option1 -service: - extensions: - - addon - telemetry: - insights: yeah! - pipelines: - traces: - exporters: - - otlp/exporter - - con - processors: - - modify_2000 - receivers: - - otlp -` - - assert.Equal(t, expected, yamlCollector) -} - -func TestGetTelemetryFromYAML(t *testing.T) { - collectorYaml, err := os.ReadFile("./testdata/otelcol-demo.yaml") - require.NoError(t, err) - - cfg := &Config{} - err = go_yaml.Unmarshal(collectorYaml, cfg) - require.NoError(t, err) - telemetry := &Telemetry{ - Metrics: MetricsConfig{ - Level: "detailed", - Address: "0.0.0.0:8888", - }, - } - logger := logr.Discard() - assert.Equal(t, telemetry, cfg.Service.GetTelemetry(logger)) -} - -func TestGetTelemetryFromYAMLIsNil(t *testing.T) { - collectorYaml, err := os.ReadFile("./testdata/otelcol-couchbase.yaml") - require.NoError(t, err) - - cfg := &Config{} - err = go_yaml.Unmarshal(collectorYaml, cfg) - require.NoError(t, err) - logger := logr.Discard() - assert.Nil(t, cfg.Service.GetTelemetry(logger)) -} - -func TestConfigMetricsEndpoint(t *testing.T) { - for _, tt := range []struct { - desc string - expectedAddr string - expectedPort int32 - expectedErr bool - config Service - }{ - { - desc: "custom port", - expectedAddr: "localhost", - expectedPort: 9090, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "localhost:9090", - }, - }, - }, - }, - }, - { - desc: "custom port ipv6", - expectedAddr: "[::]", - expectedPort: 9090, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "[::]:9090", - }, - }, - }, - }, - }, - { - desc: "missing port", - expectedAddr: "localhost", - expectedPort: 8888, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "localhost", - }, - }, - }, - }, - }, - { - desc: "missing port ipv6", - expectedAddr: "[::]", - expectedPort: 8888, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "[::]", - }, - }, - }, - }, - }, - { - desc: "env var and missing port", - expectedAddr: "${env:POD_IP}", - expectedPort: 8888, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "${env:POD_IP}", - }, - }, - }, - }, - }, - { - desc: "env var and missing port ipv6", - expectedAddr: "[${env:POD_IP}]", - expectedPort: 8888, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "[${env:POD_IP}]", - }, - }, - }, - }, - }, - { - desc: "env var and with port", - expectedAddr: "${POD_IP}", - expectedPort: 1234, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "${POD_IP}:1234", - }, - }, - }, - }, - }, - { - desc: "env var and with port ipv6", - expectedAddr: "[${POD_IP}]", - expectedPort: 1234, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "[${POD_IP}]:1234", - }, - }, - }, - }, - }, - { - desc: "port is env var", - expectedErr: true, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "localhost:${env:POD_PORT}", - }, - }, - }, - }, - }, - { - desc: "port is env var ipv6", - expectedErr: true, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "[::]:${env:POD_PORT}", - }, - }, - }, - }, - }, - { - desc: "missing address", - expectedAddr: "0.0.0.0", - expectedPort: 8888, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "level": "detailed", - }, - }, - }, - }, - }, - { - desc: "missing metrics", - expectedAddr: "0.0.0.0", - expectedPort: 8888, - config: Service{ - Telemetry: &AnyConfig{}, - }, - }, - { - desc: "missing telemetry", - expectedAddr: "0.0.0.0", - expectedPort: 8888, - }, - { - desc: "configured telemetry", - expectedAddr: "1.2.3.4", - expectedPort: 4567, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "1.2.3.4:4567", - }, - }, - }, - }, - }, - { - desc: "derive from readers prometheus host+port", - expectedAddr: "0.0.0.0", - expectedPort: 8889, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "level": "detailed", - "readers": []any{ - map[string]any{ - "pull": map[string]any{ - "exporter": map[string]any{ - "prometheus": map[string]any{ - "host": "0.0.0.0", - "port": 8889, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - { - desc: "derive from readers prometheus port only (default host)", - expectedAddr: "0.0.0.0", - expectedPort: 8899, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "readers": []any{ - map[string]any{ - "pull": map[string]any{ - "exporter": map[string]any{ - "prometheus": map[string]any{ - "port": 8899, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - { - desc: "derive from readers prometheus host only (default port)", - expectedAddr: "127.0.0.1", - expectedPort: 8888, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "readers": []any{ - map[string]any{ - "pull": map[string]any{ - "exporter": map[string]any{ - "prometheus": map[string]any{ - "host": "127.0.0.1", - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - { - desc: "readers takes precedence over address", - expectedAddr: "0.0.0.0", - expectedPort: 8889, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "address": "1.2.3.4:4567", - "readers": []any{ - map[string]any{ - "pull": map[string]any{ - "exporter": map[string]any{ - "prometheus": map[string]any{ - "host": "0.0.0.0", - "port": 8889, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - { - desc: "readers present but no prometheus -> defaults", - expectedAddr: "0.0.0.0", - expectedPort: 8888, - config: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "readers": []any{ - map[string]any{ - "pull": map[string]any{ - "exporter": map[string]any{ - "otlp": map[string]any{ - "protocols": map[string]any{ - "http": map[string]any{ - "endpoint": "0.0.0.0:19001", - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - } { - t.Run(tt.desc, func(t *testing.T) { - // these are acceptable failures, we return to the collector's default metric port - addr, port, err := tt.config.MetricsEndpoint(logr.Discard()) - if tt.expectedErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - assert.Equal(t, tt.expectedAddr, addr) - assert.Equal(t, tt.expectedPort, port) - }) - } -} - -func TestConfig_GetEnabledComponents(t *testing.T) { - tests := []struct { - name string - file string - want map[ComponentKind]map[string]any - }{ - { - name: "connectors", - file: "testdata/otelcol-connectors.yaml", - want: map[ComponentKind]map[string]any{ - KindReceiver: { - "foo": struct{}{}, - "count": struct{}{}, - }, - KindProcessor: {}, - KindExporter: { - "bar": struct{}{}, - "count": struct{}{}, - }, - KindExtension: {}, - }, - }, - { - name: "couchbase", - file: "testdata/otelcol-couchbase.yaml", - want: map[ComponentKind]map[string]any{ - KindReceiver: { - "prometheus/couchbase": struct{}{}, - }, - KindProcessor: { - "filter/couchbase": struct{}{}, - "metricstransform/couchbase": struct{}{}, - "transform/couchbase": struct{}{}, - }, - KindExporter: { - "prometheus": struct{}{}, - }, - KindExtension: {}, - }, - }, - { - name: "demo", - file: "testdata/otelcol-demo.yaml", - want: map[ComponentKind]map[string]any{ - KindReceiver: { - "otlp": struct{}{}, - }, - KindProcessor: { - "memory_limiter": struct{}{}, - }, - KindExporter: { - "debug": struct{}{}, - "zipkin": struct{}{}, - "otlp": struct{}{}, - "prometheus": struct{}{}, - }, - KindExtension: { - "health_check": struct{}{}, - "pprof": struct{}{}, - "zpages": struct{}{}, - }, - }, - }, - { - name: "extensions", - file: "testdata/otelcol-extensions.yaml", - want: map[ComponentKind]map[string]any{ - KindReceiver: { - "otlp": struct{}{}, - }, - KindProcessor: {}, - KindExporter: { - "otlp/auth": struct{}{}, - }, - KindExtension: { - "oauth2client": struct{}{}, - }, - }, - }, - { - name: "filelog", - file: "testdata/otelcol-filelog.yaml", - want: map[ComponentKind]map[string]any{ - KindReceiver: { - "filelog": struct{}{}, - }, - KindProcessor: {}, - KindExporter: { - "debug": struct{}{}, - }, - KindExtension: {}, - }, - }, - { - name: "null", - file: "testdata/otelcol-null-values.yaml", - want: map[ComponentKind]map[string]any{ - KindReceiver: {}, - KindProcessor: {}, - KindExporter: {}, - KindExtension: {}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - collectorYaml, err := os.ReadFile(tt.file) - require.NoError(t, err) - - c := &Config{} - err = go_yaml.Unmarshal(collectorYaml, c) - require.NoError(t, err) - assert.Equalf(t, tt.want, c.GetEnabledComponents(), "GetEnabledComponents()") - }) - } -} - -func TestConfig_getEnvironmentVariablesForComponentKinds(t *testing.T) { - tests := []struct { - name string - config *Config - componentKinds []ComponentKind - envVarsLen int - }{ - { - name: "no env vars", - config: &Config{ - Receivers: AnyConfig{ - Object: map[string]any{ - "myreceiver": map[string]any{ - "env": "test", - }, - }, - }, - Service: Service{ - Pipelines: map[string]*Pipeline{ - "test": { - Receivers: []string{"myreceiver"}, - }, - }, - }, - }, - componentKinds: []ComponentKind{KindReceiver}, - envVarsLen: 0, - }, - { - name: "kubeletstats env vars", - config: &Config{ - Receivers: AnyConfig{ - Object: map[string]any{ - "kubeletstats": map[string]any{}, - }, - }, - Service: Service{ - Pipelines: map[string]*Pipeline{ - "test": { - Receivers: []string{"kubeletstats"}, - }, - }, - }, - }, - componentKinds: []ComponentKind{KindReceiver}, - envVarsLen: 1, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - logger := logr.Discard() - envVars, err := tt.config.GetEnvironmentVariables(logger) - - assert.NoError(t, err) - assert.Len(t, envVars, tt.envVarsLen) - }) - } -} - -func TestConfig_GetReceiverPorts(t *testing.T) { - tests := []struct { - name string - file string - want []v1.ServicePort - wantErr bool - }{ - { - name: "k8sevents", - file: "testdata/otelcol-k8sevents.yaml", - want: []v1.ServicePort{ - { - Name: "otlp-http", - Protocol: "", - AppProtocol: ptr.To("http"), - Port: 4318, - TargetPort: intstr.FromInt32(4318), - }, - }, - wantErr: false, // Silently fail - }, - { - name: "connectors", - file: "testdata/otelcol-connectors.yaml", - want: nil, - wantErr: false, // Silently fail - }, - { - name: "couchbase", - file: "testdata/otelcol-couchbase.yaml", - want: nil, // Couchbase uses a prometheus scraper, no ports should be opened - }, - { - name: "demo", - file: "testdata/otelcol-demo.yaml", - want: []v1.ServicePort{ - { - Name: "otlp-grpc", - Protocol: "", - AppProtocol: ptr.To("grpc"), - Port: 4317, - TargetPort: intstr.FromInt32(4317), - }, - }, - }, - { - name: "extensions", - file: "testdata/otelcol-extensions.yaml", - want: []v1.ServicePort{ - { - Name: "otlp-grpc", - Protocol: "", - AppProtocol: ptr.To("grpc"), - Port: 4317, - TargetPort: intstr.FromInt32(4317), - }, - }, - }, - { - name: "filelog", - file: "testdata/otelcol-filelog.yaml", - want: nil, - }, - { - name: "null", - file: "testdata/otelcol-null-values.yaml", - want: nil, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - collectorYaml, err := os.ReadFile(tt.file) - require.NoError(t, err) - - c := &Config{} - err = go_yaml.Unmarshal(collectorYaml, c) - require.NoError(t, err) - ports, err := c.GetReceiverPorts(logr.Discard()) - if tt.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - assert.Equalf(t, tt.want, ports, "GetReceiverPorts()") - }) - } -} - -func TestConfig_GetExporterPorts(t *testing.T) { - tests := []struct { - name string - file string - want []v1.ServicePort - wantErr bool - }{ - { - name: "connectors", - file: "testdata/otelcol-connectors.yaml", - want: nil, - wantErr: false, - }, - { - name: "couchbase", - file: "testdata/otelcol-couchbase.yaml", - want: []v1.ServicePort{ - { - Name: "prometheus", - Port: 9123, - }, - }, - }, - { - name: "demo", - file: "testdata/otelcol-demo.yaml", - want: []v1.ServicePort{ - { - Name: "prometheus", - Port: 8889, - }, - }, - }, - { - name: "extensions", - file: "testdata/otelcol-extensions.yaml", - want: nil, - }, - { - name: "filelog", - file: "testdata/otelcol-filelog.yaml", - want: nil, - }, - { - name: "null", - file: "testdata/otelcol-null-values.yaml", - want: nil, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - collectorYaml, err := os.ReadFile(tt.file) - require.NoError(t, err) - - c := &Config{} - err = go_yaml.Unmarshal(collectorYaml, c) - require.NoError(t, err) - ports, err := c.GetExporterPorts(logr.Discard()) - if tt.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - assert.ElementsMatchf(t, tt.want, ports, "GetReceiverPorts()") - }) - } -} - -func TestConfig_GetLivenessProbe(t *testing.T) { - tests := []struct { - name string - config *Config - wantProbe *v1.Probe - wantErr bool - }{ - { - name: "nil extensions should return nil", - config: &Config{ - Extensions: nil, - Service: Service{ - Extensions: []string{}, - }, - }, - wantProbe: nil, - }, - { - name: "nil extensions with health_check in service extensions should return nil", - config: &Config{ - Extensions: nil, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: nil, - }, - { - name: "empty extensions should return nil", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{}, - }, - Service: Service{ - Extensions: []string{}, - }, - }, - wantProbe: nil, - }, - { - name: "empty extensions with health_check in service extensions should return probe", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{}, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension enabled should return probe", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{}, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension with custom path", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{ - "path": "/healthz", - }, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/healthz", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension with custom endpoint port", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{ - "endpoint": "0.0.0.0:8080", - }, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(8080), - }, - }, - }, - }, - { - name: "extension without liveness probe should return nil", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "jaeger_query": map[string]any{}, - }, - }, - Service: Service{ - Extensions: []string{"jaeger_query"}, - }, - }, - wantProbe: nil, - }, - { - name: "invalid health_check config should return error", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": func() {}, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := tt.config.GetLivenessProbe(logr.Discard()) - if (err != nil) != tt.wantErr { - t.Errorf("Config.GetLivenessProbe() error = %v, wantErr %v", err, tt.wantErr) - return - } - if diff := cmp.Diff(tt.wantProbe, got); diff != "" { - t.Errorf("Config.GetLivenessProbe() mismatch (-want +got):\n%s", diff) - } - }) - } -} - -func TestConfig_GetReadinessProbe(t *testing.T) { - tests := []struct { - name string - config *Config - wantProbe *v1.Probe - wantErr bool - }{ - { - name: "nil extensions should return nil", - config: &Config{ - Extensions: nil, - Service: Service{ - Extensions: []string{}, - }, - }, - wantProbe: nil, - }, - { - name: "nil extensions with health_check in service extensions should return nil", - config: &Config{ - Extensions: nil, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: nil, - }, - { - name: "empty extensions should return nil", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{}, - }, - Service: Service{ - Extensions: []string{}, - }, - }, - wantProbe: nil, - }, - { - name: "empty extensions with health_check in service extensions should return probe", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{}, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension enabled should return probe", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{}, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension with custom path", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{ - "path": "/healthz", - }, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/healthz", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension with custom endpoint port", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{ - "endpoint": "0.0.0.0:8080", - }, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(8080), - }, - }, - }, - }, - { - name: "extension without readiness probe should return nil", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "jaeger_query": map[string]any{}, - }, - }, - Service: Service{ - Extensions: []string{"jaeger_query"}, - }, - }, - wantProbe: nil, - }, - { - name: "invalid health_check config should return error", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": func() {}, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := tt.config.GetReadinessProbe(logr.Discard()) - if (err != nil) != tt.wantErr { - t.Errorf("Config.GetReadinessProbe() error = %v, wantErr %v", err, tt.wantErr) - return - } - if diff := cmp.Diff(tt.wantProbe, got); diff != "" { - t.Errorf("Config.GetReadinessProbe() mismatch (-want +got):\n%s", diff) - } - }) - } -} - -func TestConfig_GetStartupProbe(t *testing.T) { - tests := []struct { - name string - config *Config - wantProbe *v1.Probe - wantErr bool - }{ - { - name: "nil extensions should return nil", - config: &Config{ - Extensions: nil, - Service: Service{ - Extensions: []string{}, - }, - }, - wantProbe: nil, - }, - { - name: "nil extensions with health_check in service extensions should return nil", - config: &Config{ - Extensions: nil, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: nil, - }, - { - name: "empty extensions should return nil", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{}, - }, - Service: Service{ - Extensions: []string{}, - }, - }, - wantProbe: nil, - }, - { - name: "empty extensions with health_check in service extensions should return probe", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{}, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension enabled should return probe", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{}, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension with custom path", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{ - "path": "/healthz", - }, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/healthz", - Port: intstr.FromInt32(13133), - }, - }, - }, - }, - { - name: "health_check extension with custom endpoint port", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": map[string]any{ - "endpoint": "0.0.0.0:8080", - }, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantProbe: &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: "/", - Port: intstr.FromInt32(8080), - }, - }, - }, - }, - { - name: "extension without startup probe should return nil", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "jaeger_query": map[string]any{}, - }, - }, - Service: Service{ - Extensions: []string{"jaeger_query"}, - }, - }, - wantProbe: nil, - }, - { - name: "invalid health_check config should return error", - config: &Config{ - Extensions: &AnyConfig{ - Object: map[string]any{ - "health_check": func() {}, - }, - }, - Service: Service{ - Extensions: []string{"health_check"}, - }, - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := tt.config.GetStartupProbe(logr.Discard()) - if (err != nil) != tt.wantErr { - t.Errorf("Config.GetStartupProbe() error = %v, wantErr %v", err, tt.wantErr) - return - } - if diff := cmp.Diff(tt.wantProbe, got); diff != "" { - t.Errorf("Config.GetStartupProbe() mismatch (-want +got):\n%s", diff) - } - }) - } -} - -func TestTelemetryLogsPreservedWithMetrics(t *testing.T) { - // Test case where logs configuration exists and metrics is added - cfg := &Config{ - Service: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "logs": map[string]any{ - "level": "debug", - }, - }, - }, - }, - } - - expected := &Config{ - Service: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "logs": map[string]any{ - "level": "debug", - }, - "metrics": map[string]any{ - "readers": []any{ - map[string]any{ - "pull": map[string]any{ - "exporter": map[string]any{ - "prometheus": map[string]any{ - "host": "0.0.0.0", - "port": int32(8888), - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - _, err := cfg.Service.ApplyDefaults(logr.Discard()) - require.NoError(t, err) - - logger := logr.Discard() - telemetry := cfg.Service.GetTelemetry(logger) - require.NotNil(t, telemetry) - require.Equal(t, expected, cfg) -} - -func TestTelemetryIncompleteConfigAppliesDefaults(t *testing.T) { - cfg := &Config{ - Service: Service{ - Telemetry: &AnyConfig{ - Object: map[string]any{ - "metrics": map[string]any{ - "level": "basic", - "readers": []any{ - map[string]any{ - "periodic": map[string]any{ - "exporter": map[string]any{ - "otlp": map[string]any{ - "endpoint": "otlp_host:4317", - // Missing protocol - makes this invalid - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - _, err := cfg.Service.ApplyDefaults(logr.Discard()) - require.NoError(t, err) - - logger := logr.Discard() - telemetry := cfg.Service.GetTelemetry(logger) - require.NotNil(t, telemetry) - - require.Len(t, telemetry.Metrics.Readers, 1) - - require.NotNil(t, telemetry.Metrics.Readers[0].Pull) - require.NotNil(t, telemetry.Metrics.Readers[0].Pull.Exporter.Prometheus) - require.Equal(t, "0.0.0.0", *telemetry.Metrics.Readers[0].Pull.Exporter.Prometheus.Host) - require.Equal(t, 8888, *telemetry.Metrics.Readers[0].Pull.Exporter.Prometheus.Port) -} - func TestAnyConfigDeepCopyInto_NestedMapIndependence(t *testing.T) { src := AnyConfig{Object: map[string]any{ "prometheus": map[string]any{ diff --git a/internal/manifests/collector/config_replace.go b/internal/manifests/collector/config_replace.go index 2dfc636296..48b4b1d86d 100644 --- a/internal/manifests/collector/config_replace.go +++ b/internal/manifests/collector/config_replace.go @@ -11,12 +11,13 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) func ReplaceConfig(otelcol v1beta1.OpenTelemetryCollector, targetAllocator *v1alpha1.TargetAllocator, options ...ta.TAOption) (string, error) { collectorSpec := otelcol.Spec taEnabled := targetAllocator != nil - cfgStr, err := collectorSpec.Config.Yaml() + cfgStr, err := otelconfig.Yaml(&collectorSpec.Config) if err != nil { return "", err } diff --git a/internal/manifests/collector/configmap.go b/internal/manifests/collector/configmap.go index cdc5a156e1..27b58d2831 100644 --- a/internal/manifests/collector/configmap.go +++ b/internal/manifests/collector/configmap.go @@ -15,6 +15,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" "github.com/open-telemetry/opentelemetry-operator/pkg/constants" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -27,7 +28,7 @@ func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) { // This ensures collectors get updated TLS settings when the operator restarts // after a cluster TLS profile change, without requiring CR updates. if params.Config.Internal.OperandTLSProfile != nil { - _, err := otelCol.Spec.Config.ApplyDefaults(params.Log, components.WithTLSProfile(params.Config.Internal.OperandTLSProfile)) + _, err := otelconfig.ApplyDefaults(&otelCol.Spec.Config, params.Log, components.WithTLSProfile(params.Config.Internal.OperandTLSProfile)) if err != nil { params.Log.Error(err, "failed to apply TLS defaults to collector config") return nil, err diff --git a/internal/manifests/collector/container.go b/internal/manifests/collector/container.go index 5013ea7000..4e1d6f1439 100644 --- a/internal/manifests/collector/container.go +++ b/internal/manifests/collector/container.go @@ -18,6 +18,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/certmanager" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" "github.com/open-telemetry/opentelemetry-operator/pkg/constants" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -93,19 +94,19 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme } } - livenessProbe, livenessProbeErr := otelcol.Spec.Config.GetLivenessProbe(logger) + livenessProbe, livenessProbeErr := otelconfig.GetLivenessProbe(&otelcol.Spec.Config, logger) if livenessProbeErr != nil { logger.Error(livenessProbeErr, "cannot create liveness probe.") } else { defaultProbeSettings(livenessProbe, otelcol.Spec.LivenessProbe) } - readinessProbe, readinessProbeErr := otelcol.Spec.Config.GetReadinessProbe(logger) + readinessProbe, readinessProbeErr := otelconfig.GetReadinessProbe(&otelcol.Spec.Config, logger) if readinessProbeErr != nil { logger.Error(readinessProbeErr, "cannot create readiness probe.") } else { defaultProbeSettings(readinessProbe, otelcol.Spec.ReadinessProbe) } - startupProbe, startupProbeErr := otelcol.Spec.Config.GetStartupProbe(logger) + startupProbe, startupProbeErr := otelconfig.GetStartupProbe(&otelcol.Spec.Config, logger) if startupProbeErr != nil { logger.Error(startupProbeErr, "cannot create startup probe.") } else { @@ -132,7 +133,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme func getConfigContainerPorts(logger logr.Logger, conf v1beta1.Config) ([]corev1.ContainerPort, error) { ports := []corev1.ContainerPort{} - ps, err := conf.GetAllPorts(logger) + ps, err := otelconfig.GetAllPorts(&conf, logger) if err != nil { return ports, err } @@ -158,7 +159,7 @@ func getConfigContainerPorts(logger logr.Logger, conf v1beta1.Config) ([]corev1. } } - _, metricsPort, err := conf.Service.MetricsEndpoint(logger) + _, metricsPort, err := otelconfig.MetricsEndpoint(&conf.Service, logger) if err != nil { logger.Info("couldn't determine metrics port from configuration, using 8888 default value", "error", err) metricsPort = 8888 @@ -357,7 +358,7 @@ func getInferredContainerEnvVars(otelcol v1beta1.OpenTelemetryCollector, logger ) } - if configEnvVars, err := otelcol.Spec.Config.GetEnvironmentVariables(logger); err != nil { + if configEnvVars, err := otelconfig.GetEnvironmentVariables(&otelcol.Spec.Config, logger); err != nil { logger.Error(err, "could not get the environment variables from the config") } else { envVars = append(envVars, configEnvVars...) diff --git a/internal/manifests/collector/ingress.go b/internal/manifests/collector/ingress.go index 9e7ce8f9bd..b02dbd83ab 100644 --- a/internal/manifests/collector/ingress.go +++ b/internal/manifests/collector/ingress.go @@ -15,6 +15,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) func Ingress(params manifests.Params) (*networkingv1.Ingress, error) { @@ -124,7 +125,7 @@ func createSubdomainIngressRules(otelcol, hostname string, ports []corev1.Servic } func servicePortsFromCfg(logger logr.Logger, otelcol v1beta1.OpenTelemetryCollector) ([]corev1.ServicePort, error) { - ports, err := otelcol.Spec.Config.GetReceiverPorts(logger) + ports, err := otelconfig.GetReceiverPorts(&otelcol.Spec.Config, logger) if err != nil { logger.Error(err, "couldn't build the ingress for this instance") return nil, err diff --git a/internal/manifests/collector/podmonitor.go b/internal/manifests/collector/podmonitor.go index df15c3bfa1..0dee543a1a 100644 --- a/internal/manifests/collector/podmonitor.go +++ b/internal/manifests/collector/podmonitor.go @@ -15,6 +15,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) var monitoringPortName = "monitoring" @@ -56,7 +57,7 @@ func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) { } func metricsEndpointsFromConfig(logger logr.Logger, otelcol v1beta1.OpenTelemetryCollector) []monitoringv1.PodMetricsEndpoint { - exporterPorts, err := otelcol.Spec.Config.GetExporterPorts(logger) + exporterPorts, err := otelconfig.GetExporterPorts(&otelcol.Spec.Config, logger) if err != nil { logger.Error(err, "couldn't build endpoints to podMonitors from configuration") return []monitoringv1.PodMetricsEndpoint{} diff --git a/internal/manifests/collector/rbac.go b/internal/manifests/collector/rbac.go index ad3ec07aab..544b9fc6a7 100644 --- a/internal/manifests/collector/rbac.go +++ b/internal/manifests/collector/rbac.go @@ -13,11 +13,12 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" "github.com/open-telemetry/opentelemetry-operator/internal/rbac" ) func ClusterRole(params manifests.Params) (*rbacv1.ClusterRole, error) { - rules, err := params.OtelCol.Spec.Config.GetAllRbacRules(params.Log) + rules, err := otelconfig.GetAllRbacRules(¶ms.OtelCol.Spec.Config, params.Log) if err != nil { return nil, err } else if len(rules) == 0 { @@ -43,7 +44,7 @@ func ClusterRole(params manifests.Params) (*rbacv1.ClusterRole, error) { } func ClusterRoleBinding(params manifests.Params) (*rbacv1.ClusterRoleBinding, error) { - rules, err := params.OtelCol.Spec.Config.GetAllRbacRules(params.Log) + rules, err := otelconfig.GetAllRbacRules(¶ms.OtelCol.Spec.Config, params.Log) if err != nil { return nil, err } else if len(rules) == 0 { @@ -82,7 +83,7 @@ func ClusterRoleBinding(params manifests.Params) (*rbacv1.ClusterRoleBinding, er func CheckRbacRules(params manifests.Params, saName string) ([]string, error) { ctx := context.Background() - rules, err := params.OtelCol.Spec.Config.GetAllRbacRules(params.Log) + rules, err := otelconfig.GetAllRbacRules(¶ms.OtelCol.Spec.Config, params.Log) if err != nil { return nil, err } diff --git a/internal/manifests/collector/service.go b/internal/manifests/collector/service.go index e4bb749b2e..8c50fcb1f6 100644 --- a/internal/manifests/collector/service.go +++ b/internal/manifests/collector/service.go @@ -16,6 +16,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) // headless and monitoring labels are to differentiate the base/headless/monitoring services from the clusterIP service. @@ -71,7 +72,7 @@ func MonitoringService(params manifests.Params) (*corev1.Service, error) { return nil, err } - _, metricsPort, err := params.OtelCol.Spec.Config.Service.MetricsEndpoint(params.Log) + _, metricsPort, err := otelconfig.MetricsEndpoint(¶ms.OtelCol.Spec.Config.Service, params.Log) if err != nil { return nil, err } @@ -107,7 +108,7 @@ func ExtensionService(params manifests.Params) (*corev1.Service, error) { return nil, err } - ports, err := params.OtelCol.Spec.Config.GetExtensionPorts(params.Log) + ports, err := otelconfig.GetExtensionPorts(¶ms.OtelCol.Spec.Config, params.Log) if err != nil { return nil, err } @@ -141,7 +142,7 @@ func Service(params manifests.Params) (*corev1.Service, error) { return nil, err } - ports, err := params.OtelCol.Spec.Config.GetReceiverAndExporterPorts(params.Log) + ports, err := otelconfig.GetReceiverAndExporterPorts(¶ms.OtelCol.Spec.Config, params.Log) if err != nil { return nil, err } diff --git a/internal/manifests/collector/servicemonitor.go b/internal/manifests/collector/servicemonitor.go index 7e5180eaf0..3d024d73c4 100644 --- a/internal/manifests/collector/servicemonitor.go +++ b/internal/manifests/collector/servicemonitor.go @@ -16,6 +16,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) // ServiceMonitor returns the service monitor for the collector. @@ -96,7 +97,7 @@ func shouldCreateServiceMonitor(params manifests.Params) bool { } func endpointsFromConfig(logger logr.Logger, otelcol v1beta1.OpenTelemetryCollector) []monitoringv1.Endpoint { - exporterPorts, err := otelcol.Spec.Config.GetExporterPorts(logger) + exporterPorts, err := otelconfig.GetExporterPorts(&otelcol.Spec.Config, logger) if err != nil { logger.Error(err, "couldn't build service monitors from configuration") return []monitoringv1.Endpoint{} diff --git a/internal/manifests/collector/volume.go b/internal/manifests/collector/volume.go index 75375ebcec..3747c63caa 100644 --- a/internal/manifests/collector/volume.go +++ b/internal/manifests/collector/volume.go @@ -14,6 +14,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -21,7 +22,7 @@ import ( func Volumes(cfg config.Config, otelcol v1beta1.OpenTelemetryCollector) []corev1.Volume { collectorCfg := otelcol.Spec.Config.DeepCopy() if cfg.Internal.OperandTLSProfile != nil { - _, _ = collectorCfg.ApplyDefaults(logr.Discard(), components.WithTLSProfile(cfg.Internal.OperandTLSProfile)) + _, _ = otelconfig.ApplyDefaults(collectorCfg, logr.Discard(), components.WithTLSProfile(cfg.Internal.OperandTLSProfile)) } hash, _ := manifestutils.GetConfigMapSHA(*collectorCfg) configMapName := naming.ConfigMap(otelcol.Name, hash) diff --git a/internal/manifests/targetallocator/configmap.go b/internal/manifests/targetallocator/configmap.go index 8eef95b409..4f7d4f8d83 100644 --- a/internal/manifests/targetallocator/configmap.go +++ b/internal/manifests/targetallocator/configmap.go @@ -17,6 +17,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" "github.com/open-telemetry/opentelemetry-operator/pkg/constants" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -184,7 +185,7 @@ func getScrapeConfigs(taScrapeConfigs []v1beta1.AnyConfig, collectorConfig v1bet scrapeConfigs = append(scrapeConfigs, taScrapeConfigs...) } - configStr, err := collectorConfig.Yaml() + configStr, err := otelconfig.Yaml(&collectorConfig) if err != nil { return nil, err } diff --git a/internal/manifests/targetallocator/configmap_test.go b/internal/manifests/targetallocator/configmap_test.go index 1ce8630012..f9235ca1ca 100644 --- a/internal/manifests/targetallocator/configmap_test.go +++ b/internal/manifests/targetallocator/configmap_test.go @@ -18,6 +18,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/certmanager" "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -617,7 +618,7 @@ func TestGetScrapeConfigsFromOtelConfig(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - configStr, err := testCase.input.Yaml() + configStr, err := otelconfig.Yaml(&testCase.input) require.NoError(t, err) actual, err := getScrapeConfigsFromOtelConfig(configStr) assert.Equal(t, testCase.wantErr, err) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 9471ef82fa..1f79187d10 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -63,7 +63,7 @@ func Bootstrap() (metric.MeterProvider, error) { return sdkmetric.NewMeterProvider(sdkmetric.WithReader(exporter)), err } -func New(prv metric.MeterProvider, ctx context.Context, cl client.Reader) (*Metrics, error) { +func New(ctx context.Context, prv metric.MeterProvider, cl client.Reader) (*Metrics, error) { meter := prv.Meter(meterName) modeCounter, err := meter.Int64UpDownCounter(modeMetricName) if err != nil { diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index d96a779436..ade007d414 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -134,7 +134,7 @@ func TestOTELCollectorCRDMetrics(t *testing.T) { reader := metric.NewManualReader() provider := metric.NewMeterProvider(metric.WithReader(reader)) cl := fake.NewClientBuilder().WithScheme(scheme).Build() - crdMetrics, err := New(provider, context.Background(), cl) + crdMetrics, err := New(context.Background(), provider, cl) assert.NoError(t, err) for _, tt := range tests { @@ -213,7 +213,7 @@ func TestOTELCollectorInitMetrics(t *testing.T) { cl := fake.NewClientBuilder().WithLists(list).WithScheme(scheme).Build() reader := metric.NewManualReader() provider := metric.NewMeterProvider(metric.WithReader(reader)) - _, err = New(provider, context.Background(), cl) + _, err = New(context.Background(), provider, cl) assert.NoError(t, err) rm := metricdata.ResourceMetrics{} diff --git a/internal/otelconfig/config.go b/internal/otelconfig/config.go new file mode 100644 index 0000000000..62f9cf08df --- /dev/null +++ b/internal/otelconfig/config.go @@ -0,0 +1,531 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package otelconfig + +import ( + "bytes" + "encoding/json" + "fmt" + "math" + "slices" + "strings" + + "dario.cat/mergo" + "github.com/go-logr/logr" + go_yaml "github.com/goccy/go-yaml" + otelConfig "go.opentelemetry.io/contrib/otelconf/v0.3.0" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" + "github.com/open-telemetry/opentelemetry-operator/internal/components" + "github.com/open-telemetry/opentelemetry-operator/internal/components/exporters" + "github.com/open-telemetry/opentelemetry-operator/internal/components/extensions" + "github.com/open-telemetry/opentelemetry-operator/internal/components/processors" + "github.com/open-telemetry/opentelemetry-operator/internal/components/receivers" +) + +// GetEnabledComponents constructs a list of enabled components by component type. +func GetEnabledComponents(c *v1beta1.Config) map[v1beta1.ComponentKind]map[string]any { + toReturn := map[v1beta1.ComponentKind]map[string]any{ + v1beta1.KindReceiver: {}, + v1beta1.KindProcessor: {}, + v1beta1.KindExporter: {}, + v1beta1.KindExtension: {}, + } + for _, extension := range c.Service.Extensions { + toReturn[v1beta1.KindExtension][extension] = struct{}{} + } + + for _, pipeline := range c.Service.Pipelines { + if pipeline == nil { + continue + } + for _, componentId := range pipeline.Receivers { + toReturn[v1beta1.KindReceiver][componentId] = struct{}{} + } + for _, componentId := range pipeline.Exporters { + toReturn[v1beta1.KindExporter][componentId] = struct{}{} + } + for _, componentId := range pipeline.Processors { + toReturn[v1beta1.KindProcessor][componentId] = struct{}{} + } + } + for _, componentId := range c.Service.Extensions { + toReturn[v1beta1.KindExtension][componentId] = struct{}{} + } + return toReturn +} + +// getRbacRulesForComponentKinds gets the RBAC Rules for the given ComponentKind(s). +func getRbacRulesForComponentKinds(c *v1beta1.Config, logger logr.Logger, componentKinds ...v1beta1.ComponentKind) ([]rbacv1.PolicyRule, error) { + var rules []rbacv1.PolicyRule + enabledComponents := GetEnabledComponents(c) + for _, componentKind := range componentKinds { + var retriever components.ParserRetriever + var cfg v1beta1.AnyConfig + switch componentKind { + case v1beta1.KindReceiver: + retriever = receivers.ReceiverFor + cfg = c.Receivers + case v1beta1.KindExporter: + retriever = exporters.ParserFor + cfg = c.Exporters + case v1beta1.KindProcessor: + retriever = processors.ProcessorFor + if c.Processors == nil { + cfg = v1beta1.AnyConfig{} + } else { + cfg = *c.Processors + } + case v1beta1.KindExtension: + retriever = extensions.ParserFor + if c.Extensions == nil { + cfg = v1beta1.AnyConfig{} + } else { + cfg = *c.Extensions + } + default: + logger.V(1).Info("unknown component kind", "kind", componentKind) + continue + } + for componentName := range enabledComponents[componentKind] { + parser := retriever(componentName) + parsedRules, err := parser.GetRBACRules(logger, cfg.Object[componentName]) + if err != nil { + return nil, err + } + rules = append(rules, parsedRules...) + } + } + return rules, nil +} + +// getPortsForComponentKinds gets the ports for the given ComponentKind(s). +func getPortsForComponentKinds(c *v1beta1.Config, logger logr.Logger, componentKinds ...v1beta1.ComponentKind) ([]corev1.ServicePort, error) { + var ports []corev1.ServicePort + enabledComponents := GetEnabledComponents(c) + for _, componentKind := range componentKinds { + var retriever components.ParserRetriever + var cfg v1beta1.AnyConfig + switch componentKind { + case v1beta1.KindReceiver: + retriever = receivers.ReceiverFor + cfg = c.Receivers + case v1beta1.KindExporter: + retriever = exporters.ParserFor + cfg = c.Exporters + case v1beta1.KindProcessor: + continue + case v1beta1.KindExtension: + retriever = extensions.ParserFor + if c.Extensions == nil { + cfg = v1beta1.AnyConfig{} + } else { + cfg = *c.Extensions + } + } + for componentName := range enabledComponents[componentKind] { + parser := retriever(componentName) + parsedPorts, err := parser.Ports(logger, componentName, cfg.Object[componentName]) + if err != nil { + return nil, err + } + ports = append(ports, parsedPorts...) + } + } + + slices.SortFunc(ports, func(i, j corev1.ServicePort) int { + return strings.Compare(i.Name, j.Name) + }) + + return ports, nil +} + +// getEnvironmentVariablesForComponentKinds gets the environment variables for the given ComponentKind(s). +func getEnvironmentVariablesForComponentKinds(c *v1beta1.Config, logger logr.Logger, componentKinds ...v1beta1.ComponentKind) ([]corev1.EnvVar, error) { + envVars := []corev1.EnvVar{} + enabledComponents := GetEnabledComponents(c) + for _, componentKind := range componentKinds { + var retriever components.ParserRetriever + var cfg v1beta1.AnyConfig + + switch componentKind { + case v1beta1.KindReceiver: + retriever = receivers.ReceiverFor + cfg = c.Receivers + case v1beta1.KindExporter, v1beta1.KindProcessor, v1beta1.KindExtension: + continue + } + for componentName := range enabledComponents[componentKind] { + parser := retriever(componentName) + parsedEnvVars, err := parser.GetEnvironmentVariables(logger, cfg.Object[componentName]) + if err != nil { + return nil, err + } + envVars = append(envVars, parsedEnvVars...) + } + } + + slices.SortFunc(envVars, func(i, j corev1.EnvVar) int { + return strings.Compare(i.Name, j.Name) + }) + + return envVars, nil +} + +// applyDefaultForComponentKinds applies defaults to the endpoints for the given ComponentKind(s). +// If defaultsCfg.TLSProfile is set, TLS defaults are also applied via the Parser.GetDefaultConfig method. +// Returns a list of events that should be recorded by the caller. +func applyDefaultForComponentKinds(c *v1beta1.Config, logger logr.Logger, parserOpts []components.DefaultOption, componentKinds ...v1beta1.ComponentKind) ([]v1beta1.EventInfo, error) { + events, err := ServiceApplyDefaults(&c.Service, logger) + if err != nil { + return events, err + } + enabledComponents := GetEnabledComponents(c) + for _, componentKind := range componentKinds { + var retriever components.ParserRetriever + var cfg v1beta1.AnyConfig + switch componentKind { + case v1beta1.KindReceiver: + retriever = receivers.ReceiverFor + cfg = c.Receivers + case v1beta1.KindExporter, v1beta1.KindProcessor: + retriever = exporters.ParserFor + cfg = c.Exporters + case v1beta1.KindExtension: + if c.Extensions == nil { + continue + } + retriever = extensions.ParserFor + cfg = *c.Extensions + } + for componentName := range enabledComponents[componentKind] { + parser := retriever(componentName) + componentConf := cfg.Object[componentName] + newCfg, err := parser.GetDefaultConfig(logger, componentConf, parserOpts...) + if err != nil { + return events, err + } + + // We need to ensure we don't remove any fields in defaulting. + mappedCfg, ok := newCfg.(map[string]any) + if !ok || mappedCfg == nil { + logger.V(1).Info("returned default configuration invalid", + "warn", "could not apply component defaults", + "component", componentName, + ) + continue + } + + if componentConf == nil { + componentConf = map[string]any{} + } + if err := mergo.Merge(&mappedCfg, componentConf); err != nil { + return events, err + } + cfg.Object[componentName] = mappedCfg + } + } + + return events, nil +} + +// GetReceiverPorts gets the ports for receivers. +func GetReceiverPorts(c *v1beta1.Config, logger logr.Logger) ([]corev1.ServicePort, error) { + return getPortsForComponentKinds(c, logger, v1beta1.KindReceiver) +} + +// GetExporterPorts gets the ports for exporters. +func GetExporterPorts(c *v1beta1.Config, logger logr.Logger) ([]corev1.ServicePort, error) { + return getPortsForComponentKinds(c, logger, v1beta1.KindExporter) +} + +// GetExtensionPorts gets the ports for extensions. +func GetExtensionPorts(c *v1beta1.Config, logger logr.Logger) ([]corev1.ServicePort, error) { + return getPortsForComponentKinds(c, logger, v1beta1.KindExtension) +} + +// GetReceiverAndExporterPorts gets the ports for receivers and exporters. +func GetReceiverAndExporterPorts(c *v1beta1.Config, logger logr.Logger) ([]corev1.ServicePort, error) { + return getPortsForComponentKinds(c, logger, v1beta1.KindReceiver, v1beta1.KindExporter) +} + +// GetAllPorts gets the ports for all component kinds that expose ports. +func GetAllPorts(c *v1beta1.Config, logger logr.Logger) ([]corev1.ServicePort, error) { + return getPortsForComponentKinds(c, logger, v1beta1.KindReceiver, v1beta1.KindExporter, v1beta1.KindExtension) +} + +// GetEnvironmentVariables gets the environment variables for receivers. +func GetEnvironmentVariables(c *v1beta1.Config, logger logr.Logger) ([]corev1.EnvVar, error) { + return getEnvironmentVariablesForComponentKinds(c, logger, v1beta1.KindReceiver) +} + +// GetAllRbacRules gets the RBAC rules for all component kinds. +func GetAllRbacRules(c *v1beta1.Config, logger logr.Logger) ([]rbacv1.PolicyRule, error) { + return getRbacRulesForComponentKinds(c, logger, v1beta1.KindReceiver, v1beta1.KindExporter, v1beta1.KindProcessor, v1beta1.KindExtension) +} + +// ApplyDefaults applies default configuration values to the collector config. +// Optional DefaultsOption arguments can be provided to customize behavior. +func ApplyDefaults(c *v1beta1.Config, logger logr.Logger, opts ...components.DefaultOption) ([]v1beta1.EventInfo, error) { + return applyDefaultForComponentKinds(c, logger, opts, v1beta1.KindReceiver, v1beta1.KindExporter, v1beta1.KindExtension) +} + +// GetLivenessProbe gets the first enabled liveness probe. There should only ever be one extension enabled +// that provides the hinting for the liveness probe. +func GetLivenessProbe(c *v1beta1.Config, logger logr.Logger) (*corev1.Probe, error) { + if c.Extensions == nil { + return nil, nil + } + + enabledComponents := GetEnabledComponents(c) + for componentName := range enabledComponents[v1beta1.KindExtension] { + parser := extensions.ParserFor(componentName) + if probe, err := parser.GetLivenessProbe(logger, c.Extensions.Object[componentName]); err != nil { + return nil, err + } else if probe != nil { + return probe, nil + } + } + return nil, nil +} + +// GetReadinessProbe gets the first enabled readiness probe. There should only ever be one extension enabled +// that provides the hinting for the readiness probe. +func GetReadinessProbe(c *v1beta1.Config, logger logr.Logger) (*corev1.Probe, error) { + if c.Extensions == nil { + return nil, nil + } + + enabledComponents := GetEnabledComponents(c) + for componentName := range enabledComponents[v1beta1.KindExtension] { + parser := extensions.ParserFor(componentName) + if probe, err := parser.GetReadinessProbe(logger, c.Extensions.Object[componentName]); err != nil { + return nil, err + } else if probe != nil { + return probe, nil + } + } + return nil, nil +} + +// GetStartupProbe gets the first enabled startup probe. There should only ever be one extension enabled +// that provides the hinting for the startup probe. +func GetStartupProbe(c *v1beta1.Config, logger logr.Logger) (*corev1.Probe, error) { + if c.Extensions == nil { + return nil, nil + } + + enabledComponents := GetEnabledComponents(c) + for componentName := range enabledComponents[v1beta1.KindExtension] { + parser := extensions.ParserFor(componentName) + if probe, err := parser.GetStartupProbe(logger, c.Extensions.Object[componentName]); err != nil { + return nil, err + } else if probe != nil { + return probe, nil + } + } + return nil, nil +} + +// Yaml encodes the current object and returns it as a string. +func Yaml(c *v1beta1.Config) (string, error) { + var buf bytes.Buffer + yamlEncoder := go_yaml.NewEncoder(&buf, go_yaml.IndentSequence(true), go_yaml.AutoInt()) + if err := yamlEncoder.Encode(&c); err != nil { + return "", err + } + return buf.String(), nil +} + +// NullObjects returns null objects in the config. +func NullObjects(c *v1beta1.Config) []string { + var nullKeys []string + if nulls := getNullValuedKeys(c.Receivers.Object); len(nulls) > 0 { + nullKeys = append(nullKeys, addPrefix("receivers.", nulls)...) + } + if nulls := getNullValuedKeys(c.Exporters.Object); len(nulls) > 0 { + nullKeys = append(nullKeys, addPrefix("exporters.", nulls)...) + } + if c.Processors != nil { + if nulls := getNullValuedKeys(c.Processors.Object); len(nulls) > 0 { + nullKeys = append(nullKeys, addPrefix("processors.", nulls)...) + } + } + if c.Extensions != nil { + if nulls := getNullValuedKeys(c.Extensions.Object); len(nulls) > 0 { + nullKeys = append(nullKeys, addPrefix("extensions.", nulls)...) + } + } + if c.Connectors != nil { + if nulls := getNullValuedKeys(c.Connectors.Object); len(nulls) > 0 { + nullKeys = append(nullKeys, addPrefix("connectors.", nulls)...) + } + } + // Make the return deterministic. The config uses maps therefore processing order is non-deterministic. + slices.Sort(nullKeys) + return nullKeys +} + +// MetricsEndpoint attempts gets the host and port number from the host address without doing any validation regarding the +// address itself. +// It works even before env var expansion happens, when a simple `net.SplitHostPort` would fail because of the extra colon +// from the env var, i.e. the address looks like "${env:POD_IP}:4317", "${env:POD_IP}", or "${POD_IP}". +// In cases which the port itself is a variable, i.e. "${env:POD_IP}:${env:PORT}", this returns an error. This happens +// because the port is used to generate Service objects and mappings. +func MetricsEndpoint(s *v1beta1.Service, logger logr.Logger) (host string, port int32, err error) { + telemetry := GetTelemetry(s, logger) + if telemetry == nil { + return defaultServiceHost, defaultServicePort, nil + } + + if telemetry.Metrics.Address != "" && len(telemetry.Metrics.Readers) == 0 { + host, port, err := parseAddressEndpoint(telemetry.Metrics.Address) + if err != nil { + return "", 0, err + } + + return host, port, nil + } + + for _, r := range telemetry.Metrics.Readers { + if r.Pull == nil { + continue + } + prom := r.Pull.Exporter.Prometheus + if prom == nil { + continue + } + host := defaultServiceHost + if prom.Host != nil && *prom.Host != "" { + host = *prom.Host + } + port := defaultServicePort + if prom.Port != nil && *prom.Port != 0 { + if *prom.Port < 0 || *prom.Port > math.MaxUint16 { + return "", 0, fmt.Errorf("invalid prometheus metrics port: %d", *prom.Port) + } + port = int32(*prom.Port) + } + return host, port, nil + } + + return defaultServiceHost, defaultServicePort, nil +} + +// ServiceApplyDefaults inserts configuration defaults if it has not been set. +// Returns a list of events that should be recorded by the caller. +func ServiceApplyDefaults(s *v1beta1.Service, logger logr.Logger) ([]v1beta1.EventInfo, error) { + var events []v1beta1.EventInfo + tel := GetTelemetry(s, logger) + + if tel == nil { + logger.V(2).Info("no telemetry configuration parsed, creating default") + tel = &v1beta1.Telemetry{} + s.Telemetry = &v1beta1.AnyConfig{ + Object: map[string]any{}, + } + } + + if tel.Metrics.Address != "" || len(tel.Metrics.Readers) != 0 { + // The user already set the address or the readers, so we don't need to do anything + logger.V(1).Info("telemetry configuration already provided by user, skipping defaults", + "metricsAddress", tel.Metrics.Address, + "readersCount", len(tel.Metrics.Readers)) + return events, nil + } + + logger.V(2).Info("no telemetry readers configuration found, applying default Prometheus endpoint") + + host, port, err := MetricsEndpoint(s, logger) + if err != nil { + logger.Error(err, "failed to determine metrics endpoint for default configuration") + return events, err + } + + reader := AddPrometheusMetricsEndpoint(host, port) + tel.Metrics.Readers = append(tel.Metrics.Readers, reader) + + events = append(events, v1beta1.EventInfo{ + Type: corev1.EventTypeNormal, + Reason: "Spec.Service.Telemetry.DefaultsApplied", + Message: fmt.Sprintf("Applied default Prometheus telemetry configuration (host: %s, port: %d)", host, port), + }) + + telConfig, err := TelemetryToAnyConfig(tel) + if err != nil { + return events, err + } + + if err := mergo.Merge(&s.Telemetry.Object, telConfig.Object); err != nil { + return events, err + } + return events, nil +} + +// AddPrometheusMetricsEndpoint creates a MetricReader with a Prometheus pull exporter. +func AddPrometheusMetricsEndpoint(host string, port int32) otelConfig.MetricReader { + portInt := int(port) + return otelConfig.MetricReader{ + Pull: &otelConfig.PullMetricReader{ + Exporter: otelConfig.PullMetricExporter{ + Prometheus: &otelConfig.Prometheus{ + Host: &host, + Port: &portInt, + }, + }, + }, + } +} + +// GetTelemetry serves as a helper function to access the fields we care about in the underlying telemetry struct. +// This exists to avoid needing to worry extra fields in the telemetry struct. +func GetTelemetry(s *v1beta1.Service, logger logr.Logger) *v1beta1.Telemetry { + if s.Telemetry == nil { + logger.V(2).Info("no spec.service.telemetry configuration found") + return nil + } + + // Convert map to JSON bytes + jsonData, err := json.Marshal(s.Telemetry) + if err != nil { + logger.Error(err, "failed to marshal telemetry configuration to JSON", "telemetry", s.Telemetry.Object) + return nil + } + + logger.V(2).Info("marshaled telemetry configuration", "json", string(jsonData)) + + t := &v1beta1.Telemetry{} + // Unmarshal JSON into the provided struct + if err := json.Unmarshal(jsonData, t); err != nil { + logger.Error(err, "failed to unmarshal telemetry configuration, this may indicate invalid configuration", "json", string(jsonData), "originalConfig", s.Telemetry.Object) + return nil + } + + logger.V(2).Info("successfully parsed telemetry configuration", + "metricsLevel", t.Metrics.Level, + "metricsAddress", t.Metrics.Address, + "readersCount", len(t.Metrics.Readers)) + + return t +} + +// TelemetryToAnyConfig converts the Telemetry struct to an AnyConfig struct. +func TelemetryToAnyConfig(t *v1beta1.Telemetry) (*v1beta1.AnyConfig, error) { + data, err := json.Marshal(t) + if err != nil { + return nil, err + } + var result map[string]any + if err := json.Unmarshal(data, &result); err != nil { + return nil, err + } + + normalizeConfig(result) + + return &v1beta1.AnyConfig{ + Object: result, + }, nil +} diff --git a/internal/otelconfig/config_test.go b/internal/otelconfig/config_test.go new file mode 100644 index 0000000000..ccca956483 --- /dev/null +++ b/internal/otelconfig/config_test.go @@ -0,0 +1,1425 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package otelconfig + +import ( + "encoding/json" + "os" + "testing" + + "github.com/go-logr/logr" + go_yaml "github.com/goccy/go-yaml" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" +) + +func TestNullObjects(t *testing.T) { + collectorYaml, err := os.ReadFile("./testdata/otelcol-null-values.yaml") + require.NoError(t, err) + + collectorJson, err := go_yaml.YAMLToJSON(collectorYaml) + require.NoError(t, err) + + cfg := &v1beta1.Config{} + err = json.Unmarshal(collectorJson, cfg) + require.NoError(t, err) + + nullObjects := NullObjects(cfg) + assert.Equal(t, []string{"connectors.spanmetrics:", "exporters.otlp.endpoint:", "extensions.health_check:", "processors.batch:", "receivers.otlp.protocols.grpc:", "receivers.otlp.protocols.http:"}, nullObjects) +} + +func TestNullObjects_issue_3445(t *testing.T) { + collectorYaml, err := os.ReadFile("./testdata/issue-3452.yaml") + require.NoError(t, err) + + collectorJson, err := go_yaml.YAMLToJSON(collectorYaml) + require.NoError(t, err) + + cfg := &v1beta1.Config{} + err = json.Unmarshal(collectorJson, cfg) + require.NoError(t, err) + + _, err = ApplyDefaults(cfg, logr.Discard()) + require.NoError(t, err) + assert.Empty(t, NullObjects(cfg)) +} + +func TestNullObjects_go_yaml(t *testing.T) { + collectorYaml, err := os.ReadFile("./testdata/otelcol-null-values.yaml") + require.NoError(t, err) + + cfg := &v1beta1.Config{} + err = go_yaml.Unmarshal(collectorYaml, cfg) + require.NoError(t, err) + + nullObjects := NullObjects(cfg) + assert.Equal(t, []string{"connectors.spanmetrics:", "exporters.otlp.endpoint:", "extensions.health_check:", "processors.batch:", "receivers.otlp.protocols.grpc:", "receivers.otlp.protocols.http:"}, nullObjects) +} + +func TestConfigYaml(t *testing.T) { + cfg := &v1beta1.Config{ + Receivers: v1beta1.AnyConfig{ + Object: map[string]any{ + "otlp": nil, + }, + }, + Processors: &v1beta1.AnyConfig{ + Object: map[string]any{ + "modify_2000": "enabled", + }, + }, + Exporters: v1beta1.AnyConfig{ + Object: map[string]any{ + "otlp/exporter": nil, + }, + }, + Connectors: &v1beta1.AnyConfig{ + Object: map[string]any{ + "con": "magic", + }, + }, + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "addon": "option1", + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"addon"}, + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "insights": "yeah!", + }, + }, + Pipelines: map[string]*v1beta1.Pipeline{ + "traces": { + Receivers: []string{"otlp"}, + Processors: []string{"modify_2000"}, + Exporters: []string{"otlp/exporter", "con"}, + }, + }, + }, + } + yamlCollector, err := Yaml(cfg) + require.NoError(t, err) + + const expected = `receivers: + otlp: null +exporters: + otlp/exporter: null +processors: + modify_2000: enabled +connectors: + con: magic +extensions: + addon: option1 +service: + extensions: + - addon + telemetry: + insights: yeah! + pipelines: + traces: + exporters: + - otlp/exporter + - con + processors: + - modify_2000 + receivers: + - otlp +` + + assert.Equal(t, expected, yamlCollector) +} + +func TestGetTelemetryFromYAML(t *testing.T) { + collectorYaml, err := os.ReadFile("./testdata/otelcol-demo.yaml") + require.NoError(t, err) + + cfg := &v1beta1.Config{} + err = go_yaml.Unmarshal(collectorYaml, cfg) + require.NoError(t, err) + telemetry := &v1beta1.Telemetry{ + Metrics: v1beta1.MetricsConfig{ + Level: "detailed", + Address: "0.0.0.0:8888", + }, + } + logger := logr.Discard() + assert.Equal(t, telemetry, GetTelemetry(&cfg.Service, logger)) +} + +func TestGetTelemetryFromYAMLIsNil(t *testing.T) { + collectorYaml, err := os.ReadFile("./testdata/otelcol-couchbase.yaml") + require.NoError(t, err) + + cfg := &v1beta1.Config{} + err = go_yaml.Unmarshal(collectorYaml, cfg) + require.NoError(t, err) + logger := logr.Discard() + assert.Nil(t, GetTelemetry(&cfg.Service, logger)) +} + +func TestConfigMetricsEndpoint(t *testing.T) { + for _, tt := range []struct { + desc string + expectedAddr string + expectedPort int32 + expectedErr bool + config v1beta1.Service + }{ + { + desc: "custom port", + expectedAddr: "localhost", + expectedPort: 9090, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "localhost:9090", + }, + }, + }, + }, + }, + { + desc: "custom port ipv6", + expectedAddr: "[::]", + expectedPort: 9090, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "[::]:9090", + }, + }, + }, + }, + }, + { + desc: "missing port", + expectedAddr: "localhost", + expectedPort: 8888, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "localhost", + }, + }, + }, + }, + }, + { + desc: "missing port ipv6", + expectedAddr: "[::]", + expectedPort: 8888, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "[::]", + }, + }, + }, + }, + }, + { + desc: "env var and missing port", + expectedAddr: "${env:POD_IP}", + expectedPort: 8888, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "${env:POD_IP}", + }, + }, + }, + }, + }, + { + desc: "env var and missing port ipv6", + expectedAddr: "[${env:POD_IP}]", + expectedPort: 8888, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "[${env:POD_IP}]", + }, + }, + }, + }, + }, + { + desc: "env var and with port", + expectedAddr: "${POD_IP}", + expectedPort: 1234, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "${POD_IP}:1234", + }, + }, + }, + }, + }, + { + desc: "env var and with port ipv6", + expectedAddr: "[${POD_IP}]", + expectedPort: 1234, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "[${POD_IP}]:1234", + }, + }, + }, + }, + }, + { + desc: "port is env var", + expectedErr: true, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "localhost:${env:POD_PORT}", + }, + }, + }, + }, + }, + { + desc: "port is env var ipv6", + expectedErr: true, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "[::]:${env:POD_PORT}", + }, + }, + }, + }, + }, + { + desc: "missing address", + expectedAddr: "0.0.0.0", + expectedPort: 8888, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "level": "detailed", + }, + }, + }, + }, + }, + { + desc: "missing metrics", + expectedAddr: "0.0.0.0", + expectedPort: 8888, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{}, + }, + }, + { + desc: "missing telemetry", + expectedAddr: "0.0.0.0", + expectedPort: 8888, + }, + { + desc: "configured telemetry", + expectedAddr: "1.2.3.4", + expectedPort: 4567, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "1.2.3.4:4567", + }, + }, + }, + }, + }, + { + desc: "derive from readers prometheus host+port", + expectedAddr: "0.0.0.0", + expectedPort: 8889, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "level": "detailed", + "readers": []any{ + map[string]any{ + "pull": map[string]any{ + "exporter": map[string]any{ + "prometheus": map[string]any{ + "host": "0.0.0.0", + "port": 8889, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + desc: "derive from readers prometheus port only (default host)", + expectedAddr: "0.0.0.0", + expectedPort: 8899, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "readers": []any{ + map[string]any{ + "pull": map[string]any{ + "exporter": map[string]any{ + "prometheus": map[string]any{ + "port": 8899, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + desc: "derive from readers prometheus host only (default port)", + expectedAddr: "127.0.0.1", + expectedPort: 8888, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "readers": []any{ + map[string]any{ + "pull": map[string]any{ + "exporter": map[string]any{ + "prometheus": map[string]any{ + "host": "127.0.0.1", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + desc: "readers takes precedence over address", + expectedAddr: "0.0.0.0", + expectedPort: 8889, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "address": "1.2.3.4:4567", + "readers": []any{ + map[string]any{ + "pull": map[string]any{ + "exporter": map[string]any{ + "prometheus": map[string]any{ + "host": "0.0.0.0", + "port": 8889, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + desc: "readers present but no prometheus -> defaults", + expectedAddr: "0.0.0.0", + expectedPort: 8888, + config: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "readers": []any{ + map[string]any{ + "pull": map[string]any{ + "exporter": map[string]any{ + "otlp": map[string]any{ + "protocols": map[string]any{ + "http": map[string]any{ + "endpoint": "0.0.0.0:19001", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } { + t.Run(tt.desc, func(t *testing.T) { + // these are acceptable failures, we return to the collector's default metric port + addr, port, err := MetricsEndpoint(&tt.config, logr.Discard()) + if tt.expectedErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.expectedAddr, addr) + assert.Equal(t, tt.expectedPort, port) + }) + } +} + +func TestConfig_GetEnabledComponents(t *testing.T) { + tests := []struct { + name string + file string + want map[v1beta1.ComponentKind]map[string]any + }{ + { + name: "connectors", + file: "testdata/otelcol-connectors.yaml", + want: map[v1beta1.ComponentKind]map[string]any{ + v1beta1.KindReceiver: { + "foo": struct{}{}, + "count": struct{}{}, + }, + v1beta1.KindProcessor: {}, + v1beta1.KindExporter: { + "bar": struct{}{}, + "count": struct{}{}, + }, + v1beta1.KindExtension: {}, + }, + }, + { + name: "couchbase", + file: "testdata/otelcol-couchbase.yaml", + want: map[v1beta1.ComponentKind]map[string]any{ + v1beta1.KindReceiver: { + "prometheus/couchbase": struct{}{}, + }, + v1beta1.KindProcessor: { + "filter/couchbase": struct{}{}, + "metricstransform/couchbase": struct{}{}, + "transform/couchbase": struct{}{}, + }, + v1beta1.KindExporter: { + "prometheus": struct{}{}, + }, + v1beta1.KindExtension: {}, + }, + }, + { + name: "demo", + file: "testdata/otelcol-demo.yaml", + want: map[v1beta1.ComponentKind]map[string]any{ + v1beta1.KindReceiver: { + "otlp": struct{}{}, + }, + v1beta1.KindProcessor: { + "memory_limiter": struct{}{}, + }, + v1beta1.KindExporter: { + "debug": struct{}{}, + "zipkin": struct{}{}, + "otlp": struct{}{}, + "prometheus": struct{}{}, + }, + v1beta1.KindExtension: { + "health_check": struct{}{}, + "pprof": struct{}{}, + "zpages": struct{}{}, + }, + }, + }, + { + name: "extensions", + file: "testdata/otelcol-extensions.yaml", + want: map[v1beta1.ComponentKind]map[string]any{ + v1beta1.KindReceiver: { + "otlp": struct{}{}, + }, + v1beta1.KindProcessor: {}, + v1beta1.KindExporter: { + "otlp/auth": struct{}{}, + }, + v1beta1.KindExtension: { + "oauth2client": struct{}{}, + }, + }, + }, + { + name: "filelog", + file: "testdata/otelcol-filelog.yaml", + want: map[v1beta1.ComponentKind]map[string]any{ + v1beta1.KindReceiver: { + "filelog": struct{}{}, + }, + v1beta1.KindProcessor: {}, + v1beta1.KindExporter: { + "debug": struct{}{}, + }, + v1beta1.KindExtension: {}, + }, + }, + { + name: "null", + file: "testdata/otelcol-null-values.yaml", + want: map[v1beta1.ComponentKind]map[string]any{ + v1beta1.KindReceiver: {}, + v1beta1.KindProcessor: {}, + v1beta1.KindExporter: {}, + v1beta1.KindExtension: {}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + collectorYaml, err := os.ReadFile(tt.file) + require.NoError(t, err) + + c := &v1beta1.Config{} + err = go_yaml.Unmarshal(collectorYaml, c) + require.NoError(t, err) + assert.Equalf(t, tt.want, GetEnabledComponents(c), "GetEnabledComponents()") + }) + } +} + +func TestConfig_getEnvironmentVariablesForComponentKinds(t *testing.T) { + tests := []struct { + name string + config *v1beta1.Config + componentKinds []v1beta1.ComponentKind + envVarsLen int + }{ + { + name: "no env vars", + config: &v1beta1.Config{ + Receivers: v1beta1.AnyConfig{ + Object: map[string]any{ + "myreceiver": map[string]any{ + "env": "test", + }, + }, + }, + Service: v1beta1.Service{ + Pipelines: map[string]*v1beta1.Pipeline{ + "test": { + Receivers: []string{"myreceiver"}, + }, + }, + }, + }, + componentKinds: []v1beta1.ComponentKind{v1beta1.KindReceiver}, + envVarsLen: 0, + }, + { + name: "kubeletstats env vars", + config: &v1beta1.Config{ + Receivers: v1beta1.AnyConfig{ + Object: map[string]any{ + "kubeletstats": map[string]any{}, + }, + }, + Service: v1beta1.Service{ + Pipelines: map[string]*v1beta1.Pipeline{ + "test": { + Receivers: []string{"kubeletstats"}, + }, + }, + }, + }, + componentKinds: []v1beta1.ComponentKind{v1beta1.KindReceiver}, + envVarsLen: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logger := logr.Discard() + envVars, err := GetEnvironmentVariables(tt.config, logger) + + assert.NoError(t, err) + assert.Len(t, envVars, tt.envVarsLen) + }) + } +} + +func TestConfig_GetReceiverPorts(t *testing.T) { + tests := []struct { + name string + file string + want []v1.ServicePort + wantErr bool + }{ + { + name: "k8sevents", + file: "testdata/otelcol-k8sevents.yaml", + want: []v1.ServicePort{ + { + Name: "otlp-http", + Protocol: "", + AppProtocol: ptr.To("http"), + Port: 4318, + TargetPort: intstr.FromInt32(4318), + }, + }, + wantErr: false, // Silently fail + }, + { + name: "connectors", + file: "testdata/otelcol-connectors.yaml", + want: nil, + wantErr: false, // Silently fail + }, + { + name: "couchbase", + file: "testdata/otelcol-couchbase.yaml", + want: nil, // Couchbase uses a prometheus scraper, no ports should be opened + }, + { + name: "demo", + file: "testdata/otelcol-demo.yaml", + want: []v1.ServicePort{ + { + Name: "otlp-grpc", + Protocol: "", + AppProtocol: ptr.To("grpc"), + Port: 4317, + TargetPort: intstr.FromInt32(4317), + }, + }, + }, + { + name: "extensions", + file: "testdata/otelcol-extensions.yaml", + want: []v1.ServicePort{ + { + Name: "otlp-grpc", + Protocol: "", + AppProtocol: ptr.To("grpc"), + Port: 4317, + TargetPort: intstr.FromInt32(4317), + }, + }, + }, + { + name: "filelog", + file: "testdata/otelcol-filelog.yaml", + want: nil, + }, + { + name: "null", + file: "testdata/otelcol-null-values.yaml", + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + collectorYaml, err := os.ReadFile(tt.file) + require.NoError(t, err) + + c := &v1beta1.Config{} + err = go_yaml.Unmarshal(collectorYaml, c) + require.NoError(t, err) + ports, err := GetReceiverPorts(c, logr.Discard()) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equalf(t, tt.want, ports, "GetReceiverPorts()") + }) + } +} + +func TestConfig_GetExporterPorts(t *testing.T) { + tests := []struct { + name string + file string + want []v1.ServicePort + wantErr bool + }{ + { + name: "connectors", + file: "testdata/otelcol-connectors.yaml", + want: nil, + wantErr: false, + }, + { + name: "couchbase", + file: "testdata/otelcol-couchbase.yaml", + want: []v1.ServicePort{ + { + Name: "prometheus", + Port: 9123, + }, + }, + }, + { + name: "demo", + file: "testdata/otelcol-demo.yaml", + want: []v1.ServicePort{ + { + Name: "prometheus", + Port: 8889, + }, + }, + }, + { + name: "extensions", + file: "testdata/otelcol-extensions.yaml", + want: nil, + }, + { + name: "filelog", + file: "testdata/otelcol-filelog.yaml", + want: nil, + }, + { + name: "null", + file: "testdata/otelcol-null-values.yaml", + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + collectorYaml, err := os.ReadFile(tt.file) + require.NoError(t, err) + + c := &v1beta1.Config{} + err = go_yaml.Unmarshal(collectorYaml, c) + require.NoError(t, err) + ports, err := GetExporterPorts(c, logr.Discard()) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.ElementsMatchf(t, tt.want, ports, "GetReceiverPorts()") + }) + } +} + +func TestConfig_GetLivenessProbe(t *testing.T) { + tests := []struct { + name string + config *v1beta1.Config + wantProbe *v1.Probe + wantErr bool + }{ + { + name: "nil extensions should return nil", + config: &v1beta1.Config{ + Extensions: nil, + Service: v1beta1.Service{ + Extensions: []string{}, + }, + }, + wantProbe: nil, + }, + { + name: "nil extensions with health_check in service extensions should return nil", + config: &v1beta1.Config{ + Extensions: nil, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: nil, + }, + { + name: "empty extensions should return nil", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{}, + }, + Service: v1beta1.Service{ + Extensions: []string{}, + }, + }, + wantProbe: nil, + }, + { + name: "empty extensions with health_check in service extensions should return probe", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{}, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension enabled should return probe", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension with custom path", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{ + "path": "/healthz", + }, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension with custom endpoint port", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{ + "endpoint": "0.0.0.0:8080", + }, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(8080), + }, + }, + }, + }, + { + name: "extension without liveness probe should return nil", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "jaeger_query": map[string]any{}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"jaeger_query"}, + }, + }, + wantProbe: nil, + }, + { + name: "invalid health_check config should return error", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": func() {}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetLivenessProbe(tt.config, logr.Discard()) + if (err != nil) != tt.wantErr { + t.Errorf("GetLivenessProbe() error = %v, wantErr %v", err, tt.wantErr) + return + } + if diff := cmp.Diff(tt.wantProbe, got); diff != "" { + t.Errorf("GetLivenessProbe() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestConfig_GetReadinessProbe(t *testing.T) { + tests := []struct { + name string + config *v1beta1.Config + wantProbe *v1.Probe + wantErr bool + }{ + { + name: "nil extensions should return nil", + config: &v1beta1.Config{ + Extensions: nil, + Service: v1beta1.Service{ + Extensions: []string{}, + }, + }, + wantProbe: nil, + }, + { + name: "nil extensions with health_check in service extensions should return nil", + config: &v1beta1.Config{ + Extensions: nil, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: nil, + }, + { + name: "empty extensions should return nil", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{}, + }, + Service: v1beta1.Service{ + Extensions: []string{}, + }, + }, + wantProbe: nil, + }, + { + name: "empty extensions with health_check in service extensions should return probe", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{}, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension enabled should return probe", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension with custom path", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{ + "path": "/healthz", + }, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension with custom endpoint port", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{ + "endpoint": "0.0.0.0:8080", + }, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(8080), + }, + }, + }, + }, + { + name: "extension without readiness probe should return nil", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "jaeger_query": map[string]any{}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"jaeger_query"}, + }, + }, + wantProbe: nil, + }, + { + name: "invalid health_check config should return error", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": func() {}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetReadinessProbe(tt.config, logr.Discard()) + if (err != nil) != tt.wantErr { + t.Errorf("GetReadinessProbe() error = %v, wantErr %v", err, tt.wantErr) + return + } + if diff := cmp.Diff(tt.wantProbe, got); diff != "" { + t.Errorf("GetReadinessProbe() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestConfig_GetStartupProbe(t *testing.T) { + tests := []struct { + name string + config *v1beta1.Config + wantProbe *v1.Probe + wantErr bool + }{ + { + name: "nil extensions should return nil", + config: &v1beta1.Config{ + Extensions: nil, + Service: v1beta1.Service{ + Extensions: []string{}, + }, + }, + wantProbe: nil, + }, + { + name: "nil extensions with health_check in service extensions should return nil", + config: &v1beta1.Config{ + Extensions: nil, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: nil, + }, + { + name: "empty extensions should return nil", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{}, + }, + Service: v1beta1.Service{ + Extensions: []string{}, + }, + }, + wantProbe: nil, + }, + { + name: "empty extensions with health_check in service extensions should return probe", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{}, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension enabled should return probe", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension with custom path", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{ + "path": "/healthz", + }, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromInt32(13133), + }, + }, + }, + }, + { + name: "health_check extension with custom endpoint port", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": map[string]any{ + "endpoint": "0.0.0.0:8080", + }, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(8080), + }, + }, + }, + }, + { + name: "extension without startup probe should return nil", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "jaeger_query": map[string]any{}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"jaeger_query"}, + }, + }, + wantProbe: nil, + }, + { + name: "invalid health_check config should return error", + config: &v1beta1.Config{ + Extensions: &v1beta1.AnyConfig{ + Object: map[string]any{ + "health_check": func() {}, + }, + }, + Service: v1beta1.Service{ + Extensions: []string{"health_check"}, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetStartupProbe(tt.config, logr.Discard()) + if (err != nil) != tt.wantErr { + t.Errorf("GetStartupProbe() error = %v, wantErr %v", err, tt.wantErr) + return + } + if diff := cmp.Diff(tt.wantProbe, got); diff != "" { + t.Errorf("GetStartupProbe() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestTelemetryLogsPreservedWithMetrics(t *testing.T) { + // Test case where logs configuration exists and metrics is added + cfg := &v1beta1.Config{ + Service: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "logs": map[string]any{ + "level": "debug", + }, + }, + }, + }, + } + + expected := &v1beta1.Config{ + Service: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "logs": map[string]any{ + "level": "debug", + }, + "metrics": map[string]any{ + "readers": []any{ + map[string]any{ + "pull": map[string]any{ + "exporter": map[string]any{ + "prometheus": map[string]any{ + "host": "0.0.0.0", + "port": int32(8888), + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + _, err := ServiceApplyDefaults(&cfg.Service, logr.Discard()) + require.NoError(t, err) + + logger := logr.Discard() + telemetry := GetTelemetry(&cfg.Service, logger) + require.NotNil(t, telemetry) + require.Equal(t, expected, cfg) +} + +func TestTelemetryIncompleteConfigAppliesDefaults(t *testing.T) { + cfg := &v1beta1.Config{ + Service: v1beta1.Service{ + Telemetry: &v1beta1.AnyConfig{ + Object: map[string]any{ + "metrics": map[string]any{ + "level": "basic", + "readers": []any{ + map[string]any{ + "periodic": map[string]any{ + "exporter": map[string]any{ + "otlp": map[string]any{ + "endpoint": "otlp_host:4317", + // Missing protocol - makes this invalid + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + _, err := ServiceApplyDefaults(&cfg.Service, logr.Discard()) + require.NoError(t, err) + + logger := logr.Discard() + telemetry := GetTelemetry(&cfg.Service, logger) + require.NotNil(t, telemetry) + + require.Len(t, telemetry.Metrics.Readers, 1) + + require.NotNil(t, telemetry.Metrics.Readers[0].Pull) + require.NotNil(t, telemetry.Metrics.Readers[0].Pull.Exporter.Prometheus) + require.Equal(t, "0.0.0.0", *telemetry.Metrics.Readers[0].Pull.Exporter.Prometheus.Host) + require.Equal(t, 8888, *telemetry.Metrics.Readers[0].Pull.Exporter.Prometheus.Port) +} diff --git a/apis/v1beta1/helpers.go b/internal/otelconfig/helpers.go similarity index 97% rename from apis/v1beta1/helpers.go rename to internal/otelconfig/helpers.go index 61627c82ba..e250eb790a 100644 --- a/apis/v1beta1/helpers.go +++ b/internal/otelconfig/helpers.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package v1beta1 +package otelconfig import ( "fmt" @@ -12,6 +12,11 @@ import ( "strings" ) +const ( + defaultServicePort int32 = 8888 + defaultServiceHost = "0.0.0.0" +) + // parseAddressEndpoint parses the address and returns the host and port. // If the address is an environment variable, it returns the default port. // If the address is an explicit port, it returns the port. diff --git a/apis/v1beta1/helpers_test.go b/internal/otelconfig/helpers_test.go similarity index 99% rename from apis/v1beta1/helpers_test.go rename to internal/otelconfig/helpers_test.go index 49e2b1234c..eaf91c9bda 100644 --- a/apis/v1beta1/helpers_test.go +++ b/internal/otelconfig/helpers_test.go @@ -1,7 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -package v1beta1 +package otelconfig import ( "slices" diff --git a/internal/webhook/collector_webhook.go b/internal/webhook/collector_webhook.go index 41393c2900..fb13056f21 100644 --- a/internal/webhook/collector_webhook.go +++ b/internal/webhook/collector_webhook.go @@ -23,6 +23,7 @@ import ( ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" "github.com/open-telemetry/opentelemetry-operator/internal/metrics" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" "github.com/open-telemetry/opentelemetry-operator/internal/rbac" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -104,7 +105,7 @@ func (c CollectorWebhook) Default(_ context.Context, otelcol *v1beta1.OpenTeleme // TLS defaults are applied at reconciliation time (ConfigMap generation) so that // existing collectors automatically get updated TLS settings when the operator // restarts after a cluster TLS profile change. - events, err := otelcol.Spec.Config.ApplyDefaults(c.logger) + events, err := otelconfig.ApplyDefaults(&otelcol.Spec.Config, c.logger) if err != nil { return err } @@ -167,7 +168,7 @@ func (c CollectorWebhook) ValidateDelete(ctx context.Context, otelcol *v1beta1.O func (c CollectorWebhook) Validate(ctx context.Context, r *v1beta1.OpenTelemetryCollector) (admission.Warnings, error) { warnings := admission.Warnings{} - nullObjects := r.Spec.Config.NullObjects() + nullObjects := otelconfig.NullObjects(&r.Spec.Config) if len(nullObjects) > 0 { warnings = append(warnings, fmt.Sprintf("Collector config spec.config has null objects: %s. For compatibility with other tooling, such as kustomize and kubectl edit, it is recommended to use empty objects e.g. batch: {}.", strings.Join(nullObjects, ", "))) } @@ -220,7 +221,7 @@ func (c CollectorWebhook) Validate(ctx context.Context, r *v1beta1.OpenTelemetry if err := ValidatePorts(r.Spec.Ports); err != nil { return warnings, err } - ports, errPorts := r.Spec.Config.GetAllPorts(c.logger) + ports, errPorts := otelconfig.GetAllPorts(&r.Spec.Config, c.logger) if errPorts != nil { return warnings, fmt.Errorf("the OpenTelemetry config is incorrect. The port numbers are invalid: %w", errPorts) } @@ -295,7 +296,7 @@ func (c CollectorWebhook) Validate(ctx context.Context, r *v1beta1.OpenTelemetry } if c.fips != nil { - components := r.Spec.Config.GetEnabledComponents() + components := otelconfig.GetEnabledComponents(&r.Spec.Config) if notAllowedComponents := c.fips.DisabledComponents(components[v1beta1.KindReceiver], components[v1beta1.KindExporter], components[v1beta1.KindProcessor], components[v1beta1.KindExtension]); notAllowedComponents != nil { return nil, fmt.Errorf("the collector configuration contains not FIPS compliant components: %s. Please remove it from the config", notAllowedComponents) } @@ -317,7 +318,7 @@ func (c CollectorWebhook) validateTargetAllocatorConfig(ctx context.Context, r * return nil, fmt.Errorf("target allocation strategy %s is only supported in OpenTelemetry Collector mode %s", v1beta1.TargetAllocatorAllocationStrategyPerNode, v1beta1.ModeDaemonSet) } - cfgYaml, err := r.Spec.Config.Yaml() + cfgYaml, err := otelconfig.Yaml(&r.Spec.Config) if err != nil { return nil, err } diff --git a/internal/webhook/collector_webhook_test.go b/internal/webhook/collector_webhook_test.go index 5aa6f25266..bee7c71db3 100644 --- a/internal/webhook/collector_webhook_test.go +++ b/internal/webhook/collector_webhook_test.go @@ -34,6 +34,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/manifests" collectorManifests "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" "github.com/open-telemetry/opentelemetry-operator/internal/rbac" "github.com/open-telemetry/opentelemetry-operator/internal/webhook" ) @@ -551,7 +552,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) { ctx := context.Background() err := cvw.Default(ctx, &test.otelcol) if test.expected.Spec.Config.Service.Telemetry == nil { - _, applyErr := test.expected.Spec.Config.Service.ApplyDefaults(logr.Discard()) + _, applyErr := otelconfig.ServiceApplyDefaults(&test.expected.Spec.Config.Service, logr.Discard()) assert.NoError(t, applyErr, "could not apply defaults") } assert.NoError(t, err) diff --git a/main.go b/main.go index 9c97131301..3bb17bdf02 100644 --- a/main.go +++ b/main.go @@ -456,7 +456,7 @@ func main() { setupLog.Error(metricsErr, "Error bootstrapping CRD metrics") } - crdMetrics, err = metrics.New(meterProvider, ctx, mgr.GetAPIReader()) + crdMetrics, err = metrics.New(ctx, meterProvider, mgr.GetAPIReader()) if err != nil { setupLog.Error(err, "Error init CRD metrics") } diff --git a/pkg/collector/upgrade/v0_111_0.go b/pkg/collector/upgrade/v0_111_0.go index bd757eae3f..70b4779af8 100644 --- a/pkg/collector/upgrade/v0_111_0.go +++ b/pkg/collector/upgrade/v0_111_0.go @@ -10,6 +10,7 @@ import ( "github.com/go-logr/logr" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) func upgrade0_111_0(u VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) (*v1beta1.OpenTelemetryCollector, error) { @@ -17,7 +18,7 @@ func upgrade0_111_0(u VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) ( } func applyDefaults(otelcol *v1beta1.OpenTelemetryCollector, logger logr.Logger) error { - telemetryAddr, telemetryPort, err := otelcol.Spec.Config.Service.MetricsEndpoint(logger) + telemetryAddr, telemetryPort, err := otelconfig.MetricsEndpoint(&otelcol.Spec.Config.Service, logger) if err != nil { return err } diff --git a/pkg/collector/upgrade/v0_122_0.go b/pkg/collector/upgrade/v0_122_0.go index 10f96b2bc4..9bbeaa221e 100644 --- a/pkg/collector/upgrade/v0_122_0.go +++ b/pkg/collector/upgrade/v0_122_0.go @@ -5,16 +5,17 @@ package upgrade import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) func upgrade0_122_0(u VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) (*v1beta1.OpenTelemetryCollector, error) { - tel := otelcol.Spec.Config.Service.GetTelemetry(u.Log) + tel := otelconfig.GetTelemetry(&otelcol.Spec.Config.Service, u.Log) if tel == nil || tel.Metrics.Address == "" { return otelcol, nil } - host, port, err := otelcol.Spec.Config.Service.MetricsEndpoint(u.Log) + host, port, err := otelconfig.MetricsEndpoint(&otelcol.Spec.Config.Service, u.Log) if err != nil { return otelcol, err } @@ -26,10 +27,10 @@ func upgrade0_122_0(u VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) ( // differently from explicitly empty ones. By assigning "", we ensure the configuration // is updated correctly when the resource is persisted. tel.Metrics.Address = "" - reader := v1beta1.AddPrometheusMetricsEndpoint(host, port) + reader := otelconfig.AddPrometheusMetricsEndpoint(host, port) tel.Metrics.Readers = append(tel.Metrics.Readers, reader) - otelcol.Spec.Config.Service.Telemetry, err = tel.ToAnyConfig() + otelcol.Spec.Config.Service.Telemetry, err = otelconfig.TelemetryToAnyConfig(tel) if err != nil { return otelcol, err } diff --git a/pkg/sidecar/pod_test.go b/pkg/sidecar/pod_test.go index 3c25adc3fa..f38639c92b 100644 --- a/pkg/sidecar/pod_test.go +++ b/pkg/sidecar/pod_test.go @@ -15,6 +15,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/otelconfig" ) var logger = logf.Log.WithName("unit-tests") @@ -59,7 +60,7 @@ func TestAddNativeSidecar(t *testing.T) { }, } - otelcolYaml, err := otelcol.Spec.Config.Yaml() + otelcolYaml, err := otelconfig.Yaml(&otelcol.Spec.Config) require.NoError(t, err) cfg := config.Config{ CollectorImage: "some-default-image", @@ -188,7 +189,7 @@ func TestAddSidecarWhenNoSidecarExists(t *testing.T) { }, } - otelcolYaml, err := otelcol.Spec.Config.Yaml() + otelcolYaml, err := otelconfig.Yaml(&otelcol.Spec.Config) require.NoError(t, err) cfg := config.Config{ CollectorImage: "some-default-image", From f4aa85f89c56a18f6d97028618ded594b5ea8068 Mon Sep 17 00:00:00 2001 From: Ilia Petrov Date: Thu, 7 May 2026 10:03:10 +0300 Subject: [PATCH 3/6] feat: move MetricsConfig and Telemetry to the internal/otelconfig Signed-off-by: Ilia Petrov --- apis/v1beta1/config.go | 42 ---------------- apis/v1beta1/zz_generated.deepcopy.go | 32 ------------ internal/otelconfig/config.go | 49 +++++++++++++++++-- internal/otelconfig/config_test.go | 4 +- .../otelconfig}/testdata/issue-3452.yaml | 0 .../testdata/otelcol-connectors.yaml | 0 .../testdata/otelcol-couchbase.yaml | 0 .../otelconfig}/testdata/otelcol-demo.yaml | 0 .../testdata/otelcol-extensions.yaml | 0 .../otelconfig}/testdata/otelcol-filelog.yaml | 0 .../testdata/otelcol-k8sevents.yaml | 0 .../testdata/otelcol-null-values.yaml | 0 .../testdata/otelcol-pipelines.yaml | 0 13 files changed, 47 insertions(+), 80 deletions(-) rename {apis/v1beta1 => internal/otelconfig}/testdata/issue-3452.yaml (100%) rename {apis/v1beta1 => internal/otelconfig}/testdata/otelcol-connectors.yaml (100%) rename {apis/v1beta1 => internal/otelconfig}/testdata/otelcol-couchbase.yaml (100%) rename {apis/v1beta1 => internal/otelconfig}/testdata/otelcol-demo.yaml (100%) rename {apis/v1beta1 => internal/otelconfig}/testdata/otelcol-extensions.yaml (100%) rename {apis/v1beta1 => internal/otelconfig}/testdata/otelcol-filelog.yaml (100%) rename {apis/v1beta1 => internal/otelconfig}/testdata/otelcol-k8sevents.yaml (100%) rename {apis/v1beta1 => internal/otelconfig}/testdata/otelcol-null-values.yaml (100%) rename {apis/v1beta1 => internal/otelconfig}/testdata/otelcol-pipelines.yaml (100%) diff --git a/apis/v1beta1/config.go b/apis/v1beta1/config.go index 0ca87838da..c76a85cd59 100644 --- a/apis/v1beta1/config.go +++ b/apis/v1beta1/config.go @@ -6,8 +6,6 @@ package v1beta1 import ( "encoding/json" "maps" - - otelConfig "go.opentelemetry.io/contrib/otelconf/v0.3.0" ) type ComponentKind int @@ -122,43 +120,3 @@ type Service struct { Pipelines map[string]*Pipeline `json:"pipelines" yaml:"pipelines"` } -// MetricsConfig comes from the collector. -type MetricsConfig struct { - // Level is the level of telemetry metrics, the possible values are: - // - "none" indicates that no telemetry data should be collected; - // - "basic" is the recommended and covers the basics of the service telemetry. - // - "normal" adds some other indicators on top of basic. - // - "detailed" adds dimensions and views to the previous levels. - Level string `json:"level,omitempty" yaml:"level,omitempty"` - - // Address is the [address]:port that metrics exposition should be bound to. - Address string `json:"address,omitempty" yaml:"address,omitempty"` - - otelConfig.MeterProvider `mapstructure:",squash"` -} - -func (in *MetricsConfig) DeepCopyInto(out *MetricsConfig) { - *out = *in - out.MeterProvider = in.MeterProvider -} - -// DeepCopy creates a new deepcopy of MetricsConfig. -func (in *MetricsConfig) DeepCopy() *MetricsConfig { - if in == nil { - return nil - } - out := new(MetricsConfig) - in.DeepCopyInto(out) - return out -} - -// Telemetry is an intermediary type that allows for easy access to the collector's telemetry settings. -type Telemetry struct { - Metrics MetricsConfig `json:"metrics,omitempty" yaml:"metrics,omitempty"` - - // Resource specifies user-defined attributes to include with all emitted telemetry. - // Note that some attributes are added automatically (e.g. service.version) even - // if they are not specified here. In order to suppress such attributes the - // attribute must be specified in this map with null YAML value (nil string pointer). - Resource map[string]*string `json:"resource,omitempty" yaml:"resource,omitempty"` -} diff --git a/apis/v1beta1/zz_generated.deepcopy.go b/apis/v1beta1/zz_generated.deepcopy.go index 463fc965a8..c04fac714e 100644 --- a/apis/v1beta1/zz_generated.deepcopy.go +++ b/apis/v1beta1/zz_generated.deepcopy.go @@ -935,35 +935,3 @@ func (in *TargetAllocatorPrometheusCR) DeepCopy() *TargetAllocatorPrometheusCR { in.DeepCopyInto(out) return out } - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Telemetry) DeepCopyInto(out *Telemetry) { - *out = *in - in.Metrics.DeepCopyInto(&out.Metrics) - if in.Resource != nil { - in, out := &in.Resource, &out.Resource - *out = make(map[string]*string, len(*in)) - for key, val := range *in { - var outVal *string - if val == nil { - (*out)[key] = nil - } else { - inVal := (*in)[key] - in, out := &inVal, &outVal - *out = new(string) - **out = **in - } - (*out)[key] = outVal - } - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Telemetry. -func (in *Telemetry) DeepCopy() *Telemetry { - if in == nil { - return nil - } - out := new(Telemetry) - in.DeepCopyInto(out) - return out -} diff --git a/internal/otelconfig/config.go b/internal/otelconfig/config.go index 62f9cf08df..96fe6085b0 100644 --- a/internal/otelconfig/config.go +++ b/internal/otelconfig/config.go @@ -26,6 +26,47 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/components/receivers" ) +// MetricsConfig comes from the collector. +type MetricsConfig struct { + // Level is the level of telemetry metrics, the possible values are: + // - "none" indicates that no telemetry data should be collected; + // - "basic" is the recommended and covers the basics of the service telemetry. + // - "normal" adds some other indicators on top of basic. + // - "detailed" adds dimensions and views to the previous levels. + Level string `json:"level,omitempty" yaml:"level,omitempty"` + + // Address is the [address]:port that metrics exposition should be bound to. + Address string `json:"address,omitempty" yaml:"address,omitempty"` + + otelConfig.MeterProvider `mapstructure:",squash"` +} + +func (in *MetricsConfig) DeepCopyInto(out *MetricsConfig) { + *out = *in + out.MeterProvider = in.MeterProvider +} + +// DeepCopy creates a new deepcopy of MetricsConfig. +func (in *MetricsConfig) DeepCopy() *MetricsConfig { + if in == nil { + return nil + } + out := new(MetricsConfig) + in.DeepCopyInto(out) + return out +} + +// Telemetry is an intermediary type that allows for easy access to the collector's telemetry settings. +type Telemetry struct { + Metrics MetricsConfig `json:"metrics,omitempty" yaml:"metrics,omitempty"` + + // Resource specifies user-defined attributes to include with all emitted telemetry. + // Note that some attributes are added automatically (e.g. service.version) even + // if they are not specified here. In order to suppress such attributes the + // attribute must be specified in this map with null YAML value (nil string pointer). + Resource map[string]*string `json:"resource,omitempty" yaml:"resource,omitempty"` +} + // GetEnabledComponents constructs a list of enabled components by component type. func GetEnabledComponents(c *v1beta1.Config) map[v1beta1.ComponentKind]map[string]any { toReturn := map[v1beta1.ComponentKind]map[string]any{ @@ -423,7 +464,7 @@ func ServiceApplyDefaults(s *v1beta1.Service, logger logr.Logger) ([]v1beta1.Eve if tel == nil { logger.V(2).Info("no telemetry configuration parsed, creating default") - tel = &v1beta1.Telemetry{} + tel = &Telemetry{} s.Telemetry = &v1beta1.AnyConfig{ Object: map[string]any{}, } @@ -482,7 +523,7 @@ func AddPrometheusMetricsEndpoint(host string, port int32) otelConfig.MetricRead // GetTelemetry serves as a helper function to access the fields we care about in the underlying telemetry struct. // This exists to avoid needing to worry extra fields in the telemetry struct. -func GetTelemetry(s *v1beta1.Service, logger logr.Logger) *v1beta1.Telemetry { +func GetTelemetry(s *v1beta1.Service, logger logr.Logger) *Telemetry { if s.Telemetry == nil { logger.V(2).Info("no spec.service.telemetry configuration found") return nil @@ -497,7 +538,7 @@ func GetTelemetry(s *v1beta1.Service, logger logr.Logger) *v1beta1.Telemetry { logger.V(2).Info("marshaled telemetry configuration", "json", string(jsonData)) - t := &v1beta1.Telemetry{} + t := &Telemetry{} // Unmarshal JSON into the provided struct if err := json.Unmarshal(jsonData, t); err != nil { logger.Error(err, "failed to unmarshal telemetry configuration, this may indicate invalid configuration", "json", string(jsonData), "originalConfig", s.Telemetry.Object) @@ -513,7 +554,7 @@ func GetTelemetry(s *v1beta1.Service, logger logr.Logger) *v1beta1.Telemetry { } // TelemetryToAnyConfig converts the Telemetry struct to an AnyConfig struct. -func TelemetryToAnyConfig(t *v1beta1.Telemetry) (*v1beta1.AnyConfig, error) { +func TelemetryToAnyConfig(t *Telemetry) (*v1beta1.AnyConfig, error) { data, err := json.Marshal(t) if err != nil { return nil, err diff --git a/internal/otelconfig/config_test.go b/internal/otelconfig/config_test.go index ccca956483..02b8fb8d49 100644 --- a/internal/otelconfig/config_test.go +++ b/internal/otelconfig/config_test.go @@ -145,8 +145,8 @@ func TestGetTelemetryFromYAML(t *testing.T) { cfg := &v1beta1.Config{} err = go_yaml.Unmarshal(collectorYaml, cfg) require.NoError(t, err) - telemetry := &v1beta1.Telemetry{ - Metrics: v1beta1.MetricsConfig{ + telemetry := &Telemetry{ + Metrics: MetricsConfig{ Level: "detailed", Address: "0.0.0.0:8888", }, diff --git a/apis/v1beta1/testdata/issue-3452.yaml b/internal/otelconfig/testdata/issue-3452.yaml similarity index 100% rename from apis/v1beta1/testdata/issue-3452.yaml rename to internal/otelconfig/testdata/issue-3452.yaml diff --git a/apis/v1beta1/testdata/otelcol-connectors.yaml b/internal/otelconfig/testdata/otelcol-connectors.yaml similarity index 100% rename from apis/v1beta1/testdata/otelcol-connectors.yaml rename to internal/otelconfig/testdata/otelcol-connectors.yaml diff --git a/apis/v1beta1/testdata/otelcol-couchbase.yaml b/internal/otelconfig/testdata/otelcol-couchbase.yaml similarity index 100% rename from apis/v1beta1/testdata/otelcol-couchbase.yaml rename to internal/otelconfig/testdata/otelcol-couchbase.yaml diff --git a/apis/v1beta1/testdata/otelcol-demo.yaml b/internal/otelconfig/testdata/otelcol-demo.yaml similarity index 100% rename from apis/v1beta1/testdata/otelcol-demo.yaml rename to internal/otelconfig/testdata/otelcol-demo.yaml diff --git a/apis/v1beta1/testdata/otelcol-extensions.yaml b/internal/otelconfig/testdata/otelcol-extensions.yaml similarity index 100% rename from apis/v1beta1/testdata/otelcol-extensions.yaml rename to internal/otelconfig/testdata/otelcol-extensions.yaml diff --git a/apis/v1beta1/testdata/otelcol-filelog.yaml b/internal/otelconfig/testdata/otelcol-filelog.yaml similarity index 100% rename from apis/v1beta1/testdata/otelcol-filelog.yaml rename to internal/otelconfig/testdata/otelcol-filelog.yaml diff --git a/apis/v1beta1/testdata/otelcol-k8sevents.yaml b/internal/otelconfig/testdata/otelcol-k8sevents.yaml similarity index 100% rename from apis/v1beta1/testdata/otelcol-k8sevents.yaml rename to internal/otelconfig/testdata/otelcol-k8sevents.yaml diff --git a/apis/v1beta1/testdata/otelcol-null-values.yaml b/internal/otelconfig/testdata/otelcol-null-values.yaml similarity index 100% rename from apis/v1beta1/testdata/otelcol-null-values.yaml rename to internal/otelconfig/testdata/otelcol-null-values.yaml diff --git a/apis/v1beta1/testdata/otelcol-pipelines.yaml b/internal/otelconfig/testdata/otelcol-pipelines.yaml similarity index 100% rename from apis/v1beta1/testdata/otelcol-pipelines.yaml rename to internal/otelconfig/testdata/otelcol-pipelines.yaml From 41beaf0ec998dfd1a57d565cea15e6f58424cba7 Mon Sep 17 00:00:00 2001 From: Ilia Petrov Date: Thu, 7 May 2026 10:48:47 +0300 Subject: [PATCH 4/6] feat: mv config_test copletely to internal/otelconfig Signed-off-by: Ilia Petrov --- apis/v1beta1/config_test.go | 139 ----------------------------- internal/otelconfig/config_test.go | 125 ++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 139 deletions(-) delete mode 100644 apis/v1beta1/config_test.go diff --git a/apis/v1beta1/config_test.go b/apis/v1beta1/config_test.go deleted file mode 100644 index 9b0866ede2..0000000000 --- a/apis/v1beta1/config_test.go +++ /dev/null @@ -1,139 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package v1beta1 - -import ( - "encoding/json" - "os" - "path" - "strings" - "testing" - - go_yaml "github.com/goccy/go-yaml" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestConfigFiles(t *testing.T) { - files, err := os.ReadDir("./testdata") - require.NoError(t, err) - - for _, file := range files { - if !strings.HasPrefix(file.Name(), "otelcol-") { - continue - } - - testFile := path.Join("./testdata", file.Name()) - t.Run(testFile, func(t *testing.T) { - collectorYaml, err := os.ReadFile(testFile) - require.NoError(t, err) - - collectorJson, err := go_yaml.YAMLToJSON(collectorYaml) - require.NoError(t, err) - - cfg := &Config{} - err = json.Unmarshal(collectorJson, cfg) - require.NoError(t, err) - jsonCfg, err := json.Marshal(cfg) - require.NoError(t, err) - - assert.JSONEq(t, string(collectorJson), string(jsonCfg)) - yamlCfg, err := go_yaml.JSONToYAML(jsonCfg) - require.NoError(t, err) - assert.YAMLEq(t, string(collectorYaml), string(yamlCfg)) - }) - } -} - -func TestConfigFiles_go_yaml(t *testing.T) { - files, err := os.ReadDir("./testdata") - require.NoError(t, err) - - for _, file := range files { - if !strings.HasPrefix(file.Name(), "otelcol-") { - continue - } - - testFile := path.Join("./testdata", file.Name()) - t.Run(testFile, func(t *testing.T) { - collectorYaml, err := os.ReadFile(testFile) - require.NoError(t, err) - - cfg := &Config{} - err = go_yaml.Unmarshal(collectorYaml, cfg) - require.NoError(t, err) - yamlCfg, err := go_yaml.Marshal(cfg) - require.NoError(t, err) - - require.NoError(t, err) - assert.YAMLEq(t, string(collectorYaml), string(yamlCfg)) - }) - } -} - -func TestAnyConfigDeepCopyInto_NestedMapIndependence(t *testing.T) { - src := AnyConfig{Object: map[string]any{ - "prometheus": map[string]any{ - "config": map[string]any{ - "scrape_configs": []any{ - map[string]any{ - "job_name": "kubelet", - "tls_config": map[string]any{ - "ca_file": "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt", - "insecure_skip_verify": true, - }, - }, - }, - }, - }, - }} - - dst := src.DeepCopy() - - // Mutate a nested map in the copy (simulates TLS profile injection). - scrapeConfigs := dst.Object["prometheus"].(map[string]any)["config"].(map[string]any)["scrape_configs"].([]any) - tlsConfig := scrapeConfigs[0].(map[string]any)["tls_config"].(map[string]any) - tlsConfig["min_version"] = "TLS12" - - // Source nested map must be unaffected. - srcTLS := src.Object["prometheus"].(map[string]any)["config"].(map[string]any)["scrape_configs"].([]any)[0].(map[string]any)["tls_config"].(map[string]any) - assert.NotContains(t, srcTLS, "min_version", "DeepCopy must produce independent nested maps; source was mutated through the copy") -} - -func TestAnyConfigDeepCopyInto_NilObject(t *testing.T) { - src := AnyConfig{Object: nil} - dst := src.DeepCopy() - assert.Nil(t, dst.Object) -} - -func TestAnyConfigDeepCopyInto_EmptyObject(t *testing.T) { - src := AnyConfig{Object: map[string]any{}} - dst := src.DeepCopy() - assert.NotNil(t, dst.Object) - assert.Empty(t, dst.Object) - // Mutating dst should not affect src. - dst.Object["key"] = "value" - assert.Empty(t, src.Object) -} - -func TestAnyConfigDeepCopyInto_PreservesValues(t *testing.T) { - src := AnyConfig{Object: map[string]any{ - "string_val": "hello", - "number_val": float64(42), - "bool_val": true, - "nested": map[string]any{ - "inner": "value", - "list": []any{"a", "b"}, - }, - }} - - dst := src.DeepCopy() - - assert.Equal(t, "hello", dst.Object["string_val"]) - assert.Equal(t, float64(42), dst.Object["number_val"]) - assert.Equal(t, true, dst.Object["bool_val"]) - nested := dst.Object["nested"].(map[string]any) - assert.Equal(t, "value", nested["inner"]) - assert.Equal(t, []any{"a", "b"}, nested["list"]) -} diff --git a/internal/otelconfig/config_test.go b/internal/otelconfig/config_test.go index 02b8fb8d49..b0c74ce9da 100644 --- a/internal/otelconfig/config_test.go +++ b/internal/otelconfig/config_test.go @@ -6,6 +6,8 @@ package otelconfig import ( "encoding/json" "os" + "path" + "strings" "testing" "github.com/go-logr/logr" @@ -20,6 +22,129 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" ) +func TestConfigFiles(t *testing.T) { + files, err := os.ReadDir("./testdata") + require.NoError(t, err) + + for _, file := range files { + if !strings.HasPrefix(file.Name(), "otelcol-") { + continue + } + + testFile := path.Join("./testdata", file.Name()) + t.Run(testFile, func(t *testing.T) { + collectorYaml, err := os.ReadFile(testFile) + require.NoError(t, err) + + collectorJson, err := go_yaml.YAMLToJSON(collectorYaml) + require.NoError(t, err) + + cfg := &v1beta1.Config{} + err = json.Unmarshal(collectorJson, cfg) + require.NoError(t, err) + jsonCfg, err := json.Marshal(cfg) + require.NoError(t, err) + + assert.JSONEq(t, string(collectorJson), string(jsonCfg)) + yamlCfg, err := go_yaml.JSONToYAML(jsonCfg) + require.NoError(t, err) + assert.YAMLEq(t, string(collectorYaml), string(yamlCfg)) + }) + } +} + +func TestConfigFiles_go_yaml(t *testing.T) { + files, err := os.ReadDir("./testdata") + require.NoError(t, err) + + for _, file := range files { + if !strings.HasPrefix(file.Name(), "otelcol-") { + continue + } + + testFile := path.Join("./testdata", file.Name()) + t.Run(testFile, func(t *testing.T) { + collectorYaml, err := os.ReadFile(testFile) + require.NoError(t, err) + + cfg := &v1beta1.Config{} + err = go_yaml.Unmarshal(collectorYaml, cfg) + require.NoError(t, err) + yamlCfg, err := go_yaml.Marshal(cfg) + require.NoError(t, err) + + require.NoError(t, err) + assert.YAMLEq(t, string(collectorYaml), string(yamlCfg)) + }) + } +} + +func TestAnyConfigDeepCopyInto_NestedMapIndependence(t *testing.T) { + src := v1beta1.AnyConfig{Object: map[string]any{ + "prometheus": map[string]any{ + "config": map[string]any{ + "scrape_configs": []any{ + map[string]any{ + "job_name": "kubelet", + "tls_config": map[string]any{ + "ca_file": "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt", + "insecure_skip_verify": true, + }, + }, + }, + }, + }, + }} + + dst := src.DeepCopy() + + // Mutate a nested map in the copy (simulates TLS profile injection). + scrapeConfigs := dst.Object["prometheus"].(map[string]any)["config"].(map[string]any)["scrape_configs"].([]any) + tlsConfig := scrapeConfigs[0].(map[string]any)["tls_config"].(map[string]any) + tlsConfig["min_version"] = "TLS12" + + // Source nested map must be unaffected. + srcTLS := src.Object["prometheus"].(map[string]any)["config"].(map[string]any)["scrape_configs"].([]any)[0].(map[string]any)["tls_config"].(map[string]any) + assert.NotContains(t, srcTLS, "min_version", "DeepCopy must produce independent nested maps; source was mutated through the copy") +} + +func TestAnyConfigDeepCopyInto_NilObject(t *testing.T) { + src := v1beta1.AnyConfig{Object: nil} + dst := src.DeepCopy() + assert.Nil(t, dst.Object) +} + +func TestAnyConfigDeepCopyInto_EmptyObject(t *testing.T) { + src := v1beta1.AnyConfig{Object: map[string]any{}} + dst := src.DeepCopy() + assert.NotNil(t, dst.Object) + assert.Empty(t, dst.Object) + // Mutating dst should not affect src. + dst.Object["key"] = "value" + assert.Empty(t, src.Object) +} + +func TestAnyConfigDeepCopyInto_PreservesValues(t *testing.T) { + src := v1beta1.AnyConfig{Object: map[string]any{ + "string_val": "hello", + "number_val": float64(42), + "bool_val": true, + "nested": map[string]any{ + "inner": "value", + "list": []any{"a", "b"}, + }, + }} + + dst := src.DeepCopy() + + assert.Equal(t, "hello", dst.Object["string_val"]) + assert.Equal(t, float64(42), dst.Object["number_val"]) + assert.Equal(t, true, dst.Object["bool_val"]) + nested := dst.Object["nested"].(map[string]any) + assert.Equal(t, "value", nested["inner"]) + assert.Equal(t, []any{"a", "b"}, nested["list"]) +} + func TestNullObjects(t *testing.T) { collectorYaml, err := os.ReadFile("./testdata/otelcol-null-values.yaml") require.NoError(t, err) From 178d5cde7aa83c870bd6adc0446c1e39975603c0 Mon Sep 17 00:00:00 2001 From: Ilia Petrov Date: Thu, 7 May 2026 14:44:38 +0300 Subject: [PATCH 5/6] fix: lint issues Signed-off-by: Ilia Petrov --- apis/v1beta1/config.go | 1 - internal/otelconfig/config.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/apis/v1beta1/config.go b/apis/v1beta1/config.go index c76a85cd59..637073314c 100644 --- a/apis/v1beta1/config.go +++ b/apis/v1beta1/config.go @@ -119,4 +119,3 @@ type Service struct { // +kubebuilder:pruning:PreserveUnknownFields Pipelines map[string]*Pipeline `json:"pipelines" yaml:"pipelines"` } - diff --git a/internal/otelconfig/config.go b/internal/otelconfig/config.go index 96fe6085b0..b4e02e03a6 100644 --- a/internal/otelconfig/config.go +++ b/internal/otelconfig/config.go @@ -58,7 +58,7 @@ func (in *MetricsConfig) DeepCopy() *MetricsConfig { // Telemetry is an intermediary type that allows for easy access to the collector's telemetry settings. type Telemetry struct { - Metrics MetricsConfig `json:"metrics,omitempty" yaml:"metrics,omitempty"` + Metrics MetricsConfig `json:"metrics,omitzero" yaml:"metrics,omitempty"` // Resource specifies user-defined attributes to include with all emitted telemetry. // Note that some attributes are added automatically (e.g. service.version) even From 8222e29e4ff789975b60145a7f89340b33935eee Mon Sep 17 00:00:00 2001 From: Ilia Petrov Date: Mon, 11 May 2026 08:18:27 +0300 Subject: [PATCH 6/6] feat: add changelog entry Signed-off-by: Ilia Petrov --- ...ve-otelconfig-and-metrics-to-internal.yaml | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .chloggen/move-otelconfig-and-metrics-to-internal.yaml diff --git a/.chloggen/move-otelconfig-and-metrics-to-internal.yaml b/.chloggen/move-otelconfig-and-metrics-to-internal.yaml new file mode 100644 index 0000000000..0fa6070973 --- /dev/null +++ b/.chloggen/move-otelconfig-and-metrics-to-internal.yaml @@ -0,0 +1,29 @@ +# 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. collector, target allocator, auto-instrumentation, opamp, github action) +component: api + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Move config parsing and CRD metrics from apis to internal package + +# One or more tracking issues related to the change +issues: [4362] + +# (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: | + - Methods on `*Config` (package `apis/v1beta1`) converted to standalone functions in `internal/otelconfig`, now taking `*v1beta1.Config` as the first parameter: + - GetEnabledComponents, GetReceiverPorts, GetExporterPorts, GetExtensionPorts, GetReceiverAndExporterPorts, GetAllPorts, GetEnvironmentVariables, GetAllRbacRules, ApplyDefaults, GetLivenessProbe, GetReadinessProbe, GetStartupProbe, Yaml, NullObjects + - Methods on `*Service` converted to functions in `internal/otelconfig`: + - MetricsEndpoint, GetTelemetry + - ApplyDefaults → renamed to ServiceApplyDefaults + - Method on `*Telemetry` converted to function: + - ToAnyConfig → renamed to TelemetryToAnyConfig + - Functions moved from `apis/v1beta1` to `internal/metrics` with renames: + - BootstrapMetrics → Bootstrap + - NewMetrics(prv, ctx, cl) → New(ctx, prv, cl) (parameter reorder: ctx now first) + - Types moved out of `apis/v1beta1`: + - MetricsConfig, Telemetry → internal/otelconfig + - Metrics → internal/metrics