-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[xconsumererror]: Add Partial error type #14152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This PR adds functionality for consumers to create a partial error type. This will allow consumers to properly report partial success/failure with failed item counts, which can subsequently be used when reporting sent/failed metrics.
52dae4d to
933c0ea
Compare
|
Replicating @jmacd's comment on the original PR so the discussion can take place here instead. @jmacd said: I would like to see a draft, at least, for the documentation that will accompany the package, what the consumers need to know. For example, in the fanout consumer, existing logic returns immediately when one of the fanned-out consumers returns a non-nil error. I believe we need a blanket recommendation that these errors may be treated the same as success in cases like these. However even while continuing the fan-out, the returned error should join all the partial successes with good information that an OTLP receiver can convey to SDKs and user consoles. Here are some general rules that might make sense to apply:
I've tried to extend the documentation of the public API as much as I can to address the goals of this PR. This error in particular is to allow a consumer to express partial success as a permanent error. In The semantics of how it should be treated are kind of up to whoever is upstream of the consumer receiving the error. Most may just treat it as a permanent error, with the option of extracting the count of failed items if that's of any interest. In the case of the |
The linter did not like exporting a private type, which is a fair point. I want to keep the error type private so that the only way to produce a partial error also necessitates that it's permanent, so I changed the API to `IsPartial` which produces a count and a boolean.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #14152 +/- ##
==========================================
+ Coverage 92.24% 92.25% +0.01%
==========================================
Files 658 659 +1
Lines 41171 41199 +28
==========================================
+ Hits 37978 38008 +30
+ Misses 2185 2184 -1
+ Partials 1008 1007 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
This PR adds a new partial error type. This is for scenarios where a destination has reported that some number of items failed, without giving the consumer the ability to figure out which particular items failed.
The error can be treated equivalently to a permanent error; the only change this would make to a typical permanent error scenario is that consumers will now have the ability to report the right numbers to a statistic tracker like
exporterhelper.obsReportSender.Link to tracking issue
Part of #13423
Re-opened from #13927
Testing
I have a branch off this one that shows how the
exporterhelperpackage would leverage this error type to extract the proper failed item counts for metrics. https://github.com/braydonk/opentelemetry-collector/compare/partial_error...braydonk:opentelemetry-collector:count_partial_errors?expand=1Documentation
Documentation is primarily in the godoc of the public API. I'm not sure if there's any better places to include this information as well.