v1beta1: apply telemetry config defaults in webhook

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

.chloggen/default_telemetry_settings.yaml

@@ -0,0 +1,19 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# 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: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Expose the Collector telemetry endpoint by default.
+
+# One or more tracking issues related to the change
+issues: [3361]
+
+# (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: |
+  In the upcoming collector version v0.111.0 the telemetry metrics endpoint will by default switch to localhost.
+  To avoid any disruption we fallback to "0.0.0.0:{PORT}" as default address.
+  Details can be found here: [opentelemetry-collector#11251](https://github.com/open-telemetry/opentelemetry-collector/pull/11251)

apis/v1beta1/collector_webhook_test.go

@@ -169,7 +169,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 					Mode:            v1beta1.ModeDeployment,
 					UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic,
 					Config: func() v1beta1.Config {
-						const input = `{"receivers":{"otlp":{"protocols":{"grpc":{"endpoint":"0.0.0.0:4317"},"http":{"endpoint":"0.0.0.0:4318"}}}},"exporters":{"debug":null},"service":{"pipelines":{"traces":{"receivers":["otlp"],"exporters":["debug"]}}}}`
+						const input = `{"receivers":{"otlp":{"protocols":{"grpc":{"endpoint":"0.0.0.0:4317"},"http":{"endpoint":"0.0.0.0:4318"}}}},"exporters":{"debug":null},"service":{"telemetry":{"metrics":{"address":"0.0.0.0:8888"}},"pipelines":{"traces":{"receivers":["otlp"],"exporters":["debug"]}}}}`
 						var cfg v1beta1.Config
 						require.NoError(t, yaml.Unmarshal([]byte(input), &cfg))
 						return cfg
@@ -201,7 +201,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 					Mode:            v1beta1.ModeDeployment,
 					UpgradeStrategy: v1beta1.UpgradeStrategyAutomatic,
 					Config: func() v1beta1.Config {
-						const input = `{"receivers":{"otlp":{"protocols":{"grpc":{"endpoint":"0.0.0.0:4317","headers":{"example":"another"}},"http":{"endpoint":"0.0.0.0:4000"}}}},"exporters":{"debug":null},"service":{"pipelines":{"traces":{"receivers":["otlp"],"exporters":["debug"]}}}}`
+						const input = `{"receivers":{"otlp":{"protocols":{"grpc":{"endpoint":"0.0.0.0:4317","headers":{"example":"another"}},"http":{"endpoint":"0.0.0.0:4000"}}}},"exporters":{"debug":null},"service":{"telemetry":{"metrics":{"address":"0.0.0.0:8888"}},"pipelines":{"traces":{"receivers":["otlp"],"exporters":["debug"]}}}}`
 						var cfg v1beta1.Config
 						require.NoError(t, yaml.Unmarshal([]byte(input), &cfg))
 						return cfg
@@ -554,6 +554,9 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			)
 			ctx := context.Background()
 			err := cvw.Default(ctx, &test.otelcol)
+			if test.expected.Spec.Config.Service.Telemetry == nil {
+				assert.NoError(t, test.expected.Spec.Config.Service.ApplyDefaults(), "could not apply defaults")
+			}
 			assert.NoError(t, err)
 			assert.Equal(t, test.expected, test.otelcol)
 		})

apis/v1beta1/config.go

@@ -26,6 +26,7 @@ import (
 
 	"dario.cat/mergo"
 	"github.com/go-logr/logr"
+	"github.com/mitchellh/mapstructure"
 	"gopkg.in/yaml.v3"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -228,6 +229,9 @@ func (c *Config) getPortsForComponentKinds(logger logr.Logger, componentKinds ..
 
 // applyDefaultForComponentKinds applies defaults to the endpoints for the given ComponentKind(s).
 func (c *Config) applyDefaultForComponentKinds(logger logr.Logger, componentKinds ...ComponentKind) error {
+	if err := c.Service.ApplyDefaults(); err != nil {
+		return err
+	}
 	enabledComponents := c.GetEnabledComponents()
 	for _, componentKind := range componentKinds {
 		var retriever components.ParserRetriever
@@ -371,24 +375,46 @@ type Service struct {
 	Pipelines map[string]*Pipeline `json:"pipelines" yaml:"pipelines"`
 }
 
-// MetricsPort gets the port number for the metrics endpoint from the collector config if it has been set.
-func (s *Service) MetricsPort() (int32, error) {
+// MetricsEndpoint gets the port number and host address for the metrics endpoint from the collector config if it has been set.
+func (s *Service) MetricsEndpoint() (string, int32, error) {
 	if s.GetTelemetry() == nil {
 		// telemetry isn't set, use the default
-		return 8888, nil
+		return "0.0.0.0", 8888, nil
 	}
-	_, port, netErr := net.SplitHostPort(s.GetTelemetry().Metrics.Address)
+	host, port, netErr := net.SplitHostPort(s.GetTelemetry().Metrics.Address)
 	if netErr != nil && strings.Contains(netErr.Error(), "missing port in address") {
-		return 8888, nil
+		return host, 8888, nil
 	} else if netErr != nil {
-		return 0, netErr
+		return "", 0, netErr
 	}
 	i64, err := strconv.ParseInt(port, 10, 32)
 	if err != nil {
-		return 0, err
+		return "", 0, err
+	}
+
+	return "", int32(i64), nil
+}
+
+// ApplyDefaults inserts configuration defaults if it has not been set.
+func (s *Service) ApplyDefaults() error {
+	telemetryAddr, telemetryPort, err := s.MetricsEndpoint()
+	if err != nil {
+		return err
+	}
+	tele := Telemetry{Metrics: MetricsConfig{Address: fmt.Sprintf("%s:%d", telemetryAddr, telemetryPort)}}
+	tm := &AnyConfig{}
+	if err := mapstructure.Decode(tele, &tm.Object); err != nil {
+		return fmt.Errorf("telemetry config decoding failed: %w", err)
 	}
 
-	return int32(i64), nil
+	if s.Telemetry == nil {
+		s.Telemetry = tm
+		return nil
+	}
+	if err := mergo.Merge(s.Telemetry, tm); err != nil {
+		return fmt.Errorf("telemetry config merge failed: %w", err)
+	}
+	return nil
 }
 
 // MetricsConfig comes from the collector.
@@ -398,21 +424,21 @@ type MetricsConfig struct {
 	//  - "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"`
+	Level string `json:"level,omitempty" yaml:"level,omitempty" mapstructure:"level,omitempty"`
 
 	// Address is the [address]:port that metrics exposition should be bound to.
-	Address string `json:"address,omitempty" yaml:"address,omitempty"`
+	Address string `json:"address,omitempty" yaml:"address,omitempty" mapstructure:"address,omitempty"`
 }
 
 // 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,omitempty" yaml:"metrics,omitempty" mapstructure:"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"`
+	Resource map[string]*string `json:"resource,omitempty" yaml:"resource,omitempty" mapstructure:"resource,omitempty"`
 }
 
 // GetTelemetry serves as a helper function to access the fields we care about in the underlying telemetry struct.

apis/v1beta1/config_test.go

@@ -277,7 +277,7 @@ func TestConfigToMetricsPort(t *testing.T) {
 	} {
 		t.Run(tt.desc, func(t *testing.T) {
 			// these are acceptable failures, we return to the collector's default metric port
-			port, err := tt.config.MetricsPort()
+			_, port, err := tt.config.MetricsEndpoint()
 			assert.NoError(t, err)
 			assert.Equal(t, tt.expectedPort, port)
 		})

internal/manifests/collector/container.go

@@ -223,7 +223,7 @@ func getConfigContainerPorts(logger logr.Logger, conf v1beta1.Config) (map[strin
 		}
 	}
 
-	metricsPort, err := conf.Service.MetricsPort()
+	_, metricsPort, err := conf.Service.MetricsEndpoint()
 	if err != nil {
 		logger.Info("couldn't determine metrics port from configuration, using 8888 default value", "error", err)
 		metricsPort = 8888

internal/manifests/collector/service.go

@@ -83,7 +83,7 @@ func MonitoringService(params manifests.Params) (*corev1.Service, error) {
 		return nil, err
 	}
 
-	metricsPort, err := params.OtelCol.Spec.Config.Service.MetricsPort()
+	_, metricsPort, err := params.OtelCol.Spec.Config.Service.MetricsEndpoint()
 	if err != nil {
 		return nil, err
 	}

pkg/collector/upgrade/upgrade_test.go

@@ -141,7 +141,7 @@ func TestEnvVarUpdates(t *testing.T) {
 	require.Equal(t, collectorInstance.Status.Version, persisted.Status.Version)
 
 	currentV := version.Get()
-	currentV.OpenTelemetryCollector = "0.110.0"
+	currentV.OpenTelemetryCollector = "0.111.0"
 	up := &upgrade.VersionUpgrade{
 		Log:      logger,
 		Version:  currentV,

pkg/collector/upgrade/v0_111_0.go

@@ -0,0 +1,23 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package upgrade
+
+import (
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
+)
+
+func upgrade0_111_0(_ VersionUpgrade, otelcol *v1beta1.OpenTelemetryCollector) (*v1beta1.OpenTelemetryCollector, error) { //nolint:unparam
+	return otelcol, otelcol.Spec.Config.Service.ApplyDefaults()
+}

pkg/collector/upgrade/v0_111_0_test.go

@@ -0,0 +1,99 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package upgrade_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/mitchellh/mapstructure"
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/tools/record"
+
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
+	"github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade"
+)
+
+func Test0_111_0Upgrade(t *testing.T) {
+
+	defaultCollector := v1beta1.OpenTelemetryCollector{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "OpenTelemetryCollector",
+			APIVersion: "v1beta1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "otel-my-instance",
+			Namespace: "somewhere",
+		},
+		Status: v1beta1.OpenTelemetryCollectorStatus{
+			Version: "0.110.0",
+		},
+		Spec: v1beta1.OpenTelemetryCollectorSpec{
+			OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{},
+			Config:                    v1beta1.Config{},
+		},
+	}
+
+	defaultCollectorWithConfig := defaultCollector.DeepCopy()
+	tm := &v1beta1.AnyConfig{}
+	if err := mapstructure.Decode(v1beta1.Telemetry{Metrics: v1beta1.MetricsConfig{Address: "1.2.3.4:8888"}}, &tm.Object); err != nil {
+		t.Fatal(err)
+	}
+	defaultCollectorWithConfig.Spec.Config.Service.Telemetry = tm
+
+	tt := []struct {
+		name     string
+		input    v1beta1.OpenTelemetryCollector
+		expected v1beta1.OpenTelemetryCollector
+	}{
+		{
+			name:     "telemetry settings exist",
+			input:    *defaultCollectorWithConfig,
+			expected: *defaultCollectorWithConfig,
+		},
+		{
+			name:  "telemetry settings do not exist",
+			input: *defaultCollector.DeepCopy(),
+			expected: func() v1beta1.OpenTelemetryCollector {
+				col := defaultCollector.DeepCopy()
+				tele := v1beta1.Telemetry{Metrics: v1beta1.MetricsConfig{Address: "0.0.0.0:8888"}}
+				tm := &v1beta1.AnyConfig{}
+				if err := mapstructure.Decode(tele, &tm.Object); err != nil {
+					t.Fatal(err)
+				}
+				col.Spec.Config.Service.Telemetry = tm
+				return *col
+			}(),
+		},
+	}
+
+	versionUpgrade := &upgrade.VersionUpgrade{
+		Log:      logger,
+		Version:  makeVersion("0.111.0"),
+		Client:   k8sClient,
+		Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize),
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			col, err := versionUpgrade.ManagedInstance(context.Background(), tc.input)
+			if err != nil {
+				t.Errorf("expect err: nil but got: %v", err)
+			}
+			assert.Equal(t, tc.expected.Spec.Config.Service.Telemetry, col.Spec.Config.Service.Telemetry)
+		})
+	}
+}

pkg/collector/upgrade/versions.go

@@ -106,6 +106,10 @@ var (
 			Version:        *semver.MustParse("0.110.0"),
 			upgradeV1beta1: upgrade0_110_0,
 		},
+		{
+			Version:        *semver.MustParse("0.111.0"),
+			upgradeV1beta1: upgrade0_111_0,
+		},
 	}
 
 	// Latest represents the latest version that we need to upgrade. This is not necessarily the latest known version.

tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer-go/02-assert.yaml

@@ -92,6 +92,9 @@ spec:
         exporters:
           debug: null
         service:
+          telemetry:
+            metrics:
+              address: 0.0.0.0:8888
           pipelines:
             traces:
               exporters:

tests/e2e-multi-instrumentation/instrumentation-multi-multicontainer/01-assert.yaml

@@ -331,6 +331,9 @@ spec:
         exporters:
           debug: null
         service:
+          telemetry:
+            metrics:
+              address: 0.0.0.0:8888
           pipelines:
             traces:
               exporters:

tests/e2e-ta-collector-mtls/ta-collector-mtls/00-assert.yaml

@@ -59,9 +59,12 @@ data:
                     - prometheus
                 receivers:
                     - prometheus
+        telemetry:
+            metrics:
+                address: 0.0.0.0:8888
 kind: ConfigMap
 metadata:
-  name: prometheus-cr-collector-52e1d2ae
+  name: prometheus-cr-collector-19c94a81
 ---
 apiVersion: v1
 kind: Pod
@@ -83,4 +86,4 @@ kind: Job
 metadata:
   name: check-ta-serving-over-https
 status:
-  succeeded: 1
+  succeeded: 1

tests/e2e-targetallocator/targetallocator-features/00-assert.yaml

@@ -20,7 +20,7 @@ spec:
           items:
           - key: collector.yaml
             path: collector.yaml
-          name: stateful-collector-85dbe673
+          name: stateful-collector-c055e8e3
         name: otc-internal
       - emptyDir: {}
         name: testvolume

tests/e2e-targetallocator/targetallocator-kubernetessd/00-assert.yaml

@@ -15,7 +15,7 @@ metadata:
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: prometheus-kubernetessd-collector-699cdaa1
+  name: prometheus-kubernetessd-collector-9c184e3a
 data:
   collector.yaml: |
     exporters:
@@ -35,6 +35,9 @@ data:
                     - prometheus
                 receivers:
                     - prometheus
+        telemetry:
+            metrics:
+                address: 0.0.0.0:8888
 ---
 apiVersion: apps/v1
 kind: DaemonSet

tests/e2e-targetallocator/targetallocator-prometheuscr/00-assert.yaml

@@ -40,6 +40,9 @@ data:
                     - prometheus
                 receivers:
                     - prometheus
+        telemetry:
+            metrics:
+                address: 0.0.0.0:8888
 kind: ConfigMap
 metadata:
-  name: prometheus-cr-collector-52e1d2ae
+  name: prometheus-cr-collector-19c94a81