[EXPORTER] Handle OTLP partial success response by RaviTriv · Pull Request #4104 · open-telemetry/opentelemetry-cpp

RaviTriv · 2026-05-25T12:12:53Z

Changes

PR to handle partial success responses from OTLP collector. As per the spec it states explicitly to not retry, the proposed solution is to log the rejection count and message.

For significant contributions please make sure you have completed the following items:

CHANGELOG.md updated for non-trivial changes
Unit tests have been added
Changes in public API reviewed

codecov · 2026-05-25T13:00:53Z

Codecov Report

❌ Patch coverage is 61.03896% with 60 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.98%. Comparing base (353fa6b) to head (d1c983c).

Files with missing lines	Patch %	Lines
exporters/otlp/src/otlp_grpc_client.cc	0.00%	18 Missing ⚠️
exporters/otlp/src/otlp_grpc_exporter.cc	26.67%	11 Missing ⚠️
...xporters/otlp/src/otlp_grpc_log_record_exporter.cc	28.58%	10 Missing ⚠️
exporters/otlp/src/otlp_grpc_metric_exporter.cc	28.58%	10 Missing ⚠️
exporters/otlp/src/otlp_http_client.cc	88.68%	6 Missing ⚠️
...xporters/otlp/src/otlp_http_log_record_exporter.cc	84.62%	2 Missing ⚠️
exporters/otlp/src/otlp_http_metric_exporter.cc	83.34%	2 Missing ⚠️
exporters/otlp/src/otlp_http_exporter.cc	93.34%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4104      +/-   ##
==========================================
+ Coverage   82.91%   82.98%   +0.08%     
==========================================
  Files         406      406              
  Lines       17153    17256     +103     
==========================================
+ Hits        14221    14319      +98     
- Misses       2932     2937       +5

Files with missing lines	Coverage Δ
...de/opentelemetry/exporters/otlp/otlp_http_client.h	`100.00% <ø> (ø)`
exporters/otlp/src/otlp_http_exporter.cc	`99.14% <93.34%> (-0.41%)`	⬇️
...xporters/otlp/src/otlp_http_log_record_exporter.cc	`98.30% <84.62%> (-0.81%)`	⬇️
exporters/otlp/src/otlp_http_metric_exporter.cc	`98.35% <83.34%> (-0.79%)`	⬇️
exporters/otlp/src/otlp_http_client.cc	`70.70% <88.68%> (+3.20%)`	⬆️
...xporters/otlp/src/otlp_grpc_log_record_exporter.cc	`80.19% <28.58%> (-6.90%)`	⬇️
exporters/otlp/src/otlp_grpc_metric_exporter.cc	`71.93% <28.58%> (+22.43%)`	⬆️
exporters/otlp/src/otlp_grpc_exporter.cc	`78.10% <26.67%> (-6.68%)`	⬇️
exporters/otlp/src/otlp_grpc_client.cc	`70.63% <0.00%> (-0.90%)`	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lalitb · 2026-06-01T21:01:49Z

      if (result_callback_)
      {
-        result_callback_(result);
+        result_callback_(result, response_);


I think this ordering is risky. response_ is owned by the arena stored in the session data, but ReleaseSession() moves that session data to the cleanup list before the callback reads response_. In async HTTP export, another thread could clean up the session and destroy the arena before this callback uses the response. Can we run the callback while the session still owns the arena, or otherwise keep the arena alive through the callback?

Good catch, reordered so that the callback runs before the ReleaseSession() to ensure that the response_ is valid when the callback runs.

lalitb · 2026-06-01T21:03:05Z

+    // On 2xx, parse the body into the caller-provided typed response
+    if (response_ != nullptr && result == sdk::common::ExportResult::kSuccess)
+    {
+      if (!response_->ParseFromString(body_))


This only handles binary protobuf responses. For OTLP/HTTP JSON mode, the server should send the response as JSON too, so ParseFromString() will fail and the partial_success response will be missed. Can we parse based on options_.content_type, and add a JSON response test as well?

Thanks for pointing this out! Updated to parse based on the the content_type and added corresponding tests

Copilot

Pull request overview

Implements OTLP “partial success” handling across HTTP and gRPC exporters by parsing Export*ServiceResponse and emitting a log that includes the rejected item count and server-provided error message (per spec guidance to not retry on partial success).

Changes:

Extend OTLP HTTP client/exporters to deserialize successful 2xx responses into typed protobuf responses (binary + JSON) and log partial success details.
Add partial-success logging to OTLP gRPC exporters (sync + async paths) via an on_complete hook in DelegateExport.
Add test infrastructure (ScopedTestLogHandler) and expand exporter tests to assert partial-success logging for HTTP/gRPC + JSON/proto.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`test_common/include/opentelemetry/test_common/sdk/common/scoped_test_log_handler.h`	Adds reusable scoped log handler for tests to capture internal logs.
`sdk/test/metrics/meter_provider_sdk_test.cc`	Switches test to use shared `ScopedTestLogHandler` helper.
`sdk/test/metrics/CMakeLists.txt`	Links metrics tests with `opentelemetry_test_common` for the new test helper header.
`sdk/test/metrics/BUILD`	Adds Bazel dependency on `//test_common:headers` for the new test helper header.
`exporters/otlp/test/otlp_http_metric_exporter_test.cc`	Adds HTTP metric exporter partial-success tests (proto + JSON) and log assertions.
`exporters/otlp/test/otlp_http_log_record_exporter_test.cc`	Adds HTTP log exporter partial-success tests (proto + JSON) and log assertions.
`exporters/otlp/test/otlp_http_exporter_test.cc`	Adds HTTP trace exporter partial-success tests (proto + JSON) and log assertions.
`exporters/otlp/test/otlp_grpc_metric_exporter_test.cc`	Adds gRPC metric exporter partial-success test and log assertions.
`exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc`	Adds gRPC log exporter partial-success test and log assertions.
`exporters/otlp/test/otlp_grpc_exporter_test.cc`	Adds gRPC trace exporter partial-success test and log assertions.
`exporters/otlp/src/otlp_http_metric_exporter.cc`	Allocates response on arena and logs partial success from parsed HTTP response.
`exporters/otlp/src/otlp_http_log_record_exporter.cc`	Allocates response on arena and logs partial success from parsed HTTP response.
`exporters/otlp/src/otlp_http_exporter.cc`	Allocates response on arena and logs partial success from parsed HTTP response.
`exporters/otlp/src/otlp_http_client.cc`	Parses successful HTTP response bodies into a caller-provided protobuf message (JSON/proto) and forwards it to callbacks while keeping arena alive.
`exporters/otlp/src/otlp_grpc_metric_exporter.cc`	Logs partial success for gRPC metrics responses in async and sync code paths.
`exporters/otlp/src/otlp_grpc_log_record_exporter.cc`	Logs partial success for gRPC logs responses in async and sync code paths.
`exporters/otlp/src/otlp_grpc_exporter.cc`	Logs partial success for gRPC trace responses; routes success logging through completion hook.
`exporters/otlp/src/otlp_grpc_client.cc`	Adds an optional `on_complete` callback to `DelegateExport` to inspect response on success while keeping arena alive.
`exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_client.h`	Adds typed-response async export overload and extends session data to retain arena/response lifetime.
`exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_client.h`	Adds optional `on_complete` callback to sync `DelegateExport` overloads.
`exporters/otlp/CMakeLists.txt`	Links OTLP exporter tests with `opentelemetry_test_common` for the shared log handler helper.
`exporters/otlp/BUILD`	Adds protobuf dependency and `//test_common:headers` deps needed by updated tests.
`CHANGELOG.md`	Adds changelog entry for OTLP partial success handling.

lalitb · 2026-06-02T20:07:16Z

+    {
+      if (content_type_ == HttpRequestContentType::kJson)
+      {
+        if (!google::protobuf::util::JsonStringToMessage(body_, response_).ok())


Can we avoid treating this as a success if parsing the response body fails? Right now we only log the parse error, so the exporter may still continue with an empty response and miss partial_success.

Yep, updated to set the result as kFailure on parse fail

lalitb · 2026-06-08T01:40:12Z

    return opentelemetry::sdk::common::ExportResult::kFailure;
  }

  return opentelemetry::sdk::common::ExportResult::kSuccess;


Can we return the final callback result here instead of always returning success? OnResponse() now changes the result to kFailure when parsing the response body fails, but this path still returns kSuccess as long as the wait completed. That means sync HTTP export can log a parse failure but still report success to the caller

good catch! definitely, updated the sync path to return kFailure if parse fails.

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.

owent

LGTM. thanks.

marcalff · 2026-06-16T18:48:43Z

LGTM, but the real expert in this area is @owent , trusting his review.

@lalitb Since you looked at this PR already, do you have any remaining comments ?

Ok to merge ?

RaviTriv commented May 25, 2026

View reviewed changes

Comment thread exporters/otlp/test/otlp_grpc_exporter_test.cc Outdated

owent reviewed May 26, 2026

View reviewed changes

Comment thread exporters/otlp/src/otlp_grpc_exporter.cc Outdated

owent reviewed May 26, 2026

View reviewed changes

Comment thread exporters/otlp/src/otlp_grpc_log_record_exporter.cc Outdated

owent reviewed May 26, 2026

View reviewed changes

Comment thread exporters/otlp/src/otlp_http_client.cc

owent reviewed May 26, 2026

View reviewed changes

Comment thread exporters/otlp/src/otlp_http_metric_exporter.cc Outdated

RaviTriv marked this pull request as ready for review May 27, 2026 21:14

RaviTriv requested a review from a team as a code owner May 27, 2026 21:14

lalitb reviewed Jun 1, 2026

View reviewed changes

lalitb requested a review from Copilot June 2, 2026 20:00

Copilot started reviewing on behalf of lalitb June 2, 2026 20:00 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

lalitb reviewed Jun 2, 2026

View reviewed changes

RaviTriv force-pushed the ravtrive/handle-oltp-partial-success-response branch 4 times, most recently from 62795e6 to b24d27e Compare June 8, 2026 00:06

lalitb reviewed Jun 8, 2026

View reviewed changes

owent reviewed Jun 9, 2026

View reviewed changes

Comment thread exporters/otlp/src/otlp_http_exporter.cc Outdated

owent reviewed Jun 9, 2026

View reviewed changes

Comment thread exporters/otlp/src/otlp_http_log_record_exporter.cc Outdated

owent reviewed Jun 9, 2026

View reviewed changes

Comment thread exporters/otlp/src/otlp_http_client.cc Outdated

RaviTriv force-pushed the ravtrive/handle-oltp-partial-success-response branch from d7540b8 to 76ec719 Compare June 9, 2026 23:08

RaviTriv added 7 commits June 10, 2026 06:19

Handle OTLP partial success response

56724ad

fix iwyu include functional

c81ad6c

update test coverage

ad92f4d

fix segfault and iwyu ci failures

a835c83

fix clang tidy shadow warning in metric stub mock

b2561a4

use DelegateExport callback to keep gRPC calls in client

e0cca85

move HTTP response lifecycle into HttpSessionData

c7367b4

RaviTriv added 13 commits June 10, 2026 06:19

fix iwyu no include removed protobuf stub headers

9058a39

remove unused mutex header

a0130f8

fail on response parse error

1e9526c

fix use after free with protobuf shutdown raii guard

7ff4475

fix clangtidy

36c3240

remove unused headers in http metric export test

64e935a

add global log handler header to http metric exporter test

12cfd00

add memory header for IWYU shared_ptr

25e3dad

return response parse failure for sync export

9312b66

skip the parse fail test in async build

8240c34

replace new with make_unique

47830e2

remove old unused overloads

e7e111f

fix changelog after rebase

cf2e36a

RaviTriv force-pushed the ravtrive/handle-oltp-partial-success-response branch from 76ec719 to cf2e36a Compare June 10, 2026 10:20

RaviTriv added 2 commits June 10, 2026 07:35

format

3e4c833

Fix IWYU for protobuf json headers

668464a

lalitb requested a review from Copilot June 10, 2026 13:18

Copilot started reviewing on behalf of lalitb June 10, 2026 13:19 View session

Copilot AI reviewed Jun 10, 2026

View reviewed changes

Comment thread exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_client.h

Comment thread exporters/otlp/src/otlp_http_client.cc Outdated

Comment thread exporters/otlp/src/otlp_http_client.cc

RaviTriv added 3 commits June 10, 2026 17:23

avoid allocation on async export path

6dbea8a

log success once body parses succesfully

6322fb7

update export doc

910bded

owent approved these changes Jun 14, 2026

View reviewed changes

marcalff added 2 commits June 14, 2026 19:02

Merge branch 'main' into ravtrive/handle-oltp-partial-success-response

7d45184

Merge branch 'main' into ravtrive/handle-oltp-partial-success-response

ae1473e

owent mentioned this pull request Jun 18, 2026

Cleanup old new operator calling in OTLP. #4168

Open

marcalff added 3 commits June 19, 2026 15:49

Merge branch 'main' into ravtrive/handle-oltp-partial-success-response

01d6f91

Merge branch 'main' into ravtrive/handle-oltp-partial-success-response

9b4fd43

Merge branch 'main' into ravtrive/handle-oltp-partial-success-response

d1c983c

Conversation

RaviTriv commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Uh oh!

codecov Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

owent left a comment

Choose a reason for hiding this comment

Uh oh!

marcalff commented Jun 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

RaviTriv commented May 25, 2026 •

edited

Loading

codecov Bot commented May 25, 2026 •

edited

Loading