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

Exporter retry logic should not be the responsibility of the SDK #509

Closed
jkwatson opened this issue Mar 10, 2020 · 4 comments · Fixed by #511
Closed

Exporter retry logic should not be the responsibility of the SDK #509

jkwatson opened this issue Mar 10, 2020 · 4 comments · Fixed by #511

Comments

@jkwatson
Copy link
Contributor

Currently, the span export API specification allows an exporter to return a "FAILED_RETRYABLE" code. Given the diversity of backends, and the multitude of possible failure modes (including HTTP 429s, which includes a suggested time delay in the response headers), I would suggest that retry logic should be the responsibility of the exporter itself, rather than trying to design an exporter API that would cover all the cases.

So, I suggest we remove the option of FAILED_RETRYABLE from the span export API, and have the exporter return only two possible results (FAILED and SUCCESS).

@tsloughter
Copy link
Member

Yes. We actually discussed this a while ago in a spec sig meeting and decided on exactly this but no one ever made the change to the SDK spec.

@jkwatson
Copy link
Contributor Author

Indeed. I had thought the change had been made, but apparently it hasn't. I'll put in a PR to update the spec.

@bogdandrutu
Copy link
Member

@jkwatson to be honest this does not change too much. In the OTLP implementation we still need to determine when the OTLP exporter should or should not do retries. I think the only change in the PR you referenced will be the return type, but then I will need to add a retry logic that still uses the logic implemented there to determine when to retry.

@bogdandrutu
Copy link
Member

I am not trying to say that we should not change the definition of the exporter, but I want to say that we still need open-telemetry/oteps#65 to determine when to retry in the OTLP exporter.

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 a pull request may close this issue.

3 participants