Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OTLP traces export errors use a consistent error message prefix #3516

Merged
merged 12 commits into from
Jan 27, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
61 changes: 61 additions & 0 deletions exporters/otlp/internal/wrappederror.go
Original file line number Diff line number Diff line change
@@ -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{
jmacd marked this conversation as resolved.
Show resolved Hide resolved
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
}
32 changes: 32 additions & 0 deletions exporters/otlp/internal/wrappederror_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
7 changes: 6 additions & 1 deletion exporters/otlp/otlptrace/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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.
Expand Down
63 changes: 63 additions & 0 deletions exporters/otlp/otlptrace/exporter_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
15 changes: 9 additions & 6 deletions exporters/otlp/otlptrace/otlptracegrpc/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package otlptracegrpc_test

import (
"context"
"errors"
"fmt"
"net"
"strings"
Expand Down Expand Up @@ -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())
dashpole marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlptrace/otlptracehttp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down
20 changes: 13 additions & 7 deletions exporters/otlp/otlptrace/otlptracehttp/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ package otlptracehttp_test

import (
"context"
"errors"
"fmt"
"net/http"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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)
jmacd marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand All @@ -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())
}

Expand Down Expand Up @@ -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")
}