Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Handle partial-success responses for OTLP trace #3106
Handle partial-success responses for OTLP trace #3106
Changes from all commits
92c8191
a67f694
bc374c3
0bda269
4ddf3b5
0e8a80c
8284366
8723d2e
b14e132
0c71272
14db574
54d9dcf
7d538e5
34b7fa5
d3ff369
e207c7a
3c35b52
7331bd6
4998910
20f5c53
a04ac8a
96eadb0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm conflicted about where this should live. On the one hand, I would like users to create an ErrorHandler that does something more intelligent than just log a string. On the other hand, I would prefer not to expose a new API surface needlessly.
If no one has any other objections to this than I can live with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a fan of structured logging; I would advocate for a backwards-compatible mechanism to accomplish it. The
PartialSuccess
error can implement some future API that enables it to be expressed in a structured way when the producer logs it to a structure-aware logger.I didn't know where to put it either. At this level in the current package structure, it falls into the top-level package, which feels OK to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With no objections to this, we can leave it where it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather have these live in an internal package, at lease to start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what a user will be able to do with an error handler other than log the error given they cannot retry the export. And adding this to the public API seems like adding something only OTel developers will use or care about, not users.