Skip to content
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

Add support for partial success in an OTLP export response [2] #2696

Merged

Conversation

joaopgrassi
Copy link
Member

@joaopgrassi joaopgrassi commented Jul 26, 2022

Fixes #2454, Closes #2636

This is a second take on #2636. It's basically the same changes, except it uses rejected_<signal> semantics instead of accepted_<signal> as discussed in the PRs and in the spec meeting on 7/19/2022.

Changes

This PR adds guidance around how servers can signal a partial success to clients. It indicates in which situations servers can respond with a partial success and how the fields introduced in opentelemetry-proto #414 should be populated.

I also refactored the OTLP/gRPC Response section a bit, to make it consistent with the HTTP one. The main inconsistency I found was that the gRPC section did not have clear sections for Success, Failure as the HTTP does. Now both share the same sections and the wording should be consistent between the two.

Related
Proto change: open-telemetry/opentelemetry-proto#414

cc @tigrannajaryan @jmacd @jsuereth @jack-berg @reyang @yurishkuro

@joaopgrassi joaopgrassi requested review from a team July 26, 2022 11:18
specification/protocol/otlp.md Outdated Show resolved Hide resolved
specification/protocol/otlp.md Outdated Show resolved Hide resolved
specification/protocol/otlp.md Outdated Show resolved Hide resolved
specification/protocol/otlp.md Outdated Show resolved Hide resolved
specification/protocol/otlp.md Show resolved Hide resolved
specification/protocol/otlp.md Outdated Show resolved Hide resolved
specification/protocol/otlp.md Outdated Show resolved Hide resolved
@joaopgrassi
Copy link
Member Author

Hi all, just an update on the current state of this PR:

All discussions are solved and I think we should be good to go!

The last thing to be solved (here) was around using the partial success to convey warnings/suggestions to clients when ALL the data was accepted.

This is useful, for example, when the server did some massaging on the data (character normalization) and wants to send a "warning" to clients that this happened. Another usage would be to signal suggestions to clients, e.g. "hey you are sending the data via a less efficient/secure way, you should do xyz" like @reyang suggested here.

Let me know your thoughts.

specification/protocol/otlp.md Outdated Show resolved Hide resolved
specification/protocol/otlp.md Outdated Show resolved Hide resolved
@jmacd
Copy link
Contributor

jmacd commented Aug 2, 2022

@joaopgrassi please rebase (or otherwise resolve "This branch is out-of-date with the base branch")?

@joaopgrassi
Copy link
Member Author

@jmacd rebased and also allowed edits from maintainers on the branch, in case it needs again and I'm away.

@jmacd jmacd merged commit dbea54d into open-telemetry:main Aug 2, 2022
@arminru arminru deleted the feature/otlp-partial-success-2 branch August 2, 2022 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability for OTLP response to signal partial acceptance of data
8 participants