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.

Sign up for GitHub

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

Jump to bottom

fix: SQS crashing error with no active transaction #2114

Merged
merged 5 commits into from Jun 17, 2021
Merged

fix: SQS crashing error with no active transaction #2114

merged 5 commits into from Jun 17, 2021

Conversation

astorm
Copy link
Contributor

@astorm astorm commented Jun 16, 2021 •
edited

Fixes #2113

This PR modifies adds an additional guard clause to the SQS instrumentation that checks to see if a span was actually started or not. It also removed the old currentTransaction guard clause from shouldIgnoreRequest and modifies the unit test accordingly.

Checklist

  • Implement code
  • Add tests
  • Add CHANGELOG.asciidoc entry

@elastic-apm-tech elastic-apm-tech added this to In Progress in APM-Agents (OLD) Jun 16, 2021
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Jun 16, 2021
@apmmachine
Copy link
Collaborator

apmmachine commented Jun 16, 2021 •
edited

💚 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: astorm commented: jenkins run the tests please

  • Start Time: 2021-06-16T18:43:26.109+0000

  • Duration: 19 min 39 sec

  • Commit: 85c3502

Test stats 🧪

Test Results
Failed 0
Passed 20944
Skipped 0
Total 20944

Trends 🧪

Image of Build Times

Image of Tests

trentm
trentm previously approved these changes Jun 16, 2021
View reviewed changes
Copy link
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

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

One nit.

Assuming tests pass, LGTM.

CHANGELOG.asciidoc Outdated Show resolved Hide resolved
@astorm
Copy link
Contributor Author

astorm commented Jun 16, 2021

jenkins run the tests please

@astorm astorm requested a review from trentm June 16, 2021 21:25
@astorm astorm merged commit a07eb3a into master Jun 17, 2021
@astorm astorm deleted the astorm/sqs-span-no-transaction branch June 17, 2021 21:28
APM-Agents (OLD) automation moved this from In Progress to Done Jun 17, 2021
dgieselaar pushed a commit to dgieselaar/apm-agent-nodejs that referenced this pull request Sep 10, 2021
@astorm @dgieselaar
fix: SQS crashing error with no active transaction (elastic#2114)
13491bc 
* fix: SQS crashing error with no active transaction

elastic#2113
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trentm trentm trentm approved these changes

Assignees
No one assigned
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
APM-Agents (OLD)
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SQS instrumentation can cause Errors
3 participants
@astorm @apmmachine @trentm