From 2a2a722cf5683ab7f611d5fef0ecc95a0f345119 Mon Sep 17 00:00:00 2001 From: Corbin Phelps Date: Thu, 17 Feb 2022 14:02:15 -0500 Subject: [PATCH] Set Collector Log in New to avoid Shutdown panic (#4827) * Set collector logger to Nop on initialization to ensure Shutdown will not panic if called before logger init Signed-off-by: Corbin Phelps * Updated changelog Signed-off-by: Corbin Phelps --- CHANGELOG.md | 4 ++++ service/collector.go | 2 ++ service/collector_test.go | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 400bf19db97..4849deb26e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Add validation to check at least one endpoint is specified in otlphttpexporter's configuration (#4860) +## 🧰 Bug fixes 🧰 + +- Initialized logger with collector to avoid potential race condition panic on `Shutdown` (#4827) + ## v0.45.0 Beta ### 🛑 Breaking changes 🛑 diff --git a/service/collector.go b/service/collector.go index a7e76ea57bb..ace363a84ca 100644 --- a/service/collector.go +++ b/service/collector.go @@ -92,6 +92,8 @@ func New(set CollectorSettings) (*Collector, error) { state := atomic.Value{} state.Store(Starting) return &Collector{ + logger: zap.NewNop(), // Set a Nop logger as a place holder until a logger is created based on configuration + set: set, state: state, }, nil diff --git a/service/collector_test.go b/service/collector_test.go index b8e8e1bc4bd..a5825813c41 100644 --- a/service/collector_test.go +++ b/service/collector_test.go @@ -135,6 +135,24 @@ func TestCollector_Start(t *testing.T) { }, time.Second*2, time.Millisecond*200) } +// TestCollector_ShutdownNoop verifies that shutdown can be called even if a collector +// has yet to be started and it will execute without error. +func TestCollector_ShutdownNoop(t *testing.T) { + factories, err := testcomponents.DefaultFactories() + require.NoError(t, err) + + set := CollectorSettings{ + BuildInfo: component.NewDefaultBuildInfo(), + Factories: factories, + ConfigProvider: MustNewDefaultConfigProvider([]string{filepath.Join("testdata", "otelcol-config.yaml")}, nil), + } + col, err := New(set) + require.NoError(t, err) + + // Should be able to call Shutdown on an unstarted collector and nothing happens + require.NotPanics(t, func() { col.Shutdown() }) +} + type mockColTelemetry struct{} func (tel *mockColTelemetry) init(*Collector) error {