Skip to content

Mark telemetry.disableAddressFieldForInternalTelemetry as stable #13152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .chloggen/disableAddressFieldForInternalTelemetry.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# 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. otlpreceiver)
component: service/telemetry

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Mark "telemetry.disableAddressFieldForInternalTelemetry" as stable

# One or more tracking issues or pull requests related to the change
issues: [13152]

# (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:

# 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]
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ change_type: enhancement
component: service

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Remove stablized featuregate useOtelWithSDKConfigurationForInternalTelemetry
note: Remove stabilized featuregate useOtelWithSDKConfigurationForInternalTelemetry

# One or more tracking issues or pull requests related to the change
issues: [13152]
Expand Down
4 changes: 2 additions & 2 deletions otelcol/unmarshaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func TestServiceUnmarshalError(t *testing.T) {
},
},
}),
expectError: "error decoding 'telemetry': decoding failed due to the following error(s):\n\nerror decoding 'logs': decoding failed due to the following error(s):\n\nerror decoding 'level': unrecognized level: \"UNKNOWN\"",
expectError: "decoding failed due to the following error(s):\n\nerror decoding 'telemetry.logs': decoding failed due to the following error(s):\n\nerror decoding 'level': unrecognized level: \"UNKNOWN\"",
},
{
name: "invalid-metrics-level",
Expand All @@ -168,7 +168,7 @@ func TestServiceUnmarshalError(t *testing.T) {
},
},
}),
expectError: "error decoding 'telemetry': decoding failed due to the following error(s):\n\nerror decoding 'metrics': decoding failed due to the following error(s):\n\nerror decoding 'level': unknown metrics level \"unknown\"",
expectError: "decoding failed due to the following error(s):\n\nerror decoding 'telemetry.metrics': decoding failed due to the following error(s):\n\nerror decoding 'level': unknown metrics level \"unknown\"",
},
{
name: "invalid-service-extensions-section",
Expand Down
6 changes: 1 addition & 5 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func New(ctx context.Context, set Settings, cfg Config) (*Service, error) {
return nil, err
}

if cfg.Telemetry.Metrics.Level != configtelemetry.LevelNone && (len(mpConfig.Readers) != 0 || cfg.Telemetry.Metrics.Address != "") {
if cfg.Telemetry.Metrics.Level != configtelemetry.LevelNone && len(mpConfig.Readers) != 0 {
if err = proctelemetry.RegisterProcessMetrics(srv.telemetrySettings); err != nil {
return nil, fmt.Errorf("failed to register process metrics: %w", err)
}
Expand All @@ -242,10 +242,6 @@ func logsAboutMeterProvider(logger *zap.Logger, cfg telemetry.MetricsConfig, mp
return
}

if len(cfg.Address) != 0 {
logger.Warn("service::telemetry::metrics::address is being deprecated in favor of service::telemetry::metrics::readers")
}

if lmp, ok := mp.(interface {
LogAboutServers(logger *zap.Logger, cfg telemetry.MetricsConfig)
}); ok {
Expand Down
61 changes: 3 additions & 58 deletions service/telemetry/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,17 @@ package telemetry // import "go.opentelemetry.io/collector/service/telemetry"

import (
"errors"
"fmt"
"net"
"strconv"

config "go.opentelemetry.io/contrib/otelconf/v0.3.0"

"go.opentelemetry.io/collector/config/configtelemetry"
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/service/telemetry/internal/migration"
)

var _ confmap.Unmarshaler = (*Config)(nil)

var disableAddressFieldForInternalTelemetryFeatureGate = featuregate.GlobalRegistry().MustRegister(
var _ = featuregate.GlobalRegistry().MustRegister(
"telemetry.disableAddressFieldForInternalTelemetry",
featuregate.StageBeta,
featuregate.StageStable,
featuregate.WithRegisterFromVersion("v0.111.0"),
featuregate.WithRegisterToVersion("v0.123.0"),
featuregate.WithRegisterToVersion("v0.127.0"),
featuregate.WithRegisterDescription("controls whether the deprecated address field for internal telemetry is still supported"))

// Config defines the configurable settings for service telemetry.
Expand Down Expand Up @@ -57,53 +49,6 @@ type MetricsConfig = migration.MetricsConfigV030
// Experimental: *NOTE* this structure is subject to change or removal in the future.
type TracesConfig = migration.TracesConfigV030

func (c *Config) Unmarshal(conf *confmap.Conf) error {
if err := conf.Unmarshal(c); err != nil {
return err
}

// If the support for "metrics::address" is disabled, nothing to do.
// TODO: when this gate is marked stable remove the whole Unmarshal definition.
if disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled() {
return nil
}

if len(c.Metrics.Address) != 0 { //nolint:staticcheck // SA1019
host, port, err := net.SplitHostPort(c.Metrics.Address) //nolint:staticcheck // SA1019
if err != nil {
return fmt.Errorf("failing to parse metrics address %q: %w", c.Metrics.Address, err) //nolint:staticcheck // SA1019
}
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) //nolint:staticcheck // SA1019
}

// 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.PullMetricExporter{
Prometheus: &config.Prometheus{
Host: &host,
Port: &portInt,
WithoutScopeInfo: newPtr(true),
WithoutUnits: newPtr(true),
WithoutTypeSuffix: newPtr(true),
WithResourceConstantLabels: &config.IncludeExclude{
Included: []string{},
},
},
},
},
})
}

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 readers should not be empty
Expand Down
70 changes: 0 additions & 70 deletions service/telemetry/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"go.opentelemetry.io/collector/config/configtelemetry"
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/confmap/confmaptest"
"go.opentelemetry.io/collector/featuregate"
)

func TestComponentConfigStruct(t *testing.T) {
Expand All @@ -30,82 +29,13 @@ func TestUnmarshalDefaultConfig(t *testing.T) {
}

func TestUnmarshalEmptyMetricReaders(t *testing.T) {
prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), false))
defer func() {
// Restore previous value.
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev))
}()
cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_empty_readers.yaml"))
require.NoError(t, err)
cfg := NewFactory().CreateDefaultConfig()
require.NoError(t, cm.Unmarshal(&cfg))
require.Empty(t, cfg.(*Config).Metrics.Readers)
}

func TestUnmarshalConfigDeprecatedAddress(t *testing.T) {
prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), false))
defer func() {
// Restore previous value.
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev))
}()
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, "localhost", *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Host)
assert.Equal(t, 6666, *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Port)
}

func TestUnmarshalConfigDeprecatedAddressGateEnabled(t *testing.T) {
prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), true))
defer func() {
// Restore previous value.
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev))
}()
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, "localhost", *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Host)
assert.Equal(t, 8888, *cfg.(*Config).Metrics.Readers[0].Pull.Exporter.Prometheus.Port)
}

func TestUnmarshalConfigInvalidDeprecatedAddress(t *testing.T) {
prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), false))
defer func() {
// Restore previous value.
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev))
}()
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) {
prev := disableAddressFieldForInternalTelemetryFeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), false))
defer func() {
// Restore previous value.
require.NoError(t, featuregate.GlobalRegistry().Set(disableAddressFieldForInternalTelemetryFeatureGate.ID(), prev))
}()
cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_deprecated_address_and_readers.yaml"))
require.NoError(t, err)
cfg := NewFactory().CreateDefaultConfig()
require.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, "192.168.0.1", *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
Expand Down
2 changes: 0 additions & 2 deletions service/telemetry/internal/migration/v0.2.0.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ type tracesConfigV020 struct {

type metricsConfigV020 struct {
Level configtelemetry.Level `mapstructure:"level"`
Address string `mapstructure:"address"`
Readers []configv02.MetricReader `mapstructure:"readers"`
}

Expand Down Expand Up @@ -166,7 +165,6 @@ func metricsConfigV02ToV03(v2 metricsConfigV020, v3 *MetricsConfigV030) error {
}
}
v3.Level = v2.Level
v3.Address = v2.Address
v3.Readers = readers
return nil
}
Expand Down
6 changes: 1 addition & 5 deletions service/telemetry/internal/migration/v0.3.0.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,14 @@ type MetricsConfigV030 struct {
// - "detailed" adds dimensions and views to the previous levels.
Level configtelemetry.Level `mapstructure:"level"`

// Deprecated: [v0.111.0] use readers configuration.
Address string `mapstructure:"address,omitempty"`

config.MeterProvider `mapstructure:",squash"`
}

func (c *MetricsConfigV030) Unmarshal(conf *confmap.Conf) error {
unmarshaled := *c
if err := conf.Unmarshal(c); err != nil {
v02 := metricsConfigV020{
Level: unmarshaled.Level,
Address: unmarshaled.Address,
Level: unmarshaled.Level,
}
// try unmarshaling using v0.2.0 struct
if e := conf.Unmarshal(&v02); e != nil {
Expand Down
Loading