Deprecate service::telemetry::metrics::address

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

.chloggen/deprecate-address.yaml

@@ -0,0 +1,20 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: 'deprecation'
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: service/telemetry
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Deprecate service::telemetry::metrics::address in favor of service::telemetry::metrics::readers.
+
+# One or more tracking issues or pull requests related to the change
+issues: [11205]
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [user]

otelcol/config_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"go.opentelemetry.io/contrib/config"
 	"go.uber.org/zap/zapcore"
 
 	"go.opentelemetry.io/collector/component"
@@ -274,8 +275,17 @@ func generateConfig() *Config {
 					InitialFields:     map[string]any{"fieldKey": "filed-value"},
 				},
 				Metrics: telemetry.MetricsConfig{
-					Level:   configtelemetry.LevelNormal,
-					Address: ":8080",
+					Level: configtelemetry.LevelNormal,
+					Readers: []config.MetricReader{
+						{
+							Pull: &config.PullMetricReader{Exporter: config.MetricExporter{
+								Prometheus: &config.Prometheus{
+									Host: newPtr("localhost"),
+									Port: newPtr(8080),
+								},
+							}},
+						},
+					},
 				},
 			},
 			Extensions: []component.ID{component.MustNewID("nop")},
@@ -289,3 +299,7 @@ func generateConfig() *Config {
 		},
 	}
 }
+
+func newPtr[T int | string](str T) *T {
+	return &str
+}

service/config_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"go.opentelemetry.io/contrib/config"
 	"go.uber.org/zap/zapcore"
 
 	"go.opentelemetry.io/collector/component"
@@ -66,7 +67,7 @@ func TestConfigValidate(t *testing.T) {
 			cfgFn: func() *Config {
 				cfg := generateConfig()
 				cfg.Telemetry.Metrics.Level = configtelemetry.LevelBasic
-				cfg.Telemetry.Metrics.Address = ""
+				cfg.Telemetry.Metrics.Readers = nil
 				return cfg
 			},
 			expected: nil,
@@ -95,8 +96,13 @@ func generateConfig() *Config {
 				InitialFields:     map[string]any{"fieldKey": "filed-value"},
 			},
 			Metrics: telemetry.MetricsConfig{
-				Level:   configtelemetry.LevelNormal,
-				Address: ":8080",
+				Level: configtelemetry.LevelNormal,
+				Readers: []config.MetricReader{{
+					Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{
+						Host: newPtr("localhost"),
+						Port: newPtr(8080),
+					}}}},
+				},
 			},
 		},
 		Extensions: extensions.Config{component.MustNewID("nop")},

service/service_test.go

@@ -256,78 +256,6 @@ func TestServiceTelemetryCleanupOnError(t *testing.T) {
 }
 
 func TestServiceTelemetry(t *testing.T) {
-	for _, tc := range ownMetricsTestCases() {
-		t.Run(fmt.Sprintf("ipv4_%s", tc.name), func(t *testing.T) {
-			testCollectorStartHelper(t, tc, "tcp4")
-		})
-		t.Run(fmt.Sprintf("ipv6_%s", tc.name), func(t *testing.T) {
-			testCollectorStartHelper(t, tc, "tcp6")
-		})
-	}
-}
-
-func testCollectorStartHelper(t *testing.T, tc ownMetricsTestCase, network string) {
-	var once sync.Once
-	loggingHookCalled := false
-	hook := func(zapcore.Entry) error {
-		once.Do(func() {
-			loggingHookCalled = true
-		})
-		return nil
-	}
-
-	var (
-		metricsAddr string
-		zpagesAddr  string
-	)
-	switch network {
-	case "tcp", "tcp4":
-		metricsAddr = testutil.GetAvailableLocalAddress(t)
-		zpagesAddr = testutil.GetAvailableLocalAddress(t)
-	case "tcp6":
-		metricsAddr = testutil.GetAvailableLocalIPv6Address(t)
-		zpagesAddr = testutil.GetAvailableLocalIPv6Address(t)
-	}
-	require.NotZero(t, metricsAddr, "network must be either of tcp, tcp4 or tcp6")
-	require.NotZero(t, zpagesAddr, "network must be either of tcp, tcp4 or tcp6")
-
-	set := newNopSettings()
-	set.BuildInfo = component.BuildInfo{Version: "test version", Command: otelCommand}
-	set.ExtensionsConfigs = map[component.ID]component.Config{
-		component.MustNewID("zpages"): &zpagesextension.Config{
-			ServerConfig: confighttp.ServerConfig{Endpoint: zpagesAddr},
-		},
-	}
-	set.ExtensionsFactories = map[component.Type]extension.Factory{component.MustNewType("zpages"): zpagesextension.NewFactory()}
-	set.LoggingOptions = []zap.Option{zap.Hooks(hook)}
-
-	cfg := newNopConfig()
-	cfg.Extensions = []component.ID{component.MustNewID("zpages")}
-	cfg.Telemetry.Metrics.Address = metricsAddr
-	cfg.Telemetry.Resource = make(map[string]*string)
-	// Include resource attributes under the service::telemetry::resource key.
-	for k, v := range tc.userDefinedResource {
-		cfg.Telemetry.Resource[k] = v
-	}
-
-	// Create a service, check for metrics, shutdown and repeat to ensure that telemetry can be started/shutdown and started again.
-	for i := 0; i < 2; i++ {
-		srv, err := New(context.Background(), set, cfg)
-		require.NoError(t, err)
-
-		require.NoError(t, srv.Start(context.Background()))
-		// Sleep for 1 second to ensure the http server is started.
-		time.Sleep(1 * time.Second)
-		assert.True(t, loggingHookCalled)
-
-		assertResourceLabels(t, srv.telemetrySettings.Resource, tc.expectedLabels)
-		assertMetrics(t, metricsAddr, tc.expectedLabels)
-		assertZPages(t, zpagesAddr)
-		require.NoError(t, srv.Shutdown(context.Background()))
-	}
-}
-
-func TestServiceTelemetryWithReaders(t *testing.T) {
 	for _, tc := range ownMetricsTestCases() {
 		t.Run(fmt.Sprintf("ipv4_%s", tc.name), func(t *testing.T) {
 			testCollectorStartHelperWithReaders(t, tc, "tcp4")
@@ -375,7 +303,6 @@ func testCollectorStartHelperWithReaders(t *testing.T, tc ownMetricsTestCase, ne
 
 	cfg := newNopConfig()
 	cfg.Extensions = []component.ID{component.MustNewID("zpages")}
-	cfg.Telemetry.Metrics.Address = ""
 	cfg.Telemetry.Metrics.Readers = []config.MetricReader{
 		{
 			Pull: &config.PullMetricReader{
@@ -711,8 +638,13 @@ func newNopConfigPipelineConfigs(pipelineCfgs pipelines.Config) Config {
 				InitialFields:     map[string]any(nil),
 			},
 			Metrics: telemetry.MetricsConfig{
-				Level:   configtelemetry.LevelBasic,
-				Address: "localhost:8888",
+				Level: configtelemetry.LevelBasic,
+				Readers: []config.MetricReader{{
+					Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{
+						Host: newPtr("localhost"),
+						Port: newPtr(8080),
+					}}}},
+				},
 			},
 		},
 	}
@@ -744,3 +676,7 @@ func newConfigWatcherExtensionFactory(name component.Type) extension.Factory {
 		component.StabilityLevelDevelopment,
 	)
 }
+
+func newPtr[T int | string](str T) *T {
+	return &str
+}

service/telemetry/config.go

@@ -5,14 +5,27 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry"
 
 import (
 	"fmt"
+	"net"
+	"strconv"
 	"time"
 
 	"go.opentelemetry.io/contrib/config"
 	"go.uber.org/zap/zapcore"
 
 	"go.opentelemetry.io/collector/config/configtelemetry"
+	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/featuregate"
 )
 
+var _ confmap.Unmarshaler = (*Config)(nil)
+
+var disableAddressFieldForInternalTelemetryFeatureGate = featuregate.GlobalRegistry().MustRegister(
+	"telemetry.disableAddressFieldForInternalTelemetry",
+	featuregate.StageAlpha,
+	featuregate.WithRegisterFromVersion("v0.110.0"),
+	featuregate.WithRegisterToVersion("v0.110.0"),
+	featuregate.WithRegisterDescription("controls whether the deprecated address field for internal telemetry is still supported"))
+
 // Config defines the configurable settings for service telemetry.
 type Config struct {
 	Logs    LogsConfig    `mapstructure:"logs"`
@@ -119,7 +132,7 @@ type MetricsConfig struct {
 	//  - "detailed" adds dimensions and views to the previous levels.
 	Level configtelemetry.Level `mapstructure:"level"`
 
-	// Address is the [address]:port that metrics exposition should be bound to.
+	// Deprecated: [v0.110.0] use readers configuration.
 	Address string `mapstructure:"address"`
 
 	// Readers allow configuration of metric readers to emit metrics to
@@ -143,11 +156,47 @@ type TracesConfig struct {
 	Processors []config.SpanProcessor `mapstructure:"processors"`
 }
 
+func (c *Config) Unmarshal(conf *confmap.Conf) error {
+	if err := conf.Unmarshal(c); err != nil {
+		return err
+	}
+	if disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled() {
+		if len(c.Metrics.Address) != 0 {
+			host, port, err := net.SplitHostPort(c.Metrics.Address)
+			if err != nil {
+				return fmt.Errorf("failing to parse metrics address %q: %w", c.Metrics.Address, err)
+			}
+			portInt, err := strconv.Atoi(port)
+			if err != nil {
+				return fmt.Errorf("failing to extract the port from the metrics address %q: %w", c.Metrics.Address, err)
+			}
+
+			// User did not overwrite readers, so we will remove the default configured reader.
+			if !conf.IsSet("metrics::readers") {
+				c.Metrics.Readers = nil
+			}
+
+			c.Metrics.Readers = append(c.Metrics.Readers, config.MetricReader{
+				Pull: &config.PullMetricReader{
+					Exporter: config.MetricExporter{
+						Prometheus: &config.Prometheus{
+							Host: &host,
+							Port: &portInt,
+						},
+					},
+				},
+			})
+		}
+
+	}
+	return nil
+}
+
 // Validate checks whether the current configuration is valid
 func (c *Config) Validate() error {
-	// Check when service telemetry metric level is not none, the metrics address should not be empty
-	if c.Metrics.Level != configtelemetry.LevelNone && c.Metrics.Address == "" && len(c.Metrics.Readers) == 0 {
-		return fmt.Errorf("collector telemetry metric address or reader should exist when metric level is not none")
+	// Check when service telemetry metric level is not none, the metrics readers should not be empty
+	if c.Metrics.Level != configtelemetry.LevelNone && len(c.Metrics.Readers) == 0 {
+		return fmt.Errorf("collector telemetry metrics reader should exist when metric level is not none")
 	}
 
 	return nil

service/telemetry/config_test.go

@@ -4,15 +4,60 @@
 package telemetry
 
 import (
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/contrib/config"
 
+	"go.opentelemetry.io/collector/component/componenttest"
 	"go.opentelemetry.io/collector/config/configtelemetry"
+	"go.opentelemetry.io/collector/confmap"
+	"go.opentelemetry.io/collector/confmap/confmaptest"
 )
 
-func TestLoadConfig(t *testing.T) {
+func TestComponentConfigStruct(t *testing.T) {
+	require.NoError(t, componenttest.CheckConfigStruct(NewFactory().CreateDefaultConfig()))
+}
+
+func TestUnmarshalDefaultConfig(t *testing.T) {
+	factory := NewFactory()
+	cfg := factory.CreateDefaultConfig()
+	assert.NoError(t, confmap.New().Unmarshal(&cfg))
+	assert.Equal(t, factory.CreateDefaultConfig(), cfg)
+}
+
+func TestUnmarshalConfigDeprecatedAddress(t *testing.T) {
+	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_deprecated_address.yaml"))
+	require.NoError(t, err)
+	cfg := NewFactory().CreateDefaultConfig()
+	require.NoError(t, cm.Unmarshal(&cfg))
+	require.Len(t, cfg.(*Config).Metrics.Readers, 1)
+	assert.Equal(t, "", *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Host)
+	assert.Equal(t, 6666, *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Port)
+}
+
+func TestUnmarshalConfigInvalidDeprecatedAddress(t *testing.T) {
+	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_invalid_deprecated_address.yaml"))
+	require.NoError(t, err)
+	cfg := NewFactory().CreateDefaultConfig()
+	require.Error(t, cm.Unmarshal(&cfg))
+}
+
+func TestUnmarshalConfigDeprecatedAddressAndReaders(t *testing.T) {
+	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_deprecated_address_and_readers.yaml"))
+	require.NoError(t, err)
+	cfg := NewFactory().CreateDefaultConfig()
+	assert.NoError(t, cm.Unmarshal(&cfg))
+	require.Len(t, cfg.(*Config).Metrics.Readers, 2)
+	assert.Equal(t, "localhost", *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Host)
+	assert.Equal(t, 9999, *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Port)
+	assert.Equal(t, "localhost", *cfg.(*Config).Metrics.Readers[1].Pull.Exporter.Prometheus.Host)
+	assert.Equal(t, 6666, *cfg.(*Config).Metrics.Readers[1].Pull.Exporter.Prometheus.Port)
+}
+
+func TestConfigValidate(t *testing.T) {
 	tests := []struct {
 		name    string
 		cfg     *Config
@@ -22,8 +67,13 @@ func TestLoadConfig(t *testing.T) {
 			name: "basic metric telemetry",
 			cfg: &Config{
 				Metrics: MetricsConfig{
-					Level:   configtelemetry.LevelBasic,
-					Address: "127.0.0.1:3333",
+					Level: configtelemetry.LevelBasic,
+					Readers: []config.MetricReader{{
+						Pull: &config.PullMetricReader{Exporter: config.MetricExporter{Prometheus: &config.Prometheus{
+							Host: newPtr("127.0.0.1"),
+							Port: newPtr(3333),
+						}}}},
+					},
 				},
 			},
 			success: true,
@@ -33,23 +83,11 @@ func TestLoadConfig(t *testing.T) {
 			cfg: &Config{
 				Metrics: MetricsConfig{
 					Level:   configtelemetry.LevelBasic,
-					Address: "",
+					Readers: nil,
 				},
 			},
 			success: false,
 		},
-		{
-			name: "valid metric telemetry with metric readers",
-			cfg: &Config{
-				Metrics: MetricsConfig{
-					Level: configtelemetry.LevelBasic,
-					Readers: []config.MetricReader{
-						{Pull: &config.PullMetricReader{}},
-					},
-				},
-			},
-			success: true,
-		},
 	}
 
 	for _, tt := range tests {