Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle 2xx as success for OTLP HTTP trace and metric exporters #4365

Merged
merged 11 commits into from
Aug 7, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +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)
MrAlias marked this conversation as resolved.
Show resolved Hide resolved

### Changed

Expand Down
26 changes: 26 additions & 0 deletions exporters/otlp/otlpmetric/internal/otest/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package otest // import "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/inte
import (
"context"
"fmt"
"net/http"
"testing"
"time"

Expand Down Expand Up @@ -272,6 +273,31 @@ 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("NoContent", func(t *testing.T) {
rCh := make(chan ExportResult, 1)
rCh <- ExportResult{
Err: &HTTPResponseError{
Status: http.StatusNoContent,
},
}

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

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)

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

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

Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlpmetric/otlpmetrichttp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou

var rErr error
switch resp.StatusCode {
case http.StatusOK:
case http.StatusOK, http.StatusNoContent:
// Success, do not retry.

// Read the partial success message, if any.
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 @@ -166,7 +166,7 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
}

switch resp.StatusCode {
case http.StatusOK:
case http.StatusOK, http.StatusNoContent:
// Success, do not retry.
// Read the partial success message, if any.
var respData bytes.Buffer
Expand Down
27 changes: 27 additions & 0 deletions exporters/otlp/otlptrace/otlptracehttp/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,3 +401,30 @@ func TestPartialSuccess(t *testing.T) {
require.Contains(t, errs[0].Error(), "partially successful")
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()))
}()

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))
}
Loading