From ca4cdfe4c0de07ac63889f8eb38f44e3cdc50a7b Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Fri, 16 Dec 2022 11:38:01 -0800 Subject: [PATCH] small contingency improvement in global error handler (#3543) Signed-off-by: Bogdan Drutu Signed-off-by: Bogdan Drutu --- CHANGELOG.md | 4 ++++ handler.go | 25 ++++++++++++------------- handler_test.go | 6 +++--- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f0e0943382..40dc1d752ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add `Producer` interface and `Reader.RegisterProducer(Producer)` to `go.opentelemetry.io/otel/sdk/metric` to enable external metric Producers. (#3524) +### Changed + +- Global error handler uses an atomic value instead of a mutex. (#3543) + ## [1.11.2/0.34.0] 2022-12-05 ### Added diff --git a/handler.go b/handler.go index 36cf09f7290..ecd363ab516 100644 --- a/handler.go +++ b/handler.go @@ -17,7 +17,8 @@ package otel // import "go.opentelemetry.io/otel" import ( "log" "os" - "sync" + "sync/atomic" + "unsafe" ) var ( @@ -34,28 +35,26 @@ var ( ) type delegator struct { - lock *sync.RWMutex - eh ErrorHandler + delegate unsafe.Pointer } func (d *delegator) Handle(err error) { - d.lock.RLock() - defer d.lock.RUnlock() - d.eh.Handle(err) + d.getDelegate().Handle(err) +} + +func (d *delegator) getDelegate() ErrorHandler { + return *(*ErrorHandler)(atomic.LoadPointer(&d.delegate)) } // setDelegate sets the ErrorHandler delegate. func (d *delegator) setDelegate(eh ErrorHandler) { - d.lock.Lock() - defer d.lock.Unlock() - d.eh = eh + atomic.StorePointer(&d.delegate, unsafe.Pointer(&eh)) } func defaultErrorHandler() *delegator { - return &delegator{ - lock: &sync.RWMutex{}, - eh: &errLogger{l: log.New(os.Stderr, "", log.LstdFlags)}, - } + d := &delegator{} + d.setDelegate(&errLogger{l: log.New(os.Stderr, "", log.LstdFlags)}) + return d } // errLogger logs errors if no delegate is set, otherwise they are delegated. diff --git a/handler_test.go b/handler_test.go index 32906198f8c..b6b9b20cae0 100644 --- a/handler_test.go +++ b/handler_test.go @@ -54,7 +54,7 @@ type HandlerTestSuite struct { func (s *HandlerTestSuite) SetupSuite() { s.errCatcher = new(testErrCatcher) - s.origHandler = globalErrorHandler.eh + s.origHandler = globalErrorHandler.getDelegate() globalErrorHandler.setDelegate(&errLogger{l: log.New(s.errCatcher, "", 0)}) } @@ -111,12 +111,12 @@ func (s *HandlerTestSuite) TestAllowMultipleSets() { secondary := &errLogger{l: log.New(notUsed, "", 0)} SetErrorHandler(secondary) s.Require().Same(GetErrorHandler(), globalErrorHandler, "set changed globalErrorHandler") - s.Require().Same(globalErrorHandler.eh, secondary, "new Handler not set") + s.Require().Same(globalErrorHandler.getDelegate(), secondary, "new Handler not set") tertiary := &errLogger{l: log.New(notUsed, "", 0)} SetErrorHandler(tertiary) s.Require().Same(GetErrorHandler(), globalErrorHandler, "set changed globalErrorHandler") - s.Assert().Same(globalErrorHandler.eh, tertiary, "user Handler not overridden") + s.Assert().Same(globalErrorHandler.getDelegate(), tertiary, "user Handler not overridden") } func TestHandlerTestSuite(t *testing.T) {