Skip to content

Commit

Permalink
otlp: Clients to close body uniformly (#5954)
Browse files Browse the repository at this point in the history
There were inconsistencies in closing the response body. For traces, the
Close happened in a defer statement and any error was logged. Logs and
metrics were less rigorous. It appeared Close() wasn't always called,
and when it was, errors were returned sometimes and ignored at other
times.

This applies the defer logic from traces to the other two and removes
other Close() calls.

This was part of PR #5929, and has been split out as
requested:
#5929 (comment)).
  • Loading branch information
mark-pictor-csec authored Nov 12, 2024
1 parent ac5caea commit 0bf9572
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 9 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Fixed

- Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5954)
- Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5954)
- Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5954)

<!-- Released section -->
<!-- Don't change this section unless doing release -->

Expand Down
11 changes: 7 additions & 4 deletions exporters/otlp/otlplog/otlploghttp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,13 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs)
if err != nil {
return err
}
if resp != nil && resp.Body != nil {
defer func() {
if err := resp.Body.Close(); err != nil {
otel.Handle(err)
}
}()
}

var rErr error
switch sc := resp.StatusCode; {
Expand Down Expand Up @@ -193,7 +200,6 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs)
// debugging the actual issue.
var respData bytes.Buffer
if _, err := io.Copy(&respData, resp.Body); err != nil {
_ = resp.Body.Close()
return err
}

Expand All @@ -208,9 +214,6 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs)
rErr = fmt.Errorf("failed to send logs to %s: %s", request.URL, resp.Status)
}

if err := resp.Body.Close(); err != nil {
return err
}
return rErr
})
}
Expand Down
11 changes: 7 additions & 4 deletions exporters/otlp/otlpmetric/otlpmetrichttp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,13 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
if err != nil {
return err
}
if resp != nil && resp.Body != nil {
defer func() {
if err := resp.Body.Close(); err != nil {
otel.Handle(err)
}
}()
}

var rErr error
switch sc := resp.StatusCode; {
Expand Down Expand Up @@ -196,7 +203,6 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
// debugging the actual issue.
var respData bytes.Buffer
if _, err := io.Copy(&respData, resp.Body); err != nil {
_ = resp.Body.Close()
return err
}

Expand All @@ -211,9 +217,6 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
rErr = fmt.Errorf("failed to send metrics to %s: %s", request.URL, resp.Status)
}

if err := resp.Body.Close(); err != nil {
return err
}
return rErr
})
}
Expand Down
1 change: 0 additions & 1 deletion exporters/otlp/otlptrace/otlptracehttp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
// debugging the actual issue.
var respData bytes.Buffer
if _, err := io.Copy(&respData, resp.Body); err != nil {
_ = resp.Body.Close()
return err
}

Expand Down

0 comments on commit 0bf9572

Please sign in to comment.