Skip to content

Commit

Permalink
OTLP traces export errors use a consistent error message prefix (#3516)
Browse files Browse the repository at this point in the history
* OTLP traces export errors use a consistent error message prefix

* use a wrapped error

* update changelog

* merge changelog

* Update CHANGELOG.md

---------

Co-authored-by: Chester Cheung <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>
  • Loading branch information
3 people authored Jan 27, 2023
1 parent af3db6e commit ec13377
Show file tree
Hide file tree
Showing 8 changed files with 187 additions and 15 deletions.
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{
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())
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)
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")
}

0 comments on commit ec13377

Please sign in to comment.