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

ddtrace/tracer: reset payload after every transport attempt #319

Merged
merged 1 commit into from
Aug 23, 2018

Conversation

gbbr
Copy link
Contributor

@gbbr gbbr commented Aug 22, 2018

TODO

  • Revert staged version (Aug 22 10:45 AM EDT) before merging.

Change description

Previously, we were attempting to save the payload as long as the size
allows it in order to attempt and send it again on subsequent retries to
minimize the loss of traces.

This has proven to be a bad approach because there were cases where the
trace agent did only partial reads from the payload before failing,
leaving the payload in a state where it was impossible to decode using
msgpack (part of the buffer was already read).

This fix is an intermediary easy solution to the problem (we have
experimented with different approaches too, see #275). We can potentially
experiment later on with retry policies.

Closes #276

Previously, we were attempting to save the payload as long as the size
allows it in order to attempt and send it again on subsequent retries to
minimize the loss of traces.

This has proven to be a bad approach because there were cases were the
trace agent did only partial reads from the payload before failing,
leaving the payload in a state where it was impossible to decode using
msgpack (part of the buffer was already read).

This fix is an intermediary easy solution to the problem (we have
experimented with different approaches too, see #275). We can experiment
later on with retry policies.
@gbbr gbbr added bug unintended behavior that has to be fixed do-not-merge/WIP labels Aug 22, 2018
@gbbr gbbr added this to the next milestone Aug 22, 2018
@gbbr gbbr requested a review from palazzem August 22, 2018 11:42
@gbbr gbbr modified the milestones: next, 1.2.0 Aug 22, 2018
@gbbr gbbr merged commit cea9c1d into v1 Aug 23, 2018
@gbbr gbbr deleted the gbbr/payload-reset branch August 23, 2018 08:12
mingrammer pushed a commit to mingrammer/dd-trace-go that referenced this pull request Dec 22, 2020
)

Previously, we were attempting to save the payload as long as the size
allows it in order to attempt and send it again on subsequent retries to
minimize the loss of traces.

This has proven to be a bad approach because there were cases were the
trace agent did only partial reads from the payload before failing,
leaving the payload in a state where it was impossible to decode using
msgpack (part of the buffer was already read).

This fix is an intermediary easy solution to the problem (we have
experimented with different approaches too, see DataDog#275). We can experiment
later on with retry policies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unintended behavior that has to be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ddtrace/tracer: payload can only be read once
2 participants