Skip to content

Commit

Permalink
Deprecate service::telemetry::metrics::address (#11205)
Browse files Browse the repository at this point in the history
Existing configs like:

```yaml
service:
  telemetry:
    metrics:
      level: "basic"
      address: "localhost:8888"
```

Should change to:

```yaml
service:
  telemetry:
    metrics:
      level: "basic"
      readers:
        - pull:
            exporter:
              prometheus:
                host: "localhost"
                port: 8888
```

---------

Signed-off-by: Bogdan Drutu <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
  • Loading branch information
bogdandrutu and codeboten authored Sep 25, 2024
1 parent 0ed97aa commit 3eec76c
Show file tree
Hide file tree
Showing 26 changed files with 321 additions and 154 deletions.
20 changes: 20 additions & 0 deletions .chloggen/deprecate-address.yaml
Original file line number Diff line number Diff line change
@@ -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]
4 changes: 2 additions & 2 deletions otelcol/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,8 @@ func TestCollectorRun(t *testing.T) {
tests := []struct {
file string
}{
{file: "otelcol-nometrics.yaml"},
{file: "otelcol-noaddress.yaml"},
{file: "otelcol-noreaders.yaml"},
{file: "otelcol-emptyreaders.yaml"},
}

for _, tt := range tests {
Expand Down
18 changes: 16 additions & 2 deletions otelcol/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -275,8 +276,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")},
Expand All @@ -290,3 +300,7 @@ func generateConfig() *Config {
},
}
}

func newPtr[T int | string](str T) *T {
return &str
}
2 changes: 1 addition & 1 deletion otelcol/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ require (
go.opentelemetry.io/collector/processor v0.110.0
go.opentelemetry.io/collector/receiver v0.110.0
go.opentelemetry.io/collector/service v0.110.0
go.opentelemetry.io/contrib/config v0.10.0
go.uber.org/goleak v1.3.0
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.27.0
Expand Down Expand Up @@ -79,7 +80,6 @@ require (
go.opentelemetry.io/collector/processor/processorprofiles v0.110.0 // indirect
go.opentelemetry.io/collector/receiver/receiverprofiles v0.110.0 // indirect
go.opentelemetry.io/collector/semconv v0.110.0 // indirect
go.opentelemetry.io/contrib/config v0.10.0 // indirect
go.opentelemetry.io/contrib/propagators/b3 v1.30.0 // indirect
go.opentelemetry.io/otel v1.30.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.6.0 // indirect
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ exporters:
service:
telemetry:
metrics:
address: ""
readers: []
pipelines:
metrics:
receivers: [nop]
Expand Down
7 changes: 6 additions & 1 deletion otelcol/testdata/otelcol-invalid-receiver-type.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ exporters:
service:
telemetry:
metrics:
address: localhost:8888
readers:
- pull:
exporter:
prometheus:
host: "localhost"
port: 8888
pipelines:
traces:
receivers: [nop_logs]
Expand Down
7 changes: 6 additions & 1 deletion otelcol/testdata/otelcol-invalid.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ exporters:
service:
telemetry:
metrics:
address: localhost:8888
readers:
- pull:
exporter:
prometheus:
host: "localhost"
port: 8888
pipelines:
traces:
receivers: [nop]
Expand Down
7 changes: 6 additions & 1 deletion otelcol/testdata/otelcol-invalidprop.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ service:
- "unknown"
- "tracecontext"
metrics:
address: localhost:8888
readers:
- pull:
exporter:
prometheus:
host: "localhost"
port: 8888
pipelines:
traces:
receivers: [nop]
Expand Down
7 changes: 6 additions & 1 deletion otelcol/testdata/otelcol-nop.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ connectors:
service:
telemetry:
metrics:
address: localhost:8888
readers:
- pull:
exporter:
prometheus:
host: "localhost"
port: 8888
extensions: [nop]
pipelines:
traces:
Expand Down
File renamed without changes.
7 changes: 6 additions & 1 deletion otelcol/testdata/otelcol-statuswatcher.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ extensions:
service:
telemetry:
metrics:
address: localhost:8888
readers:
- pull:
exporter:
prometheus:
host: "localhost"
port: 8888
extensions: [statuswatcher]
pipelines:
traces:
Expand Down
7 changes: 6 additions & 1 deletion otelcol/testdata/otelcol-validprop.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ service:
- "b3"
- "tracecontext"
metrics:
address: localhost:8888
readers:
- pull:
exporter:
prometheus:
host: "localhost"
port: 8888
pipelines:
traces:
receivers: [nop]
Expand Down
7 changes: 3 additions & 4 deletions otelcol/unmarshaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func TestServiceUnmarshalError(t *testing.T) {
},
},
}),
expectError: "error decoding 'telemetry.logs.level': unrecognized level: \"UNKNOWN\"",
expectError: "error decoding 'telemetry': decoding failed due to the following error(s):\n\nerror decoding 'logs.level': unrecognized level: \"UNKNOWN\"",
},
{
name: "invalid-metrics-level",
Expand All @@ -170,7 +170,7 @@ func TestServiceUnmarshalError(t *testing.T) {
},
},
}),
expectError: "error decoding 'telemetry.metrics.level': unknown metrics level \"unknown\"",
expectError: "error decoding 'telemetry': decoding failed due to the following error(s):\n\nerror decoding 'metrics.level': unknown metrics level \"unknown\"",
},
{
name: "invalid-service-extensions-section",
Expand Down Expand Up @@ -204,8 +204,7 @@ func TestServiceUnmarshalError(t *testing.T) {
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
err := tt.conf.Unmarshal(&service.Config{})
require.Error(t, err)
assert.Contains(t, err.Error(), tt.expectError)
require.ErrorContains(t, err, tt.expectError)
})
}
}
12 changes: 9 additions & 3 deletions service/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -67,7 +68,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,
Expand Down Expand Up @@ -96,8 +97,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")},
Expand Down
1 change: 1 addition & 0 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ func logsAboutMeterProvider(logger *zap.Logger, cfg telemetry.MetricsConfig, mp
return
}

//nolint
if len(cfg.Address) != 0 {
logger.Warn("service::telemetry::metrics::address is being deprecated in favor of service::telemetry::metrics::readers")
}
Expand Down
86 changes: 11 additions & 75 deletions service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,78 +289,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")
Expand Down Expand Up @@ -408,7 +336,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{
Expand Down Expand Up @@ -742,8 +669,13 @@ func newNopConfigPipelineConfigs(pipelineCfgs pipelines.ConfigWithPipelineID) Co
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(8888),
}}}},
},
},
},
}
Expand Down Expand Up @@ -775,3 +707,7 @@ func newConfigWatcherExtensionFactory(name component.Type) extension.Factory {
component.StabilityLevelDevelopment,
)
}

func newPtr[T int | string](str T) *T {
return &str
}
Loading

0 comments on commit 3eec76c

Please sign in to comment.