From 5ec67e83df77751b6c5e6d386b4047d5d813e546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 31 Oct 2023 08:06:25 +0100 Subject: [PATCH] otlptracehttp, otlpmetrichttp: Retry temporary HTTP request failures (#4679) --- CHANGELOG.md | 2 ++ .../otlp/otlpmetric/otlpmetrichttp/client.go | 5 +++++ .../otlpmetric/otlpmetrichttp/client_test.go | 20 ++++++++----------- .../otlp/otlptrace/otlptracehttp/client.go | 5 +++++ .../otlptrace/otlptracehttp/client_test.go | 6 ++---- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 596adea6944..a77f0e24665 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go index af2e40d2782..7cc6f6ae7bb 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go @@ -18,6 +18,7 @@ import ( "bytes" "compress/gzip" "context" + "errors" "fmt" "io" "net" @@ -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 } diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go index b14e10c3419..2f48f472748 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go @@ -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) @@ -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) { @@ -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} diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go index 0990b88f147..068aef300ee 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client.go @@ -18,6 +18,7 @@ import ( "bytes" "compress/gzip" "context" + "errors" "fmt" "io" "net" @@ -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 } diff --git a/exporters/otlp/otlptrace/otlptracehttp/client_test.go b/exporters/otlp/otlptrace/otlptracehttp/client_test.go index 1fb58fcb69a..21838695c5e 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client_test.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client_test.go @@ -19,7 +19,6 @@ import ( "errors" "fmt" "net/http" - "os" "strings" "testing" "time" @@ -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) @@ -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) {