feat(http): add error handling for exporting #4709
Conversation
|
|
pafi-code
force-pushed
the
feature/add_error_handling_for_otlp_exporters
branch
from
d3d4031 to
afb76f2
Compare
| try: | ||
| resp = self._export(serialized_data, deadline_sec - time()) | ||
| except Exception as error: |
There was a problem hiding this comment.
Would you mind opening an issue or point to some existing issue for the behavior you're encountering? We can discuss possible fixes there.
I think there is a bug in _export() on L157, where the code assumes the connection will succeed, and so the retry loop here is never executed. However the raised exception should be ultimately caught here
If you just want the logs to go away, you can set a filter on the opentelemetry.sdk._shared_internal logger or disable it
There was a problem hiding this comment.
Yea so the issue for me was that the logger logged the entire error trace to the std.out, which totally makes sense given the code that you shared.
So my goal was to still get to know when something fails and not just disable the logger, as I feel like the otlp_exporters should handle this internally.
On top of that I would like to generally comment that doing a retry over a try-except in _export() feels kinda odd to me.
Thanks for taking your time to look into this! :)
There was a problem hiding this comment.
Do you still think I should open an issue for that generally?
Description
When using the OTEL http exporters the std.out gets polluted by unhandled error traces. I don't think that this should be the desired result. This happens when the endpoint is not available when exporting telemetry. This can be cause by different events but in general I think it should be better handled.
Therefore I added error handling with more specific and shorted error messages.
Type of change
I'm not sure if the change type is rather fix or feature, as the service was still running.
How Has This Been Tested?
Does This PR Require a Contrib Repo Change?
Checklist: