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

Transaction.Discard sets TransactionData to nil #836

Merged
merged 1 commit into from Oct 20, 2020

Conversation

axw
Copy link
Member

@axw axw commented Oct 20, 2020

Fix Transaction.Discard so that it sets TransactionData
to nil, like its documentation says. This is necessary
to avoid adding a transaction to the sync.Pool multiple
times (e.g. by calling End after after it has been discarded.)

Closes #835

Fix Transaction.Discard so that it sets TransactionData
to nil, like its documentation says. This is necessary
to avoid Transaction.End from enqueuing a transaction
after it has been discarded.
@axw axw force-pushed the transation-discard-niltransactiondata branch from e526943 to a74dd31 Compare October 20, 2020 06:16
@apmmachine
Copy link
Collaborator

apmmachine commented Oct 20, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #836 updated]

  • Start Time: 2020-10-20T06:16:46.717+0000

  • Duration: 23 min 3 sec

Test stats 🧪

Test Results
Failed 0
Passed 7628
Skipped 185
Total 7813

@axw axw requested a review from a team October 20, 2020 06:26
@axw axw marked this pull request as ready for review October 20, 2020 06:26
@@ -35,6 +35,7 @@ https://github.com/elastic/apm-agent-go/compare/v1.8.0...master[View commits]
- Add cloud metadata, configurable with ELASTIC_APM_CLOUD_PROVIDER {pull}823[#(823)]
- Round ELASTIC_APM_SAMPLING_RATE with 4 digits precision {pull}828[#(828)]
- module/apmhttp: implement io.ReaderFrom in wrapped http.ResponseWriter {pull}830[#(830)]
- Fixed Transaction.Discard so that it sets TransactionData to nil {pull}836[#(836)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be under "bug fixes"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Go agent changelog has not, to date, separated additions from fixes. There's a template comment above with a "bug fixes" section, but it's not used anywhere. Maybe it should be...

I'd like to defer that to when we release, and look at what the other agents are doing.

@axw axw merged commit d781421 into elastic:master Oct 20, 2020
@axw axw deleted the transation-discard-niltransactiondata branch October 20, 2020 08:46
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.

tx.Discard() should set tx.TransactionData to nil as tx.End()
3 participants