Skip to content

Commit

Permalink
Remove the oterror package (#1026)
Browse files Browse the repository at this point in the history
* Break up the oterror package

* Update use of ErrorHandler in project

* Update handler naming and comments
  • Loading branch information
MrAlias authored Aug 5, 2020
1 parent 6740888 commit 3780b80
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 97 deletions.
61 changes: 30 additions & 31 deletions api/global/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,73 +20,72 @@ import (
"sync"
"sync/atomic"

"go.opentelemetry.io/otel/api/oterror"
"go.opentelemetry.io/otel"
)

var (
// globalHandler provides an oterror.Handler that can be used throughout
// an OpenTelemetry instrumented project. When a user specified Handler
// is registered (`SetHandler`) all calls to `Handle` will be delegated
// to the registered Handler.
globalHandler = &handler{
// globalErrorHandler provides an ErrorHandler that can be used
// throughout an OpenTelemetry instrumented project. When a user
// specified ErrorHandler is registered (`SetErrorHandler`) all calls to
// `Handle` and will be delegated to the registered ErrorHandler.
globalErrorHandler = &loggingErrorHandler{
l: log.New(os.Stderr, "", log.LstdFlags),
}

// delegateHanderOnce ensures that a user provided Handler is only ever
// registered once.
delegateHanderOnce sync.Once
// delegateErrorHandlerOnce ensures that a user provided ErrorHandler is
// only ever registered once.
delegateErrorHandlerOnce sync.Once

// Ensure the handler implements oterror.Handle at build time.
_ oterror.Handler = (*handler)(nil)
// Comiple time check that loggingErrorHandler implements ErrorHandler.
_ otel.ErrorHandler = (*loggingErrorHandler)(nil)
)

// handler logs all errors to STDERR.
type handler struct {
// loggingErrorHandler logs all errors to STDERR.
type loggingErrorHandler struct {
delegate atomic.Value

l *log.Logger
}

// setDelegate sets the handler delegate if one is not already set.
func (h *handler) setDelegate(d oterror.Handler) {
// setDelegate sets the ErrorHandler delegate if one is not already set.
func (h *loggingErrorHandler) setDelegate(d otel.ErrorHandler) {
if h.delegate.Load() != nil {
// Delegate already registered
return
}
h.delegate.Store(d)
}

// Handle implements oterror.Handler.
func (h *handler) Handle(err error) {
// Handle implements otel.ErrorHandler.
func (h *loggingErrorHandler) Handle(err error) {
if d := h.delegate.Load(); d != nil {
d.(oterror.Handler).Handle(err)
d.(otel.ErrorHandler).Handle(err)
return
}
h.l.Print(err)
}

// Handler returns the global Handler instance. If no Handler instance has
// be explicitly set yet, a default Handler is returned that logs to STDERR
// until an Handler is set (all functionality is delegated to the set
// Handler once it is set).
func Handler() oterror.Handler {
return globalHandler
// ErrorHandler returns the global ErrorHandler instance. If no ErrorHandler
// instance has been set (`SetErrorHandler`), the default ErrorHandler which
// logs errors to STDERR is returned.
func ErrorHandler() otel.ErrorHandler {
return globalErrorHandler
}

// SetHandler sets the global Handler to be h.
func SetHandler(h oterror.Handler) {
delegateHanderOnce.Do(func() {
current := Handler()
// SetErrorHandler sets the global ErrorHandler to be h.
func SetErrorHandler(h otel.ErrorHandler) {
delegateErrorHandlerOnce.Do(func() {
current := ErrorHandler()
if current == h {
return
}
if internalHandler, ok := current.(*handler); ok {
if internalHandler, ok := current.(*loggingErrorHandler); ok {
internalHandler.setDelegate(h)
}
})
}

// Handle is a convience function for Handler().Handle(err)
// Handle is a convience function for ErrorHandler().Handle(err)
func Handle(err error) {
globalHandler.Handle(err)
ErrorHandler().Handle(err)
}
14 changes: 7 additions & 7 deletions api/global/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,20 @@ func (l *errLogger) Got() []string {
type HandlerTestSuite struct {
suite.Suite

origHandler *handler
origHandler *loggingErrorHandler
errLogger *errLogger
}

func (s *HandlerTestSuite) SetupSuite() {
s.errLogger = new(errLogger)
s.origHandler = globalHandler
globalHandler = &handler{
s.origHandler = globalErrorHandler
globalErrorHandler = &loggingErrorHandler{
l: log.New(s.errLogger, "", 0),
}
}

func (s *HandlerTestSuite) TearDownSuite() {
globalHandler = s.origHandler
globalErrorHandler = s.origHandler
}

func (s *HandlerTestSuite) SetupTest() {
Expand All @@ -66,7 +66,7 @@ func (s *HandlerTestSuite) SetupTest() {

func (s *HandlerTestSuite) TestGlobalHandler() {
errs := []string{"one", "two"}
Handler().Handle(errors.New(errs[0]))
ErrorHandler().Handle(errors.New(errs[0]))
Handle(errors.New(errs[1]))
s.Assert().Equal(errs, s.errLogger.Got())
}
Expand Down Expand Up @@ -122,10 +122,10 @@ func (s *HandlerTestSuite) TestNoDropsOnDelegate() {

// Change to another Handler. We are testing this is loss-less.
newErrLogger := new(errLogger)
secondary := &handler{
secondary := &loggingErrorHandler{
l: log.New(newErrLogger, "", 0),
}
SetHandler(secondary)
SetErrorHandler(secondary)
s.Require().NoError(wait(pause), "switched to new Handler")

// Testing done, stop sending errors.
Expand Down
3 changes: 1 addition & 2 deletions api/metric/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"go.opentelemetry.io/otel/api/kv"
"go.opentelemetry.io/otel/api/metric"
"go.opentelemetry.io/otel/api/oterror"
"go.opentelemetry.io/otel/api/unit"
mockTest "go.opentelemetry.io/otel/internal/metric"

Expand Down Expand Up @@ -392,7 +391,7 @@ func TestWrappedInstrumentError(t *testing.T) {

valuerecorder, err := meter.NewInt64ValueRecorder("test.valuerecorder")

require.Equal(t, err, oterror.ErrSDKReturnedNilImpl)
require.Equal(t, err, metric.ErrSDKReturnedNilImpl)
require.NotNil(t, valuerecorder.SyncImpl())

observer, err := meter.NewInt64ValueObserver("test.observer", func(_ context.Context, result metric.Int64ObserverResult) {})
Expand Down
9 changes: 6 additions & 3 deletions api/metric/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@ package metric

import (
"context"
"errors"

"go.opentelemetry.io/otel/api/kv"
"go.opentelemetry.io/otel/api/oterror"
)

// ErrSDKReturnedNilImpl is returned when a new `MeterImpl` returns nil.
var ErrSDKReturnedNilImpl = errors.New("SDK returned a nil implementation")

// Measurement is used for reporting a synchronous batch of metric
// values. Instances of this type should be created by synchronous
// instruments (e.g., Int64Counter.Measurement()).
Expand Down Expand Up @@ -110,7 +113,7 @@ func (h syncBoundInstrument) Unbind() {
func checkNewAsync(instrument AsyncImpl, err error) (asyncInstrument, error) {
if instrument == nil {
if err == nil {
err = oterror.ErrSDKReturnedNilImpl
err = ErrSDKReturnedNilImpl
}
instrument = NoopAsync{}
}
Expand All @@ -125,7 +128,7 @@ func checkNewAsync(instrument AsyncImpl, err error) (asyncInstrument, error) {
func checkNewSync(instrument SyncImpl, err error) (syncInstrument, error) {
if instrument == nil {
if err == nil {
err = oterror.ErrSDKReturnedNilImpl
err = ErrSDKReturnedNilImpl
}
// Note: an alternate behavior would be to synthesize a new name
// or group all duplicately-named instruments of a certain type
Expand Down
26 changes: 0 additions & 26 deletions api/oterror/doc.go

This file was deleted.

22 changes: 0 additions & 22 deletions api/oterror/errors.go

This file was deleted.

6 changes: 3 additions & 3 deletions api/oterror/handler.go → error_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package oterror
package otel

// Handler handles irremediable events.
type Handler interface {
// ErrorHandler handles irremediable events.
type ErrorHandler interface {
// Handle handles any error deemed irremediable by an OpenTelemetry
// component.
Handle(error)
Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/controller/push/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ var testHandler *handler

func init() {
testHandler = new(handler)
global.SetHandler(testHandler)
global.SetErrorHandler(testHandler)
}

type testExporter struct {
Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/correct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var testHandler *handler

func init() {
testHandler = new(handler)
global.SetHandler(testHandler)
global.SetErrorHandler(testHandler)
}

type correctnessProcessor struct {
Expand Down
2 changes: 1 addition & 1 deletion sdk/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func init() {
tid, _ = apitrace.IDFromHex("01020304050607080102040810203040")
sid, _ = apitrace.SpanIDFromHex("0102040810203040")

global.SetHandler(new(discardHandler))
global.SetErrorHandler(new(discardHandler))
}

func TestTracerFollowsExpectedAPIBehaviour(t *testing.T) {
Expand Down

0 comments on commit 3780b80

Please sign in to comment.