Skip to content

Commit

Permalink
Fix panic when telemetry metrics are disabled (open-telemetry#5642)
Browse files Browse the repository at this point in the history
* [service] Add unit test for running the Collector with telemetry metrics disabled

* Only register process metrics when telemetry metrics are enabled

* Restore `require.NoPanics` assertion

* Delete host_test.go.bak

* Add unit test when address is empty

* Address review comment

oops
  • Loading branch information
mx-psi authored Jul 8, 2022
1 parent feab949 commit 58e66af
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- Add `linux-ppc64le` architecture to cross build tests in CI

### 🧰 Bug fixes 🧰

- Fix Collector panic when disabling telemetry metrics (#5642)

## v0.55.0 Beta

### 🛑 Breaking changes 🛑
Expand Down
34 changes: 34 additions & 0 deletions service/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,40 @@ func TestCollectorStartWithOpenTelemetryMetrics(t *testing.T) {
}
}

func TestCollectorRun(t *testing.T) {
tests := []struct {
file string
}{
{file: "otelcol-nometrics.yaml"},
{file: "otelcol-noaddress.yaml"},
}

for _, tt := range tests {
t.Run(tt.file, func(t *testing.T) {
factories, err := componenttest.NopFactories()
require.NoError(t, err)

cfgProvider, err := NewConfigProvider(newDefaultConfigProviderSettings([]string{filepath.Join("testdata", tt.file)}))
require.NoError(t, err)

set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: factories,
ConfigProvider: cfgProvider,
telemetry: newColTelemetry(featuregate.NewRegistry()),
}
col, err := New(set)
require.NoError(t, err)

wg := startCollector(context.Background(), t, col)

col.Shutdown()
wg.Wait()
assert.Equal(t, Closed, col.GetState())
})
}
}

func TestCollectorShutdownBeforeRun(t *testing.T) {
factories, err := componenttest.NopFactories()
require.NoError(t, err)
Expand Down
9 changes: 6 additions & 3 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"go.uber.org/zap"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configtelemetry"
"go.opentelemetry.io/collector/service/internal"
"go.opentelemetry.io/collector/service/internal/extensions"
"go.opentelemetry.io/collector/service/internal/pipelines"
Expand Down Expand Up @@ -97,9 +98,11 @@ func newService(set *settings) (*service, error) {
return nil, fmt.Errorf("cannot build pipelines: %w", err)
}

// The process telemetry initialization requires the ballast size, which is available after the extensions are initialized.
if err = telemetry.RegisterProcessMetrics(srv.telemetryInitializer.ocRegistry, getBallastSize(srv.host)); err != nil {
return nil, fmt.Errorf("failed to register process metrics: %w", err)
if set.Config.Telemetry.Metrics.Level != configtelemetry.LevelNone && set.Config.Telemetry.Metrics.Address != "" {
// The process telemetry initialization requires the ballast size, which is available after the extensions are initialized.
if err = telemetry.RegisterProcessMetrics(srv.telemetryInitializer.ocRegistry, getBallastSize(srv.host)); err != nil {
return nil, fmt.Errorf("failed to register process metrics: %w", err)
}
}

return srv, nil
Expand Down
14 changes: 14 additions & 0 deletions service/testdata/otelcol-noaddress.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
receivers:
nop:

exporters:
nop:

service:
telemetry:
metrics:
address: ""
pipelines:
metrics:
receivers: [nop]
exporters: [nop]
14 changes: 14 additions & 0 deletions service/testdata/otelcol-nometrics.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
receivers:
nop:

exporters:
nop:

service:
telemetry:
metrics:
level: none
pipelines:
metrics:
receivers: [nop]
exporters: [nop]

0 comments on commit 58e66af

Please sign in to comment.