From 552b101149f8390240eed3b1e5f2eda2a582a7f3 Mon Sep 17 00:00:00 2001 From: Mark Pictor <93549255+mark-pictor-csec@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:05:39 -0600 Subject: [PATCH 1/3] log, metric, trace clients: close body uniformly 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. --- exporters/otlp/otlplog/otlploghttp/client.go | 11 +++++++---- exporters/otlp/otlpmetric/otlpmetrichttp/client.go | 11 +++++++---- exporters/otlp/otlptrace/otlptracehttp/client.go | 1 - 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go index 1539bec7b90..5a6e9a257ed 100644 --- a/exporters/otlp/otlplog/otlploghttp/client.go +++ b/exporters/otlp/otlplog/otlploghttp/client.go @@ -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; { @@ -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 } @@ -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 }) } diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go index 7ef295e59e3..f36388f45af 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go @@ -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; { @@ -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 } @@ -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 }) } diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go index bb2f3ffd1d8..38fabf1b660 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client.go @@ -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 } From 5d2de953aa26bbef56d421f418025dd3c1dfaf70 Mon Sep 17 00:00:00 2001 From: Mark Pictor <93549255+mark-pictor-csec@users.noreply.github.com> Date: Fri, 8 Nov 2024 11:21:24 -0600 Subject: [PATCH 2/3] update changelog with request body close --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4b14b58253..01c4552f586 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix `WithEndpointURL` to always use a secure connection when an https URL is passed in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#5944) - Fix `WithEndpointURL` to always use a secure connection when an https URL is passed in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5944) - Fix incorrect metrics generated from callbacks when multiple readers are used in `go.opentelemetry.io/otel/sdk/metric`. (#5900) +- 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) ### Changed From 29badc37ad4881d6789e03e0cdd4f44396692a2e Mon Sep 17 00:00:00 2001 From: Mark Pictor <93549255+mark-pictor-csec@users.noreply.github.com> Date: Fri, 8 Nov 2024 13:35:09 -0600 Subject: [PATCH 3/3] changelog --- CHANGELOG.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86e2ab7c5fb..dd74646c396 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) + @@ -52,9 +58,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix `WithEndpointURL` to always use a secure connection when an https URL is passed in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#5944) - Fix `WithEndpointURL` to always use a secure connection when an https URL is passed in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5944) - Fix incorrect metrics generated from callbacks when multiple readers are used in `go.opentelemetry.io/otel/sdk/metric`. (#5900) -- 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) ### Removed