{"action":"created","comment":{"url":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/comments/204007555","pull_request_review_id":139012108,"id":204007555,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIwNDAwNzU1NQ==","diff_hunk":"@@ -45,36 +44,38 @@ var _ io.Reader = (*payload)(nil)\n func newPayload() *payload {\n \tp := &payload{\n \t\theader: make([]byte, 8),\n+\t\tbuf:    new(bytes.Buffer),","path":"ddtrace/tracer/payload.go","position":19,"original_position":19,"commit_id":"0840e2f6a4bdc6784445c0f9d855741d1f9921fa","original_commit_id":"0840e2f6a4bdc6784445c0f9d855741d1f9921fa","user":{"login":"gbbr","id":6686356,"node_id":"MDQ6VXNlcjY2ODYzNTY=","avatar_url":"https://avatars2.githubusercontent.com/u/6686356?v=4","gravatar_id":"","url":"https://api.github.com/users/gbbr","html_url":"https://github.com/gbbr","followers_url":"https://api.github.com/users/gbbr/followers","following_url":"https://api.github.com/users/gbbr/following{/other_user}","gists_url":"https://api.github.com/users/gbbr/gists{/gist_id}","starred_url":"https://api.github.com/users/gbbr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gbbr/subscriptions","organizations_url":"https://api.github.com/users/gbbr/orgs","repos_url":"https://api.github.com/users/gbbr/repos","events_url":"https://api.github.com/users/gbbr/events{/privacy}","received_events_url":"https://api.github.com/users/gbbr/received_events","type":"User","site_admin":false},"body":"What I meant was to completely remove this line, and to not make `bytes.Buffer` a pointer in the structure. I found that's the way that `bytes.Buffer` is generally used. We can just use ampersand `&` to reference it everywhere else.","created_at":"2018-07-20T10:53:24Z","updated_at":"2018-07-20T10:55:07Z","html_url":"https://github.com/DataDog/dd-trace-go/pull/275#discussion_r204007555","pull_request_url":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/275","author_association":"MEMBER","_links":{"self":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/comments/204007555"},"html":{"href":"https://github.com/DataDog/dd-trace-go/pull/275#discussion_r204007555"},"pull_request":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/275"}}},"pull_request":{"url":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/275","id":202685067,"node_id":"MDExOlB1bGxSZXF1ZXN0MjAyNjg1MDY3","html_url":"https://github.com/DataDog/dd-trace-go/pull/275","diff_url":"https://github.com/DataDog/dd-trace-go/pull/275.diff","patch_url":"https://github.com/DataDog/dd-trace-go/pull/275.patch","issue_url":"https://api.github.com/repos/DataDog/dd-trace-go/issues/275","number":275,"state":"open","locked":false,"title":"ddtrace/tracer: Improve implementation of payload","user":{"login":"shidenkai0","id":8296549,"node_id":"MDQ6VXNlcjgyOTY1NDk=","avatar_url":"https://avatars3.githubusercontent.com/u/8296549?v=4","gravatar_id":"","url":"https://api.github.com/users/shidenkai0","html_url":"https://github.com/shidenkai0","followers_url":"https://api.github.com/users/shidenkai0/followers","following_url":"https://api.github.com/users/shidenkai0/following{/other_user}","gists_url":"https://api.github.com/users/shidenkai0/gists{/gist_id}","starred_url":"https://api.github.com/users/shidenkai0/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shidenkai0/subscriptions","organizations_url":"https://api.github.com/users/shidenkai0/orgs","repos_url":"https://api.github.com/users/shidenkai0/repos","events_url":"https://api.github.com/users/shidenkai0/events{/privacy}","received_events_url":"https://api.github.com/users/shidenkai0/received_events","type":"User","site_admin":false},"body":"Hi,\r\n\r\nThere is a bug in the current implementation of tracer.payload, which leads to a potential mismatch between the payload's content (in payload.buf) and the payload trace count (payload.count): http.Client.Do can call payload.Read, reading the content of payload.buf, while returning err != nil, thus leading to the payload not being reset, and a state where payload.buf is empty, while count !=0, since pushing a trace to the payload doesn't reset payload.count to 0, and the msgp array header depends on payload.count, this results in subsequent payload Reads returning an incorrect msgpack serialization of the traces array. Subsequent flushes to the agent therefore always return an error (Status: Bad Request), since the agent is unable to decode the incorrectly serialized traces array, while  payload.buf's content is still being read, thus repeating the cycle. The only way to get out of this corrupt state by then is for the service to generate enough traces to make the payload exceed payloadMaxSize before the next flush. \r\nThis is rather bothersome because it makes the instrumented service stop reporting APM traces, as traces are lost (being read and therefore lost on the tracer end, while not being decodable by the DataDog agent).\r\n\r\nThis issue typically happens when the DataDog agent is restarted, or when deploying a new version of the agent: all our production Ruby services keep on having their APM traces collected, while some go services don't, depending on whether they were flushing traces at the moment the newly started agent began listening on port 8126 (and read the \"traces flush\" request body without being able to process it, returning an error while having emptied the payload buffer on the instrumented services' end).\r\n\r\nThis pull request solves the issue by modifying the payload to make it impossible to have a mismatch between the msgpack header and the actual traces content of the payload. \r\nI also modified the payload benchmark to be more meaningful by actually measuring the whole payload lifecycle: pushing traces to a payload, then reading the whole payload, instead of only measuring pushing traces to the payload.\r\n\r\nThe new payload implementation performs the same as the former, while allocating much less memory for large payload sizes (10 MB).\r\n\r\nBenchmark results on my Machine: \r\n\r\n- Previous implementation: \r\n`goos: darwin\r\ngoarch: amd64\r\npkg: gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer\r\nBenchmarkPayloadThroughput/100KB-8         \t  200000\t     10198 ns/op\t     321 B/op\t      10 allocs/op\r\nBenchmarkPayloadThroughput/1MB-8           \t   10000\t    109813 ns/op\t     712 B/op\t      10 allocs/op\r\nBenchmarkPayloadThroughput/10MB-8          \t    1000\t   1618488 ns/op\t   32120 B/op\t      10 allocs/op\r\nPASS\r\nok  \tgopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer\t5.124s`\r\n\r\n- New implementation from this PR: \r\n`goos: darwin\r\ngoarch: amd64\r\npkg: gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer\r\nBenchmarkPayloadThroughput/100KB-8         \t  200000\t     10786 ns/op\t    1073 B/op\t      15 allocs/op\r\nBenchmarkPayloadThroughput/1MB-8           \t   10000\t    117055 ns/op\t    1102 B/op\t      15 allocs/op\r\nBenchmarkPayloadThroughput/10MB-8          \t    1000\t   1300400 ns/op\t    3752 B/op\t      15 allocs/op\r\nPASS\r\nok  \tgopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer\t4.934s`\r\n\r\nI hope my explanation was clear enough, and am available if you require any additional information.","created_at":"2018-07-19T21:11:36Z","updated_at":"2018-07-20T10:55:07Z","closed_at":null,"merged_at":null,"merge_commit_sha":"b6aa2752db75f5f76812871eb10634a1ff175fcf","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/275/commits","review_comments_url":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/275/comments","review_comment_url":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/comments{/number}","comments_url":"https://api.github.com/repos/DataDog/dd-trace-go/issues/275/comments","statuses_url":"https://api.github.com/repos/DataDog/dd-trace-go/statuses/0840e2f6a4bdc6784445c0f9d855741d1f9921fa","head":{"label":"shidenkai0:v1","ref":"v1","sha":"0840e2f6a4bdc6784445c0f9d855741d1f9921fa","user":{"login":"shidenkai0","id":8296549,"node_id":"MDQ6VXNlcjgyOTY1NDk=","avatar_url":"https://avatars3.githubusercontent.com/u/8296549?v=4","gravatar_id":"","url":"https://api.github.com/users/shidenkai0","html_url":"https://github.com/shidenkai0","followers_url":"https://api.github.com/users/shidenkai0/followers","following_url":"https://api.github.com/users/shidenkai0/following{/other_user}","gists_url":"https://api.github.com/users/shidenkai0/gists{/gist_id}","starred_url":"https://api.github.com/users/shidenkai0/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shidenkai0/subscriptions","organizations_url":"https://api.github.com/users/shidenkai0/orgs","repos_url":"https://api.github.com/users/shidenkai0/repos","events_url":"https://api.github.com/users/shidenkai0/events{/privacy}","received_events_url":"https://api.github.com/users/shidenkai0/received_events","type":"User","site_admin":false},"repo":{"id":95206630,"node_id":"MDEwOlJlcG9zaXRvcnk5NTIwNjYzMA==","name":"dd-trace-go","full_name":"shidenkai0/dd-trace-go","owner":{"login":"shidenkai0","id":8296549,"node_id":"MDQ6VXNlcjgyOTY1NDk=","avatar_url":"https://avatars3.githubusercontent.com/u/8296549?v=4","gravatar_id":"","url":"https://api.github.com/users/shidenkai0","html_url":"https://github.com/shidenkai0","followers_url":"https://api.github.com/users/shidenkai0/followers","following_url":"https://api.github.com/users/shidenkai0/following{/other_user}","gists_url":"https://api.github.com/users/shidenkai0/gists{/gist_id}","starred_url":"https://api.github.com/users/shidenkai0/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shidenkai0/subscriptions","organizations_url":"https://api.github.com/users/shidenkai0/orgs","repos_url":"https://api.github.com/users/shidenkai0/repos","events_url":"https://api.github.com/users/shidenkai0/events{/privacy}","received_events_url":"https://api.github.com/users/shidenkai0/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/shidenkai0/dd-trace-go","description":"Datadog tracing in Go","fork":true,"url":"https://api.github.com/repos/shidenkai0/dd-trace-go","forks_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/forks","keys_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/keys{/key_id}","collaborators_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/teams","hooks_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/hooks","issue_events_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/issues/events{/number}","events_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/events","assignees_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/assignees{/user}","branches_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/branches{/branch}","tags_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/tags","blobs_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/git/refs{/sha}","trees_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/git/trees{/sha}","statuses_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/statuses/{sha}","languages_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/languages","stargazers_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/stargazers","contributors_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/contributors","subscribers_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/subscribers","subscription_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/subscription","commits_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/commits{/sha}","git_commits_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/git/commits{/sha}","comments_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/comments{/number}","issue_comment_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/issues/comments{/number}","contents_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/contents/{+path}","compare_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/compare/{base}...{head}","merges_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/merges","archive_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/downloads","issues_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/issues{/number}","pulls_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/pulls{/number}","milestones_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/milestones{/number}","notifications_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/labels{/name}","releases_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/releases{/id}","deployments_url":"https://api.github.com/repos/shidenkai0/dd-trace-go/deployments","created_at":"2017-06-23T09:42:59Z","updated_at":"2018-07-17T10:14:23Z","pushed_at":"2018-07-20T10:37:29Z","git_url":"git://github.com/shidenkai0/dd-trace-go.git","ssh_url":"git@github.com:shidenkai0/dd-trace-go.git","clone_url":"https://github.com/shidenkai0/dd-trace-go.git","svn_url":"https://github.com/shidenkai0/dd-trace-go","homepage":"https://godoc.org/github.com/DataDog/dd-trace-go/tracer","size":1775,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"DataDog:v1","ref":"v1","sha":"0a3c14fdc3b36fe154aa5b222dcfdd3f4db203a1","user":{"login":"DataDog","id":365230,"node_id":"MDEyOk9yZ2FuaXphdGlvbjM2NTIzMA==","avatar_url":"https://avatars1.githubusercontent.com/u/365230?v=4","gravatar_id":"","url":"https://api.github.com/users/DataDog","html_url":"https://github.com/DataDog","followers_url":"https://api.github.com/users/DataDog/followers","following_url":"https://api.github.com/users/DataDog/following{/other_user}","gists_url":"https://api.github.com/users/DataDog/gists{/gist_id}","starred_url":"https://api.github.com/users/DataDog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DataDog/subscriptions","organizations_url":"https://api.github.com/users/DataDog/orgs","repos_url":"https://api.github.com/users/DataDog/repos","events_url":"https://api.github.com/users/DataDog/events{/privacy}","received_events_url":"https://api.github.com/users/DataDog/received_events","type":"Organization","site_admin":false},"repo":{"id":67680905,"node_id":"MDEwOlJlcG9zaXRvcnk2NzY4MDkwNQ==","name":"dd-trace-go","full_name":"DataDog/dd-trace-go","owner":{"login":"DataDog","id":365230,"node_id":"MDEyOk9yZ2FuaXphdGlvbjM2NTIzMA==","avatar_url":"https://avatars1.githubusercontent.com/u/365230?v=4","gravatar_id":"","url":"https://api.github.com/users/DataDog","html_url":"https://github.com/DataDog","followers_url":"https://api.github.com/users/DataDog/followers","following_url":"https://api.github.com/users/DataDog/following{/other_user}","gists_url":"https://api.github.com/users/DataDog/gists{/gist_id}","starred_url":"https://api.github.com/users/DataDog/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/DataDog/subscriptions","organizations_url":"https://api.github.com/users/DataDog/orgs","repos_url":"https://api.github.com/users/DataDog/repos","events_url":"https://api.github.com/users/DataDog/events{/privacy}","received_events_url":"https://api.github.com/users/DataDog/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/DataDog/dd-trace-go","description":"A Go tracing package for Datadog APM","fork":false,"url":"https://api.github.com/repos/DataDog/dd-trace-go","forks_url":"https://api.github.com/repos/DataDog/dd-trace-go/forks","keys_url":"https://api.github.com/repos/DataDog/dd-trace-go/keys{/key_id}","collaborators_url":"https://api.github.com/repos/DataDog/dd-trace-go/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/DataDog/dd-trace-go/teams","hooks_url":"https://api.github.com/repos/DataDog/dd-trace-go/hooks","issue_events_url":"https://api.github.com/repos/DataDog/dd-trace-go/issues/events{/number}","events_url":"https://api.github.com/repos/DataDog/dd-trace-go/events","assignees_url":"https://api.github.com/repos/DataDog/dd-trace-go/assignees{/user}","branches_url":"https://api.github.com/repos/DataDog/dd-trace-go/branches{/branch}","tags_url":"https://api.github.com/repos/DataDog/dd-trace-go/tags","blobs_url":"https://api.github.com/repos/DataDog/dd-trace-go/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/DataDog/dd-trace-go/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/DataDog/dd-trace-go/git/refs{/sha}","trees_url":"https://api.github.com/repos/DataDog/dd-trace-go/git/trees{/sha}","statuses_url":"https://api.github.com/repos/DataDog/dd-trace-go/statuses/{sha}","languages_url":"https://api.github.com/repos/DataDog/dd-trace-go/languages","stargazers_url":"https://api.github.com/repos/DataDog/dd-trace-go/stargazers","contributors_url":"https://api.github.com/repos/DataDog/dd-trace-go/contributors","subscribers_url":"https://api.github.com/repos/DataDog/dd-trace-go/subscribers","subscription_url":"https://api.github.com/repos/DataDog/dd-trace-go/subscription","commits_url":"https://api.github.com/repos/DataDog/dd-trace-go/commits{/sha}","git_commits_url":"https://api.github.com/repos/DataDog/dd-trace-go/git/commits{/sha}","comments_url":"https://api.github.com/repos/DataDog/dd-trace-go/comments{/number}","issue_comment_url":"https://api.github.com/repos/DataDog/dd-trace-go/issues/comments{/number}","contents_url":"https://api.github.com/repos/DataDog/dd-trace-go/contents/{+path}","compare_url":"https://api.github.com/repos/DataDog/dd-trace-go/compare/{base}...{head}","merges_url":"https://api.github.com/repos/DataDog/dd-trace-go/merges","archive_url":"https://api.github.com/repos/DataDog/dd-trace-go/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/DataDog/dd-trace-go/downloads","issues_url":"https://api.github.com/repos/DataDog/dd-trace-go/issues{/number}","pulls_url":"https://api.github.com/repos/DataDog/dd-trace-go/pulls{/number}","milestones_url":"https://api.github.com/repos/DataDog/dd-trace-go/milestones{/number}","notifications_url":"https://api.github.com/repos/DataDog/dd-trace-go/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/DataDog/dd-trace-go/labels{/name}","releases_url":"https://api.github.com/repos/DataDog/dd-trace-go/releases{/id}","deployments_url":"https://api.github.com/repos/DataDog/dd-trace-go/deployments","created_at":"2016-09-08T07:47:44Z","updated_at":"2018-07-11T17:03:32Z","pushed_at":"2018-07-20T10:37:31Z","git_url":"git://github.com/DataDog/dd-trace-go.git","ssh_url":"git@github.com:DataDog/dd-trace-go.git","clone_url":"https://github.com/DataDog/dd-trace-go.git","svn_url":"https://github.com/DataDog/dd-trace-go","homepage":"https://docs.datadoghq.com/tracing/","size":1665,"stargazers_count":61,"watchers_count":61,"language":"Go","has_issues":true,"has_projects":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":41,"mirror_url":null,"archived":false,"open_issues_count":14,"license":{"key":"other","name":"Other","spdx_id":null,"url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":41,"open_issues":14,"watchers":61,"default_branch":"v1"}},"_links":{"self":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/275"},"html":{"href":"https://github.com/DataDog/dd-trace-go/pull/275"},"issue":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/issues/275"},"comments":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/issues/275/comments"},"review_comments":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/275/comments"},"review_comment":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/pulls/275/commits"},"statuses":{"href":"https://api.github.com/repos/DataDog/dd-trace-go/statuses/0840e2f6a4bdc6784445c0f9d855741d1f9921fa"}},"author_association":"NONE"}}
{"actor":{"display_login":"gbbr"}}