From acab076d342c564062dfda9e30e5340dae9993d6 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Tue, 20 Jul 2021 15:25:06 -0700 Subject: [PATCH 1/4] use the otel ErrorHandler for handling errors. Fixes #189 --- exporter/trace/cloudtrace.go | 20 ++++++++++---------- exporter/trace/cloudtrace_test.go | 18 ++++++++++++------ exporter/trace/trace.go | 2 +- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/exporter/trace/cloudtrace.go b/exporter/trace/cloudtrace.go index 2e953601f..f9fde655e 100644 --- a/exporter/trace/cloudtrace.go +++ b/exporter/trace/cloudtrace.go @@ -19,7 +19,6 @@ import ( "context" "errors" "fmt" - "log" "time" "go.opentelemetry.io/otel" @@ -60,11 +59,12 @@ type options struct { // on-premise resource like k8s_container or generic_task. Location string - // OnError is the hook to be called when there is + // errorHandler is the hook to be called when there is // an error uploading the stats or tracing data. - // If no custom hook is set, errors are logged. + // If no custom hook is set, errors are handled with the + // OpenTelemetry global handler, which defaults to logging. // Optional. - OnError func(err error) + errorHandler otel.ErrorHandler // TraceClientOptions are additional options to be passed // to the underlying Stackdriver Trace API client. @@ -110,12 +110,12 @@ func WithProjectID(projectID string) func(o *options) { } } -// WithOnError sets the hook to be called when there is an error +// WithErrorHandler sets the hook to be called when there is an error // occurred on uploading the span data to Stackdriver. // If no custom hook is set, errors are logged. -func WithOnError(onError func(err error)) func(o *options) { +func WithErrorHandler(handler otel.ErrorHandler) func(o *options) { return func(o *options) { - o.OnError = onError + o.errorHandler = handler } } @@ -158,11 +158,11 @@ func WithDefaultTraceAttributes(attr map[string]interface{}) func(o *options) { } func (o *options) handleError(err error) { - if o.OnError != nil { - o.OnError(err) + if o.errorHandler != nil { + o.errorHandler.Handle(err) return } - log.Printf("Failed to export to Stackdriver: %v", err) + otel.Handle(err) } // defaultTimeout is used as default when timeout is not set in newContextWithTimout. diff --git a/exporter/trace/cloudtrace_test.go b/exporter/trace/cloudtrace_test.go index 017b44b48..92188ae1a 100644 --- a/exporter/trace/cloudtrace_test.go +++ b/exporter/trace/cloudtrace_test.go @@ -141,12 +141,20 @@ func TestExporter_DisplayNameFormatter(t *testing.T) { assert.EqualValues(t, "TEST_FORMAT"+spanName, mock.GetSpan(0).DisplayName.Value) } +type errorHandler struct { + errs []error +} + +func (e *errorHandler) Handle(err error) { + e.errs = append(e.errs, err) +} + func TestExporter_Timeout(t *testing.T) { // Initial test precondition mock := cloudmock.NewCloudMock() mock.SetDelay(20 * time.Millisecond) clientOpt := []option.ClientOption{option.WithGRPCConn(mock.ClientConn())} - var exportErrors []error + handler := &errorHandler{} // Create Google Cloud Trace Exporter _, shutdown, err := InstallNewPipeline( @@ -155,9 +163,7 @@ func TestExporter_Timeout(t *testing.T) { WithTraceClientOptions(clientOpt), WithTimeout(1 * time.Millisecond), // handle bundle as soon as span is received - WithOnError(func(err error) { - exportErrors = append(exportErrors, err) - }), + WithErrorHandler(handler), }, sdktrace.WithSampler(sdktrace.AlwaysSample()), ) @@ -170,10 +176,10 @@ func TestExporter_Timeout(t *testing.T) { // wait for error to be handled shutdown() // closed grpc connection assert.EqualValues(t, 0, mock.GetNumSpans()) - if got, want := len(exportErrors), 1; got != want { + if got, want := len(handler.errs), 1; got != want { t.Fatalf("len(exportErrors) = %q; want %q", got, want) } - got, want := exportErrors[0].Error(), "rpc error: code = (DeadlineExceeded|Unknown) desc = context deadline exceeded" + got, want := handler.errs[0].Error(), "rpc error: code = (DeadlineExceeded|Unknown) desc = context deadline exceeded" if match, _ := regexp.MatchString(want, got); !match { t.Fatalf("err.Error() = %q; want %q", got, want) } diff --git a/exporter/trace/trace.go b/exporter/trace/trace.go index 4b5acc262..a31277ba6 100644 --- a/exporter/trace/trace.go +++ b/exporter/trace/trace.go @@ -98,7 +98,7 @@ func (e *traceExporter) uploadSpans(ctx context.Context, spans []*tracepb.Span) if err != nil { // TODO(ymotongpoo): handle detailed error categories // span.SetStatus(codes.Unknown) - e.o.handleError(err) + e.o.handleError(fmt.Errorf("failed to export to Stackdriver: %v", err)) } return err } From 96b96517380e5906be225362530addd328219dfa Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 27 Jul 2021 13:52:53 -0400 Subject: [PATCH 2/4] Fix conflicts better --- e2e-test-server/go.sum | 4 ---- exporter/trace/cloudtrace.go | 1 + exporter/trace/cloudtrace_test.go | 12 ++++++++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/e2e-test-server/go.sum b/e2e-test-server/go.sum index bc1bd1942..a1b17a021 100644 --- a/e2e-test-server/go.sum +++ b/e2e-test-server/go.sum @@ -144,10 +144,8 @@ github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1 github.com/jstemmer/go-junit-report v0.9.1 h1:6QPYqodiu3GuPL+7mfx+NwDdp2eTkp9IfEUpgAwUN0o= github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= -github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -183,7 +181,6 @@ go.opentelemetry.io/otel/internal/metric v0.21.0 h1:gZlIBo5O51hZOOZz8vEcuRx/l5dn go.opentelemetry.io/otel/internal/metric v0.21.0/go.mod h1:iOfAaY2YycsXfYD4kaRSbLx2LKmfpKObWBEv9QK5zFo= go.opentelemetry.io/otel/metric v0.21.0 h1:ZtcJlHqVE4l8Su0WOLOd9fEPheJuYEiQ0wr9wv2p25I= go.opentelemetry.io/otel/metric v0.21.0/go.mod h1:JWCt1bjivC4iCrz/aCrM1GSw+ZcvY44KCbaeeRhzHnc= -go.opentelemetry.io/otel/oteltest v1.0.0-RC1 h1:G685iP3XiskCwk/z0eIabL55XUl2gk0cljhGk9sB0Yk= go.opentelemetry.io/otel/oteltest v1.0.0-RC1/go.mod h1:+eoIG0gdEOaPNftuy1YScLr1Gb4mL/9lpDkZ0JjMRq4= go.opentelemetry.io/otel/sdk v1.0.0-RC2 h1:ROuteeSCBaZNjiT9JcFzZepmInDvLktR28Y6qKo8bCs= go.opentelemetry.io/otel/sdk v1.0.0-RC2/go.mod h1:fgwHyiDn4e5k40TD9VX243rOxXR+jzsWBZYA2P5jpEw= @@ -526,7 +523,6 @@ google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp0 google.golang.org/protobuf v1.26.0 h1:bxAC2xTBsZGibn2RTntX0oH50xLsqy1OxA9tTL3p/lk= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/exporter/trace/cloudtrace.go b/exporter/trace/cloudtrace.go index 37bb92269..051c96603 100644 --- a/exporter/trace/cloudtrace.go +++ b/exporter/trace/cloudtrace.go @@ -21,6 +21,7 @@ import ( "fmt" "time" + "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" sdktrace "go.opentelemetry.io/otel/sdk/trace" diff --git a/exporter/trace/cloudtrace_test.go b/exporter/trace/cloudtrace_test.go index 8cc8ba671..08664fe27 100644 --- a/exporter/trace/cloudtrace_test.go +++ b/exporter/trace/cloudtrace_test.go @@ -82,6 +82,14 @@ func TestExporter_ExportSpan(t *testing.T) { assert.Len(t, mock.GetSpan(1).Links.Link, 1) } +type errorHandler struct { + errs []error +} + +func (e *errorHandler) Handle(err error) { + e.errs = append(e.errs, err) +} + func TestExporter_Timeout(t *testing.T) { // Initial test precondition mock := cloudmock.NewCloudMock() @@ -95,8 +103,8 @@ func TestExporter_Timeout(t *testing.T) { WithTraceClientOptions(clientOpt), WithTimeout(1*time.Millisecond), // handle bundle as soon as span is received - WithErrorHandler(handler), - ) + WithErrorHandler(handler), + ) assert.NoError(t, err) tp := sdktrace.NewTracerProvider( sdktrace.WithSampler(sdktrace.AlwaysSample()), From aad7c42053aae284ad03ea37fe7088c5f3f79105 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 27 Jul 2021 18:26:37 +0000 Subject: [PATCH 3/4] Commit go.sum changes that only happen under Go 1.14 --- e2e-test-server/go.sum | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/e2e-test-server/go.sum b/e2e-test-server/go.sum index a1b17a021..bc1bd1942 100644 --- a/e2e-test-server/go.sum +++ b/e2e-test-server/go.sum @@ -144,8 +144,10 @@ github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1 github.com/jstemmer/go-junit-report v0.9.1 h1:6QPYqodiu3GuPL+7mfx+NwDdp2eTkp9IfEUpgAwUN0o= github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -181,6 +183,7 @@ go.opentelemetry.io/otel/internal/metric v0.21.0 h1:gZlIBo5O51hZOOZz8vEcuRx/l5dn go.opentelemetry.io/otel/internal/metric v0.21.0/go.mod h1:iOfAaY2YycsXfYD4kaRSbLx2LKmfpKObWBEv9QK5zFo= go.opentelemetry.io/otel/metric v0.21.0 h1:ZtcJlHqVE4l8Su0WOLOd9fEPheJuYEiQ0wr9wv2p25I= go.opentelemetry.io/otel/metric v0.21.0/go.mod h1:JWCt1bjivC4iCrz/aCrM1GSw+ZcvY44KCbaeeRhzHnc= +go.opentelemetry.io/otel/oteltest v1.0.0-RC1 h1:G685iP3XiskCwk/z0eIabL55XUl2gk0cljhGk9sB0Yk= go.opentelemetry.io/otel/oteltest v1.0.0-RC1/go.mod h1:+eoIG0gdEOaPNftuy1YScLr1Gb4mL/9lpDkZ0JjMRq4= go.opentelemetry.io/otel/sdk v1.0.0-RC2 h1:ROuteeSCBaZNjiT9JcFzZepmInDvLktR28Y6qKo8bCs= go.opentelemetry.io/otel/sdk v1.0.0-RC2/go.mod h1:fgwHyiDn4e5k40TD9VX243rOxXR+jzsWBZYA2P5jpEw= @@ -523,6 +526,7 @@ google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp0 google.golang.org/protobuf v1.26.0 h1:bxAC2xTBsZGibn2RTntX0oH50xLsqy1OxA9tTL3p/lk= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= From 2258e534d0428d59ad0419dc7b76bd9cc356c9cd Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Tue, 27 Jul 2021 14:44:28 -0400 Subject: [PATCH 4/4] Update exporter/trace/trace.go Co-authored-by: Punya Biswal --- exporter/trace/trace.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporter/trace/trace.go b/exporter/trace/trace.go index ec941fe21..020ad41f6 100644 --- a/exporter/trace/trace.go +++ b/exporter/trace/trace.go @@ -98,7 +98,7 @@ func (e *traceExporter) uploadSpans(ctx context.Context, spans []*tracepb.Span) if err != nil { // TODO(ymotongpoo): handle detailed error categories // span.SetStatus(codes.Unknown) - e.o.handleError(fmt.Errorf("failed to export to Stackdriver: %v", err)) + e.o.handleError(fmt.Errorf("failed to export to Google Cloud Monitoring: %w", err)) } return err }