Skip to content

Commit

Permalink
Set Collector Log in New to avoid Shutdown panic (open-telemetry#4827)
Browse files Browse the repository at this point in the history
* Set collector logger to Nop on initialization to ensure Shutdown will not panic if called before logger init

Signed-off-by: Corbin Phelps <corbin.phelps@bluemedora.com>

* Updated changelog

Signed-off-by: Corbin Phelps <corbin.phelps@bluemedora.com>
  • Loading branch information
cpheps authored Feb 17, 2022
1 parent 8d1db96 commit 2a2a722
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 0 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 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 🛑
Expand Down
2 changes: 2 additions & 0 deletions service/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions service/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 2a2a722

Please sign in to comment.