Skip to content

Commit

Permalink
otlptracehttp, otlpmetrichttp: Retry temporary HTTP request failures (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
pellared committed Oct 31, 2023
1 parent aea3eab commit 5ec67e8
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 16 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Retry for `502 Bad Gateway` and `504 Gateway Timeout` HTTP statuses in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#4670)
- Retry for `RESOURCE_EXHAUSTED` only if RetryInfo is returned in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc`. (#4669)
- Retry for `RESOURCE_EXHAUSTED` only if RetryInfo is returned in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#4669)
- Retry temporary HTTP request failures in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#4679)
- Retry temporary HTTP request failures in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#4679)

### Fixed

Expand Down
5 changes: 5 additions & 0 deletions exporters/otlp/otlpmetric/otlpmetrichttp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"bytes"
"compress/gzip"
"context"
"errors"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -147,6 +148,10 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou

request.reset(iCtx)
resp, err := c.httpClient.Do(request.Request)
var urlErr *url.Error
if errors.As(err, &urlErr) && urlErr.Temporary() {
return newResponseError(http.Header{})
}
if err != nil {
return err
}
Expand Down
20 changes: 8 additions & 12 deletions exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ func TestClient(t *testing.T) {
t.Run("Integration", otest.RunClientTests(factory))
}

func TestNewWithInvalidEndpoint(t *testing.T) {
ctx := context.Background()
exp, err := New(ctx, WithEndpoint("host:invalid-port"))
assert.Error(t, err)
assert.Nil(t, exp)
}

func TestConfig(t *testing.T) {
factoryFunc := func(ePt string, rCh <-chan otest.ExportResult, o ...Option) (metric.Exporter, *otest.HTTPCollector) {
coll, err := otest.NewHTTPCollector(ePt, rCh)
Expand Down Expand Up @@ -113,7 +120,7 @@ func TestConfig(t *testing.T) {
t.Cleanup(func() { close(rCh) })
t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
err := exp.Export(ctx, &metricdata.ResourceMetrics{})
assert.ErrorContains(t, err, context.DeadlineExceeded.Error())
assert.ErrorAs(t, err, new(retryableError))
})

t.Run("WithCompressionGZip", func(t *testing.T) {
Expand Down Expand Up @@ -174,17 +181,6 @@ func TestConfig(t *testing.T) {
assert.Len(t, coll.Collect().Dump(), 1)
})

t.Run("WithURLPath", func(t *testing.T) {
path := "/prefix/v2/metrics"
ePt := fmt.Sprintf("http://localhost:0%s", path)
exp, coll := factoryFunc(ePt, nil, WithURLPath(path))
ctx := context.Background()
t.Cleanup(func() { require.NoError(t, coll.Shutdown(ctx)) })
t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
assert.NoError(t, exp.Export(ctx, &metricdata.ResourceMetrics{}))
assert.Len(t, coll.Collect().Dump(), 1)
})

t.Run("WithTLSClientConfig", func(t *testing.T) {
ePt := "https://localhost:0"
tlsCfg := &tls.Config{InsecureSkipVerify: true}
Expand Down
5 changes: 5 additions & 0 deletions exporters/otlp/otlptrace/otlptracehttp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"bytes"
"compress/gzip"
"context"
"errors"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -152,6 +153,10 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc

request.reset(ctx)
resp, err := d.client.Do(request.Request)
var urlErr *url.Error
if errors.As(err, &urlErr) && urlErr.Temporary() {
return newResponseError(http.Header{})
}
if err != nil {
return err
}
Expand Down
6 changes: 2 additions & 4 deletions exporters/otlp/otlptrace/otlptracehttp/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"errors"
"fmt"
"net/http"
"os"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -213,6 +212,7 @@ func TestTimeout(t *testing.T) {
otlptracehttp.WithEndpoint(mc.Endpoint()),
otlptracehttp.WithInsecure(),
otlptracehttp.WithTimeout(time.Nanosecond),
otlptracehttp.WithRetry(otlptracehttp.RetryConfig{Enabled: false}),
)
ctx := context.Background()
exporter, err := otlptrace.New(ctx, client)
Expand All @@ -221,9 +221,7 @@ func TestTimeout(t *testing.T) {
assert.NoError(t, exporter.Shutdown(ctx))
}()
err = exporter.ExportSpans(ctx, otlptracetest.SingleReadOnlySpan())
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)
assert.ErrorContains(t, err, "retry-able request failure")
}

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

0 comments on commit 5ec67e8

Please sign in to comment.