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: ensure correct run context for 'mongodb' instrumentation #2512

Merged
merged 10 commits into from
Jan 6, 2022
Merged

fix: ensure correct run context for 'mongodb' instrumentation #2512

merged 10 commits into from
Jan 6, 2022

Conversation

trentm
Copy link
Member

@trentm trentm commented Dec 9, 2021 •
edited
Loading

Also:

Refs: #2430
Fixes: #2467

Checklist

  • Whoa, we aren't running TAV=mongodb in Jenkins. Why is that?
  • Implement code
  • Add tests
  • Pass TAV tests. Still getting .end() already called error in Jenkins.
  • Add CHANGELOG.asciidoc entry

@trentm trentm self-assigned this Dec 9, 2021
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Dec 9, 2021
@apmmachine
Copy link
Contributor

apmmachine commented Dec 9, 2021 •
edited
Loading

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2022-01-05T00:01:43.477+0000

  • Duration: 21 min 0 sec

  • Commit: 8b4466d

Test stats 🧪

Test Results
Failed 0
Passed 242863
Skipped 0
Total 242863

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run module tests for <modules> : Run TAV tests for one or more modules, where <modules> can be either a comma separated list of modules (e.g. memcached,redis) or the string literal ALL to test all modules

  • run benchmark tests : Run the benchmark test only.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@trentm
Copy link
Member Author

trentm commented Dec 9, 2021

run module tests for mongodb

@trentm
see if this fixes the only-in-Jenkins TAV failure
d8cc666 
    -- running test "node test/instrumentation/modules/mongodb.test.js" with mongodb
    /app/node_modules/mongodb/lib/utils.js:698
            throw error;
            ^

    TypeError: Cannot read property 'db' of undefined
        at server.connect (/app/test/instrumentation/modules/mongodb.test.js:49:24)
        at /app/node_modules/mongodb/lib/utils.js:695:9
        at err (/app/node_modules/mongodb/lib/mongo_client.js:285:23)
        at connectCallback (/app/node_modules/mongodb/lib/operations/connect.js:367:5)
        at topology.connect.err (/app/node_modules/mongodb/lib/operations/connect.js:554:14)
        ...
@trentm
Copy link
Member Author

trentm commented Dec 9, 2021

run module tests for mongodb

@trentm
Copy link
Member Author

trentm commented Dec 9, 2021

run module tests for mongodb

@trentm
a few 'mongodb' instrumentation fixes
c7707f3 
- ensure a mongodb span is not accidentally a child of an inflight
  mongodb span
- Fix an issue where instrumentation of `new MongoClient(url)` would
  fail because the internal handling of `arguments` would add
  `arguments[1]` but arguments.length stayed at 1. Fix by first
  changing to an Array.
- Fix instrument of a connection made using the MongoClient.connect
  static method. #2467
@trentm trentm mentioned this pull request Dec 10, 2021
Closed
3 tasks
@trentm
Copy link
Member Author

trentm commented Dec 10, 2021

run module tests for mongodb

@trentm
Copy link
Member Author

trentm commented Dec 10, 2021 •
edited
Loading

Getting test failures in Jenkins CI with TAV tests. At least one of the errors is with [email protected]:

https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-nodejs%2Fapm-agent-nodejs-mbp/detail/PR-2512/8/pipeline/1036

@trentm trentm mentioned this pull request Dec 13, 2021
Closed
38 tasks
trentm added 2 commits January 4, 2022 13:16
@trentm
fix '.end() already called' errors when running in (slow) Jenkins
ce7814d 
The issue was that `tape` will automatically `t.end()` an *async* test case
    tape.test('my test case', async function () { ... })
when the returned Promise resolves. This can happen before the
`resetAgent` callback has been called.
@trentm
Merge branch 'master' into trentm/run-context-mongodb
307af64
@trentm
Copy link
Member Author

trentm commented Jan 4, 2022

run module tests for mongodb

@trentm
Copy link
Member Author

trentm commented Jan 4, 2022

run module tests for mongodb

@trentm trentm requested a review from astorm January 5, 2022 00:01
@trentm trentm marked this pull request as ready for review January 5, 2022 00:01
astorm
astorm previously approved these changes Jan 6, 2022
View reviewed changes
@trentm trentm merged commit 7d2e759 into master Jan 6, 2022
@trentm trentm deleted the trentm/run-context-mongodb branch January 6, 2022 16:40
@apmmachine
Copy link
Contributor

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2022-01-06T16:37:24.428+0000

  • Duration: 20 min 59 sec

  • Commit: d91c22a

Test stats 🧪

Test Results
Failed 0
Passed 242323
Skipped 0
Total 242323

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run module tests for <modules> : Run TAV tests for one or more modules, where <modules> can be either a comma separated list of modules (e.g. memcached,redis) or the string literal ALL to test all modules

  • run benchmark tests : Run the benchmark test only.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

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

@astorm astorm astorm left review comments

Assignees

@trentm trentm

Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MongoDB instrumentation not working with MongoClient.connect static method
3 participants
@trentm @apmmachine @astorm