-
Notifications
You must be signed in to change notification settings - Fork 759
Description
Is your feature request related to a problem?
Yes. I'm debugging an issue with OTLP export where writes appear to succeed, but the data is not appearing.
The wire protocol includes partial_success fields like in ExportTraceServiceResponse:
message ExportTraceServiceResponse {
// The details of a partially successful export request.
//
// If the request is only partially accepted
// (i.e. when the server accepts only parts of the data and rejects the rest)
// the server MUST initialize the `partial_success` field and MUST
// set the `rejected_<signal>` with the number of items it rejected.
//
// Servers MAY also make use of the `partial_success` field to convey
// warnings/suggestions to senders even when the request was fully accepted.
// In such cases, the `rejected_<signal>` MUST have a value of `0` and
// the `error_message` MUST be non-empty.
//
// A `partial_success` message with an empty value (rejected_<signal> = 0 and
// `error_message` = "") is equivalent to it not being set/present. Senders
// SHOULD interpret it the same way as in the full success case.
ExportTracePartialSuccess partial_success = 1;
}
message ExportTracePartialSuccess {
// The number of rejected spans.
//
// A `rejected_<signal>` field holding a `0` value indicates that the
// request was fully accepted.
int64 rejected_spans = 1;
// A developer-facing human-readable message in English. It should be used
// either to explain why the server rejected parts of the data during a partial
// success or to convey warnings/suggestions during a full success. The message
// should offer guidance on how users can address such issues.
//
// error_message is an optional field. An error_message with an empty value
// is equivalent to it not being set.
string error_message = 2;
}
However, it looks like the OTLPExporterMixin which invokes the export method on the stub ignores the response message and does not provide a means of logging it. The current code looks like:
try:
self._client.Export(
request=self._translate_data(data),
metadata=self._headers,
timeout=deadline_sec - time(),
)
return self._result.SUCCESS # type: ignore [reportReturnType]
It would be ideal if there were a variable to automatically log partial failures like:
export OTEL_PYTHON_LOG_PARTIAL_SUCCESS=1
... that would cause partial success to be handled.
Describe the solution you'd like
A potential solution might look something like:
def _do_nothing_with_response(unused_response):
pass
def _log_partial_success(response):
if response.HasField('partial_success'):
logger.debug(response)
_response_handler = _log_partial_success if (os.getenv('OTEL_PYTHON_LOG_PARTIAL_SUCCESS', '0') == '1') else _do_nothing_with_response
...
try:
response = self._client.Export(
request=self._translate_data(data),
metadata=self._headers,
timeout=deadline_sec - time(),
)
_response_handler(response)
return self._result.SUCCESS # type: ignore [reportReturnType]
....
Describe alternatives you've considered
Also considered something more general where an arbitrary partial_success_handler could be supplied, with the simple env-based enablement building off a more general approach like that (allowing behaviors other than just logging the partial success, such as tabulating rejection counts client-side).
Additional Context
No response
Would you like to implement a fix?
None
Tip
React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.