diff --git a/CHANGELOG.md b/CHANGELOG.md index b7537aa5242..9215ad9e217 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `traceIDRatioSampler` (given by `TraceIDRatioBased(float64)`) now uses the rightmost bits for sampling decisions, fixing random sampling when using ID generators like `xray.IDGenerator` and increasing parity with other language implementations. (#3557) +- OTLP trace exporter errors are wrapped in a type that prefixes the signal name in front of error strings (e.g., "traces export: context canceled" instead of just "context canceled"). + Existing users of the exporters attempting to identify specific errors will need to use `errors.Unwrap()` to get the underlying error. (#3516) - The OTLP exporter for traces and metrics will print the final retryable error message when attempts to retry time out. (#3514) - The instrument kind names in `go.opentelemetry.io/otel/sdk/metric` are updated to match the API. (#3562) - `InstrumentKindSyncCounter` is renamed to `InstrumentKindCounter` diff --git a/exporters/otlp/internal/wrappederror.go b/exporters/otlp/internal/wrappederror.go new file mode 100644 index 00000000000..217751da552 --- /dev/null +++ b/exporters/otlp/internal/wrappederror.go @@ -0,0 +1,61 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package internal // import "go.opentelemetry.io/otel/exporters/otlp/internal" + +// ErrorKind is used to identify the kind of export error +// being wrapped. +type ErrorKind int + +const ( + // TracesExport indicates the error comes from the OTLP trace exporter. + TracesExport ErrorKind = iota +) + +// prefix returns a prefix for the Error() string. +func (k ErrorKind) prefix() string { + switch k { + case TracesExport: + return "traces export: " + default: + return "unknown: " + } +} + +// wrappedExportError wraps an OTLP exporter error with the kind of +// signal that produced it. +type wrappedExportError struct { + wrap error + kind ErrorKind +} + +// WrapTracesError wraps an error from the OTLP exporter for traces. +func WrapTracesError(err error) error { + return wrappedExportError{ + wrap: err, + kind: TracesExport, + } +} + +var _ error = wrappedExportError{} + +// Error attaches a prefix corresponding to the kind of exporter. +func (t wrappedExportError) Error() string { + return t.kind.prefix() + t.wrap.Error() +} + +// Unwrap returns the wrapped error. +func (t wrappedExportError) Unwrap() error { + return t.wrap +} diff --git a/exporters/otlp/internal/wrappederror_test.go b/exporters/otlp/internal/wrappederror_test.go new file mode 100644 index 00000000000..995358a9f8e --- /dev/null +++ b/exporters/otlp/internal/wrappederror_test.go @@ -0,0 +1,32 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package internal // import "go.opentelemetry.io/otel/exporters/otlp/internal" + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestWrappedError(t *testing.T) { + e := WrapTracesError(context.Canceled) + + require.Equal(t, context.Canceled, errors.Unwrap(e)) + require.Equal(t, TracesExport, e.(wrappedExportError).kind) + require.Equal(t, "traces export: context canceled", e.Error()) + require.True(t, errors.Is(e, context.Canceled)) +} diff --git a/exporters/otlp/otlptrace/exporter.go b/exporters/otlp/otlptrace/exporter.go index c5ee6c098cc..b65802edbbd 100644 --- a/exporters/otlp/otlptrace/exporter.go +++ b/exporters/otlp/otlptrace/exporter.go @@ -19,6 +19,7 @@ import ( "errors" "sync" + "go.opentelemetry.io/otel/exporters/otlp/internal" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform" tracesdk "go.opentelemetry.io/otel/sdk/trace" ) @@ -45,7 +46,11 @@ func (e *Exporter) ExportSpans(ctx context.Context, ss []tracesdk.ReadOnlySpan) return nil } - return e.client.UploadTraces(ctx, protoSpans) + err := e.client.UploadTraces(ctx, protoSpans) + if err != nil { + return internal.WrapTracesError(err) + } + return nil } // Start establishes a connection to the receiving endpoint. diff --git a/exporters/otlp/otlptrace/exporter_test.go b/exporters/otlp/otlptrace/exporter_test.go new file mode 100644 index 00000000000..1007d23a7e2 --- /dev/null +++ b/exporters/otlp/otlptrace/exporter_test.go @@ -0,0 +1,63 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package otlptrace_test + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/exporters/otlp/otlptrace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + tracepb "go.opentelemetry.io/proto/otlp/trace/v1" +) + +type client struct { + uploadErr error +} + +var _ otlptrace.Client = &client{} + +func (c *client) Start(ctx context.Context) error { + return nil +} + +func (c *client) Stop(ctx context.Context) error { + return nil +} + +func (c *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.ResourceSpans) error { + return c.uploadErr +} + +func TestExporterClientError(t *testing.T) { + ctx := context.Background() + exp, err := otlptrace.New(ctx, &client{ + uploadErr: context.Canceled, + }) + assert.NoError(t, err) + + spans := tracetest.SpanStubs{{Name: "Span 0"}}.Snapshots() + err = exp.ExportSpans(ctx, spans) + + assert.Error(t, err) + assert.True(t, errors.Is(err, context.Canceled)) + assert.True(t, strings.HasPrefix(err.Error(), "traces export: "), err) + + assert.NoError(t, exp.Shutdown(ctx)) +} diff --git a/exporters/otlp/otlptrace/otlptracegrpc/client_test.go b/exporters/otlp/otlptrace/otlptracegrpc/client_test.go index 395ccc28bf4..ded71a5a3b4 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/client_test.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/client_test.go @@ -16,6 +16,7 @@ package otlptracegrpc_test import ( "context" + "errors" "fmt" "net" "strings" @@ -239,7 +240,9 @@ func TestExportSpansTimeoutHonored(t *testing.T) { // Release the export so everything is cleaned up on shutdown. close(exportBlock) - require.Equal(t, codes.DeadlineExceeded, status.Convert(err).Code()) + unwrapped := errors.Unwrap(err) + require.Equal(t, codes.DeadlineExceeded, status.Convert(unwrapped).Code()) + require.True(t, strings.HasPrefix(err.Error(), "traces export: "), err) } func TestNewWithMultipleAttributeTypes(t *testing.T) { @@ -399,18 +402,18 @@ func TestPartialSuccess(t *testing.T) { }) t.Cleanup(func() { require.NoError(t, mc.stop()) }) - errors := []error{} + errs := []error{} otel.SetErrorHandler(otel.ErrorHandlerFunc(func(err error) { - errors = append(errors, err) + errs = append(errs, err) })) ctx := context.Background() exp := newGRPCExporter(t, ctx, mc.endpoint) t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) }) require.NoError(t, exp.ExportSpans(ctx, roSpans)) - require.Equal(t, 1, len(errors)) - require.Contains(t, errors[0].Error(), "partially successful") - require.Contains(t, errors[0].Error(), "2 spans rejected") + require.Equal(t, 1, len(errs)) + require.Contains(t, errs[0].Error(), "partially successful") + require.Contains(t, errs[0].Error(), "2 spans rejected") } func TestCustomUserAgent(t *testing.T) { diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go index 79b8af73286..9fbe861717d 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client.go @@ -197,7 +197,7 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc } return newResponseError(resp.Header) default: - return fmt.Errorf("failed to send %s to %s: %s", d.name, request.URL, resp.Status) + return fmt.Errorf("failed to send to %s: %s", request.URL, resp.Status) } }) } diff --git a/exporters/otlp/otlptrace/otlptracehttp/client_test.go b/exporters/otlp/otlptrace/otlptracehttp/client_test.go index 135b6517622..859c48dd38c 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client_test.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client_test.go @@ -16,9 +16,11 @@ package otlptracehttp_test import ( "context" + "errors" "fmt" "net/http" "os" + "strings" "testing" "time" @@ -219,7 +221,9 @@ func TestTimeout(t *testing.T) { assert.NoError(t, exporter.Shutdown(ctx)) }() err = exporter.ExportSpans(ctx, otlptracetest.SingleReadOnlySpan()) - assert.Equalf(t, true, os.IsTimeout(err), "expected timeout error, got: %v", err) + unwrapped := errors.Unwrap(err) + assert.Equalf(t, true, os.IsTimeout(unwrapped), "expected timeout error, got: %v", unwrapped) + assert.True(t, strings.HasPrefix(err.Error(), "traces export: "), err) } func TestNoRetry(t *testing.T) { @@ -246,7 +250,9 @@ func TestNoRetry(t *testing.T) { }() err = exporter.ExportSpans(ctx, otlptracetest.SingleReadOnlySpan()) assert.Error(t, err) - assert.Equal(t, fmt.Sprintf("failed to send traces to http://%s/v1/traces: 400 Bad Request", mc.endpoint), err.Error()) + unwrapped := errors.Unwrap(err) + assert.Equal(t, fmt.Sprintf("failed to send to http://%s/v1/traces: 400 Bad Request", mc.endpoint), unwrapped.Error()) + assert.True(t, strings.HasPrefix(err.Error(), "traces export: ")) assert.Empty(t, mc.GetSpans()) } @@ -384,14 +390,14 @@ func TestPartialSuccess(t *testing.T) { assert.NoError(t, exporter.Shutdown(context.Background())) }() - errors := []error{} + errs := []error{} otel.SetErrorHandler(otel.ErrorHandlerFunc(func(err error) { - errors = append(errors, err) + errs = append(errs, err) })) err = exporter.ExportSpans(ctx, otlptracetest.SingleReadOnlySpan()) assert.NoError(t, err) - require.Equal(t, 1, len(errors)) - require.Contains(t, errors[0].Error(), "partially successful") - require.Contains(t, errors[0].Error(), "2 spans rejected") + require.Equal(t, 1, len(errs)) + require.Contains(t, errs[0].Error(), "partially successful") + require.Contains(t, errs[0].Error(), "2 spans rejected") }