[EXPORTER] Handle OTLP partial success response#4104
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4104 +/- ##
==========================================
- Coverage 82.01% 81.97% -0.03%
==========================================
Files 385 385
Lines 16030 16135 +105
==========================================
+ Hits 13145 13225 +80
- Misses 2885 2910 +25
🚀 New features to boost your workflow:
|
| 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); |
There was a problem hiding this comment.
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.
| 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); |
There was a problem hiding this comment.
The same as otlp_grpc_exporter.cc:186
|
|
||
| sdk::common::ExportResult OtlpHttpClient::Export( | ||
| const google::protobuf::Message &message, | ||
| google::protobuf::Message *response, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Good idea, updated to move this into an arena in HttpSessionData.
| http_client_->Export(*service_request, [metric_count]( | ||
| opentelemetry::sdk::common::ExportResult result) { | ||
|
|
||
| std::shared_ptr<proto::collector::metrics::v1::ExportMetricsServiceResponse> response = |
There was a problem hiding this comment.
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
2 participants
Fixes #1577
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.mdupdated for non-trivial changes