diff --git a/CHANGELOG.md b/CHANGELOG.md index 5194b3635..4bf29bc4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add info and debug logging to the metric SDK. (#4315) - The `go.opentelemetry.io/otel/semconv/v1.21.0` package. The package contains semantic conventions from the `v1.21.0` version of the OpenTelemetry Semantic Conventions. (#4362) +- 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) - Document the `Temporality` and `Aggregation` methods of the `"go.opentelemetry.io/otel/sdk/metric".Exporter"` need to be concurrent safe. (#4381) - Expand the set of units supported by the prometheus exporter, and don't add unit suffixes if they are already present in `go.opentelemetry.op/otel/exporters/prometheus` (#4374) diff --git a/exporters/otlp/otlpmetric/internal/otest/client.go b/exporters/otlp/otlpmetric/internal/otest/client.go index f58aa99ae..a315869c1 100644 --- a/exporters/otlp/otlpmetric/internal/otest/client.go +++ b/exporters/otlp/otlpmetric/internal/otest/client.go @@ -272,6 +272,34 @@ func RunClientTests(f ClientFactory) func(*testing.T) { want := fmt.Sprintf("%s (%d metric data points rejected)", msg, n) assert.ErrorContains(t, errs[0], want) }) + + t.Run("Other HTTP success", func(t *testing.T) { + for code := 201; code <= 299; code++ { + t.Run(fmt.Sprintf("status_%d", code), func(t *testing.T) { + rCh := make(chan ExportResult, 1) + rCh <- ExportResult{ + ResponseStatus: code, + } + + ctx := context.Background() + client, _ := f(rCh) + defer func() { + assert.NoError(t, client.Shutdown(ctx)) + }() + + 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) + + assert.NoError(t, client.UploadMetrics(ctx, nil)) + assert.Equal(t, 0, len(errs)) + }) + } + }) } } diff --git a/exporters/otlp/otlpmetric/internal/otest/collector.go b/exporters/otlp/otlpmetric/internal/otest/collector.go index 95e757dae..34756f4cc 100644 --- a/exporters/otlp/otlpmetric/internal/otest/collector.go +++ b/exporters/otlp/otlpmetric/internal/otest/collector.go @@ -50,8 +50,9 @@ type Collector interface { } type ExportResult struct { - Response *collpb.ExportMetricsServiceResponse - Err error + Response *collpb.ExportMetricsServiceResponse + ResponseStatus int + Err error } // Storage stores uploaded OTLP metric data in their proto form. @@ -376,7 +377,11 @@ func (c *HTTPCollector) respond(w http.ResponseWriter, resp ExportResult) { } w.Header().Set("Content-Type", "application/x-protobuf") - w.WriteHeader(http.StatusOK) + if resp.ResponseStatus != 0 { + w.WriteHeader(resp.ResponseStatus) + } else { + w.WriteHeader(http.StatusOK) + } if resp.Response == nil { _, _ = w.Write(emptyExportMetricsServiceResponse) } else { diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go index ac7023c7a..33f5474c1 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go @@ -153,8 +153,8 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou } var rErr error - switch resp.StatusCode { - case http.StatusOK: + switch sc := resp.StatusCode; { + case sc >= 200 && sc <= 299: // Success, do not retry. // Read the partial success message, if any. @@ -179,8 +179,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) diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go index fe8afc240..3a3cfec0c 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client.go @@ -164,8 +164,8 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc }() } - switch resp.StatusCode { - case http.StatusOK: + switch sc := resp.StatusCode; { + case sc >= 200 && sc <= 299: // Success, do not retry. // Read the partial success message, if any. var respData bytes.Buffer @@ -190,7 +190,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) diff --git a/exporters/otlp/otlptrace/otlptracehttp/client_test.go b/exporters/otlp/otlptrace/otlptracehttp/client_test.go index 279d8ada3..2020b1505 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client_test.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client_test.go @@ -401,3 +401,34 @@ func TestPartialSuccess(t *testing.T) { require.Contains(t, errs[0].Error(), "partially successful") require.Contains(t, errs[0].Error(), "2 spans rejected") } + +func TestOtherHTTPSuccess(t *testing.T) { + for code := 201; code <= 299; code++ { + t.Run(fmt.Sprintf("status_%d", code), func(t *testing.T) { + mcCfg := mockCollectorConfig{ + InjectHTTPStatus: []int{code}, + } + 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) + + assert.Equal(t, 0, len(errs)) + }) + } +}