[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 54.54545% with 70 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.96%. Comparing base (d2a68b4) to head (50d00cf).

Files with missing lines	Patch %	Lines
exporters/otlp/src/otlp_grpc_client.cc	0.00%	18 Missing ⚠️
exporters/otlp/src/otlp_http_client.cc	69.82%	16 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 ⚠️
...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.00%   81.96%   -0.03%     
==========================================
  Files         385      385              
  Lines       16031    16147     +116     
==========================================
+ Hits        13144    13233      +89     
- Misses       2887     2914      +27

Files with missing lines	Coverage Δ
...de/opentelemetry/exporters/otlp/otlp_http_client.h	`100.00% <ø> (ø)`
exporters/otlp/src/otlp_http_exporter.cc	`98.24% <93.34%> (-0.81%)`	⬇️
...xporters/otlp/src/otlp_http_log_record_exporter.cc	`96.59% <84.62%> (-1.54%)`	⬇️
exporters/otlp/src/otlp_http_metric_exporter.cc	`96.67% <83.34%> (-1.51%)`	⬇️
...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_http_client.cc	`66.86% <69.82%> (-0.70%)`	⬇️
exporters/otlp/src/otlp_grpc_client.cc	`70.63% <0.00%> (-0.90%)`	⬇️

🚀 New features to boost your workflow:

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

owent · 2026-05-26T13:09:27Z

            arena.get());
-    grpc::Status status = OtlpGrpcClient::DelegateExport(
-        trace_service_stub_.get(), std::move(context), std::move(arena), request, response);
+    grpc::Status status = trace_service_stub_->Export(context.get(), *request, response);


We should only call gRPC's APIs only in otlp_grpc_client.cc to avoid symbol conflicts.

More details can be found at #3435 #1891 #1520 #1603 #1606

Thanks for taking a look!

Got it. Updated DelegateExport to take an optional callback that runs before the arena destructs, and passed a callback that logs partial_success / success.

owent · 2026-05-26T13:10:51Z

            arena.get());
-    grpc::Status status = OtlpGrpcClient::DelegateExport(
-        log_service_stub_.get(), std::move(context), std::move(arena), request, response);
+    grpc::Status status = log_service_stub_->Export(context.get(), *request, response);


The same as otlp_grpc_exporter.cc‎:186

owent · 2026-05-26T13:28:46Z

+
+sdk::common::ExportResult OtlpHttpClient::Export(
+    const google::protobuf::Message &message,
+    google::protobuf::Message *response,


I noticed the callback currently captures shared_ptr of response but the method signature also passes pointer to response separately. It might be cleaner to package these together—perhaps as a CompletionContext—so the lifecycle is managed in one place.

And we should use arena to create response to reduce memory segment.

Good idea, updated to move this into an arena in HttpSessionData.

owent · 2026-05-26T13:30:47Z

-  http_client_->Export(*service_request, [metric_count](
-                                             opentelemetry::sdk::common::ExportResult result) {
+
+  std::shared_ptr<proto::collector::metrics::v1::ExportMetricsServiceResponse> response =


We should use arena to create response to reduce memory segment. Maybe we can move arena into session to hold the lifetime, just like it in otlp_grpc_client

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

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

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 from a0a3f29 to 8b8b3ea Compare June 3, 2026 11:13

RaviTriv added 15 commits June 3, 2026 17:19

Handle OTLP partial success response

077df26

fix iwyu include functional

369afde

update changelog

8a1167f

update test coverage

a75dcb6

fix segfault and iwyu ci failures

f1ca8e8

fix clang tidy shadow warning in metric stub mock

b02f6fe

use DelegateExport callback to keep gRPC calls in client

83017d1

move HTTP response lifecycle into HttpSessionData

a48c3ab

fix iwyu

9a62990

fix clangtidy

d29ff22

move test log handler to test common

6dc3c71

add protobuf forward declaration

d59806c

format cmake

c0f5cf3

remove unused mutex include and add memory include

99597e5

run callback before session is released

ac4dd2c

RaviTriv added 6 commits June 3, 2026 17:20

parse response based on content type

6978df9

fix iwyu no include removed protobuf stub headers

ad6f7d1

remove unused mutex header

8601a09

fail on response parse error

4094b86

fix use after free with protobuf shutdown raii guard

3f3fd2d

fix clangtidy

0cd5bb2

RaviTriv force-pushed the ravtrive/handle-oltp-partial-success-response branch from 25b916f to 0cd5bb2 Compare June 3, 2026 21:20

RaviTriv added 3 commits June 3, 2026 18:08

remove unused headers in http metric export test

eaccf46

add global log handler header to http metric exporter test

40a5052

add memory header for IWYU shared_ptr

50d00cf

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!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RaviTriv commented May 25, 2026 •

edited

Loading

codecov Bot commented May 25, 2026 •

edited

Loading