Skip to content

Commit

Permalink
accept any status code between 200 and 299
Browse files Browse the repository at this point in the history
  • Loading branch information
dmathieu committed Jul 26, 2023
1 parent eba8f19 commit 44680ef
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 48 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -17,7 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- OTLP Metrics Exporter now supports the `OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` environment variable. (#4287)
- Add `WithoutCounterSuffixes` option in `go.opentelemetry.io/otel/exporters/prometheus` to disable addition of `_total` suffixes. (#4306)
- Add info and debug logging to the metric SDK. (#4315)
- Handle 204 HTTP status responses in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#4365)
- Accept 201 to 299 HTTP status as success in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#4365)

### Changed

Expand Down
37 changes: 20 additions & 17 deletions exporters/otlp/otlpmetric/internal/otest/client.go
Expand Up @@ -17,7 +17,6 @@ package otest // import "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/inte
import (
"context"
"fmt"
"net/http"
"testing"
"time"

Expand Down Expand Up @@ -274,27 +273,31 @@ func RunClientTests(f ClientFactory) func(*testing.T) {
assert.ErrorContains(t, errs[0], want)
})

t.Run("NoContent", func(t *testing.T) {
rCh := make(chan ExportResult, 1)
rCh <- ExportResult{
ResponseStatus: http.StatusNoContent,
}
t.Run("Other HTTP success", func(t *testing.T) {
for i := 201; i <= 299; i++ {
t.Run(fmt.Sprintf("status_%d", i), func(t *testing.T) {
rCh := make(chan ExportResult, 1)
rCh <- ExportResult{
ResponseStatus: i,
}

ctx := context.Background()
client, _ := f(rCh)
ctx := context.Background()
client, _ := f(rCh)

defer func(orig otel.ErrorHandler) {
otel.SetErrorHandler(orig)
}(otel.GetErrorHandler())
defer func(orig otel.ErrorHandler) {
otel.SetErrorHandler(orig)
}(otel.GetErrorHandler())

errs := []error{}
eh := otel.ErrorHandlerFunc(func(e error) { errs = append(errs, e) })
otel.SetErrorHandler(eh)
errs := []error{}
eh := otel.ErrorHandlerFunc(func(e error) { errs = append(errs, e) })
otel.SetErrorHandler(eh)

require.NoError(t, client.UploadMetrics(ctx, nil))
require.NoError(t, client.Shutdown(ctx))
require.NoError(t, client.UploadMetrics(ctx, nil))
require.NoError(t, client.Shutdown(ctx))

require.Equal(t, 0, len(errs))
require.Equal(t, 0, len(errs))
})
}
})
}
}
Expand Down
7 changes: 3 additions & 4 deletions exporters/otlp/otlpmetric/otlpmetrichttp/client.go
Expand Up @@ -176,8 +176,8 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
}

var rErr error
switch resp.StatusCode {
case http.StatusOK, http.StatusNoContent:
switch sc := resp.StatusCode; {
case sc >= 200 && sc <= 299:
// Success, do not retry.

// Read the partial success message, if any.
Expand All @@ -202,8 +202,7 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
}
}
return nil
case http.StatusTooManyRequests,
http.StatusServiceUnavailable:
case sc == http.StatusTooManyRequests, sc == http.StatusServiceUnavailable:
// Retry-able failure.
rErr = newResponseError(resp.Header)

Expand Down
6 changes: 3 additions & 3 deletions exporters/otlp/otlptrace/otlptracehttp/client.go
Expand Up @@ -165,8 +165,8 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
}()
}

switch resp.StatusCode {
case http.StatusOK, http.StatusNoContent:
switch sc := resp.StatusCode; {
case sc >= 200 && sc <= 299:
// Success, do not retry.
// Read the partial success message, if any.
var respData bytes.Buffer
Expand All @@ -191,7 +191,7 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
}
return nil

case http.StatusTooManyRequests, http.StatusServiceUnavailable:
case sc == http.StatusTooManyRequests, sc == http.StatusServiceUnavailable:
// Retry-able failures. Drain the body to reuse the connection.
if _, err := io.Copy(io.Discard, resp.Body); err != nil {
otel.Handle(err)
Expand Down
50 changes: 27 additions & 23 deletions exporters/otlp/otlptrace/otlptracehttp/client_test.go
Expand Up @@ -402,29 +402,33 @@ func TestPartialSuccess(t *testing.T) {
require.Contains(t, errs[0].Error(), "2 spans rejected")
}

func TestNoContent(t *testing.T) {
mcCfg := mockCollectorConfig{
InjectHTTPStatus: []int{204},
}
mc := runMockCollector(t, mcCfg)
defer mc.MustStop(t)
driver := otlptracehttp.NewClient(
otlptracehttp.WithEndpoint(mc.Endpoint()),
otlptracehttp.WithInsecure(),
)
ctx := context.Background()
exporter, err := otlptrace.New(ctx, driver)
require.NoError(t, err)
defer func() {
assert.NoError(t, exporter.Shutdown(context.Background()))
}()
func TestOtherHTTPSuccess(t *testing.T) {
for i := 201; i <= 299; i++ {
t.Run(fmt.Sprintf("status_%d", i), func(t *testing.T) {
mcCfg := mockCollectorConfig{
InjectHTTPStatus: []int{i},
}
mc := runMockCollector(t, mcCfg)
defer mc.MustStop(t)
driver := otlptracehttp.NewClient(
otlptracehttp.WithEndpoint(mc.Endpoint()),
otlptracehttp.WithInsecure(),
)
ctx := context.Background()
exporter, err := otlptrace.New(ctx, driver)
require.NoError(t, err)
defer func() {
assert.NoError(t, exporter.Shutdown(context.Background()))
}()

errs := []error{}
otel.SetErrorHandler(otel.ErrorHandlerFunc(func(err error) {
errs = append(errs, err)
}))
err = exporter.ExportSpans(ctx, otlptracetest.SingleReadOnlySpan())
assert.NoError(t, err)
errs := []error{}
otel.SetErrorHandler(otel.ErrorHandlerFunc(func(err error) {
errs = append(errs, err)
}))
err = exporter.ExportSpans(ctx, otlptracetest.SingleReadOnlySpan())
assert.NoError(t, err)

require.Equal(t, 0, len(errs))
require.Equal(t, 0, len(errs))
})
}
}

0 comments on commit 44680ef

Please sign in to comment.