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(mongodb): resolve collections for command #3919

Merged
merged 7 commits into from
Mar 19, 2024
Merged

fix(mongodb): resolve collections for command #3919

merged 7 commits into from
Mar 19, 2024

Conversation

david-luna
Copy link
Member

@david-luna david-luna commented Mar 18, 2024 •
edited
Loading

MongoDb cursors spawn some extra commands when the list of documents to find is big enough. In particular the command is names getMore and the shape of the command is a bit different than the known ones (insert, find, delete, ...)

This PR seeks for a specific property within the getMore command payload to resolve the collection name and use it to create the name of the corresponding span. Tests have been modified accordingly to check for the proper span naming.

Kudos to @sibelius to propose that change

Closes: #3834

Checklist

  • Implement code
  • Add tests
  • Update TypeScript typings
  • Update documentation
  • Add CHANGELOG.asciidoc entry
  • Commit message follows commit guidelines

@david-luna david-luna requested a review from trentm March 18, 2024 10:55
david-luna
david-luna commented Mar 18, 2024
View reviewed changes
Copy link
Member Author

@david-luna david-luna left a comment

Choose a reason for hiding this comment

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

/test tav mongodb

@david-luna
Copy link
Member Author

TAV tests reveal we have the context issue also for _getMore command. Running locally I found the issue does get solved for versions >=4.11.0. Probably that command was not queued in the pool but somewhere else.

running

npm run docker:start mongodb
node ./test/instrumentation/modules/mongodb/fixtures/use-mongodb-async-context.js

gives the following traces

trace c5b88a
    `- transaction de70b1 "GET unknown route" (11.595ms, GET http://localhost:54836/insert -> 200)
       `- span a647d4 "elasticapm.test.insert" (4ms, mongodb)
       `- span c3c1b6 "elasticapm.test.getMore" (2ms, mongodb)
       `- span ea6c55 "elasticapm.test.getMore" (3ms, mongodb)
       `- span 4ad67f "elasticapm.test.getMore" (3ms, mongodb)
       `- span 8e2850 "elasticapm.test.getMore" (4ms, mongodb)
       `- span 287fca "elasticapm.test.getMore" (6ms, mongodb)
       `- span 052e18 "elasticapm.test.getMore" (6ms, mongodb)
       `- span eb7384 "elasticapm.test.getMore" (9ms, mongodb)
       `- span 20f63a "elasticapm.test.getMore" (8ms, mongodb)
       `- span ac4220 "elasticapm.test.getMore" (9ms, mongodb)
       `- span 0917b1 "elasticapm.test.getMore" (9ms, mongodb)
       `- span f291ce "elasticapm.test.getMore" (10ms, mongodb)
       `- span 970ce8 "elasticapm.test.getMore" (10ms, mongodb)
       `- span 60b494 "elasticapm.test.getMore" (9ms, mongodb)
    trace e34197
    `- transaction ffcff4 "GET unknown route" (30.211ms, GET http://localhost:54836/find -> 200)
       `- span e80b2f "elasticapm.test.find" (6ms, mongodb)
    trace b48ab5
    `- transaction cbe315 "GET unknown route" (71.778ms, GET http://localhost:54836/find -> 200)
       `- span 1c94e3 "elasticapm.test.find" (1ms, mongodb)
       `- span 2007b3 "elasticapm.test.getMore" (6ms, mongodb)
       `- span 09b0ec "elasticapm.test.getMore" (9ms, mongodb)
       `- span de8b16 "elasticapm.test.getMore" (8ms, mongodb)
       `- span 95be83 "elasticapm.test.getMore" (8ms, mongodb)
       `- span b5cdbd "elasticapm.test.getMore" (9ms, mongodb)
       `- span b4119f "elasticapm.test.getMore" (9ms, mongodb)
       `- span b19960 "elasticapm.test.getMore" (9ms, mongodb)
       `- span 274cc8 "elasticapm.test.getMore" (9ms, mongodb)
       `- span b1f249 "elasticapm.test.getMore" (10ms, mongodb)
       `- span 1ecc9b "elasticapm.test.getMore" (10ms, mongodb)
       `- span 18e0b1 "elasticapm.test.getMore" (10ms, mongodb)
       `- span 7e0a5e "elasticapm.test.getMore" (11ms, mongodb)
       `- span 118b2a "elasticapm.test.getMore" (9ms, mongodb)
       `- span 8de280 "elasticapm.test.getMore" (10ms, mongodb)
       `- span 955388 "elasticapm.test.getMore" (7ms, mongodb)
    trace 05b82d
    `- transaction 1c0cbc "GET unknown route" (71.271ms, GET http://localhost:54836/find -> 200)
       `- span 1e5c1c "elasticapm.test.find" (2ms, mongodb)
       `- span 827600 "elasticapm.test.getMore" (5ms, mongodb)
       `- span 05e6f4 "elasticapm.test.getMore" (6ms, mongodb)
       `- span ca379c "elasticapm.test.getMore" (8ms, mongodb)
       `- span 499d41 "elasticapm.test.getMore" (8ms, mongodb)
       `- span 72188f "elasticapm.test.getMore" (9ms, mongodb)
       `- span 44445e "elasticapm.test.getMore" (8ms, mongodb)
       `- span 9b5945 "elasticapm.test.getMore" (7ms, mongodb)
       `- span 19451d "elasticapm.test.getMore" (9ms, mongodb)
       `- span 98ed9f "elasticapm.test.getMore" (9ms, mongodb)
       `- span 24abe2 "elasticapm.test.getMore" (9ms, mongodb)
       `- span 52c626 "elasticapm.test.getMore" (9ms, mongodb)
       `- span 85efe5 "elasticapm.test.getMore" (10ms, mongodb)
       `- span 749068 "elasticapm.test.getMore" (10ms, mongodb)
       `- span a33393 "elasticapm.test.getMore" (11ms, mongodb)
       `- span 7a5d3c "elasticapm.test.getMore" (10ms, mongodb)
       `- span fd2d54 "elasticapm.test.getMore" (10ms, mongodb)
       `- span 3ca0d9 "elasticapm.test.getMore" (9ms, mongodb)
       `- span 54828c "elasticapm.test.getMore" (11ms, mongodb)
       `- span b74a6d "elasticapm.test.getMore" (10ms, mongodb)
       `- span 39f635 "elasticapm.test.getMore" (10ms, mongodb)
       `- span 1c8b84 "elasticapm.test.getMore" (9ms, mongodb)
       `- span a263f5 "elasticapm.test.getMore" (7ms, mongodb)
    trace 86d833
    `- transaction ea0727 "GET unknown route" (36.034ms, GET http://localhost:54836/find -> 200)
       `- span 140147 "elasticapm.test.find" (8ms, mongodb)
    trace 046ce8
    `- transaction a52252 "GET unknown route" (44.239ms, GET http://localhost:54836/find -> 200)
       `- span f84a69 "elasticapm.test.find" (3ms, mongodb)
    trace 685092
    `- transaction 62330d "GET unknown route" (58.512ms, GET http://localhost:54836/find -> 200)
       `- span 84c043 "elasticapm.test.find" (4ms, mongodb)
    trace c10b38
    `- transaction ec4ef9 "GET unknown route" (54.991ms, GET http://localhost:54836/find -> 200)
       `- span b852f3 "elasticapm.test.find" (1ms, mongodb)
    trace 285a8c
    `- transaction 6266f6 "GET unknown route" (46.731ms, GET http://localhost:54836/find -> 200)
       `- span 954acb "elasticapm.test.find" (3ms, mongodb)
    trace c9fcdf
    `- transaction 5afc6f "GET unknown route" (47.302ms, GET http://localhost:54836/find -> 200)
       `- span 6c0b0b "elasticapm.test.find" (2ms, mongodb)
    trace 476fe6
    `- transaction ba486e "GET unknown route" (46.2ms, GET http://localhost:54836/find -> 200)
       `- span b80580 "elasticapm.test.find" (3ms, mongodb)
    trace fecbf6
    `- transaction 8f85ec "GET unknown route" (47.884ms, GET http://localhost:54836/find -> 200)
       `- span 43c699 "elasticapm.test.find" (2ms, mongodb)
    trace fee660
    `- transaction 7cc5ac "GET unknown route" (48.585ms, GET http://localhost:54836/find -> 200)
       `- span edb69d "elasticapm.test.find" (1ms, mongodb)
    trace ae5292
    `- transaction e03859 "GET unknown route" (48.992ms, GET http://localhost:54836/find -> 200)
       `- span fbdaa7 "elasticapm.test.find" (2ms, mongodb)
    trace 4e3d89
    `- transaction d02db7 "GET unknown route" (53.574ms, GET http://localhost:54836/find -> 200)
       `- span 9071a3 "elasticapm.test.find" (4ms, mongodb)
    trace 881cff
    `- transaction 859b73 "GET unknown route" (51.69ms, GET http://localhost:54836/find -> 200)
       `- span 50f381 "elasticapm.test.find" (1ms, mongodb)
    trace ecd3a1
    `- transaction 1e8c3f "GET unknown route" (53.189ms, GET http://localhost:54836/find -> 200)
       `- span 6b486b "elasticapm.test.find" (1ms, mongodb)
    trace 237c65
    `- transaction 2ba225 "GET unknown route" (64.56ms, GET http://localhost:54836/find -> 200)
       `- span 0a4d56 "elasticapm.test.find" (2ms, mongodb)
    trace cdf515
    `- transaction 1b46bc "GET unknown route" (71.593ms, GET http://localhost:54836/find -> 200)
       `- span 3ffd9a "elasticapm.test.find" (7ms, mongodb)
    trace b63191
    `- transaction 21dc9c "GET unknown route" (53.805ms, GET http://localhost:54836/find -> 200)
       `- span 9cba2a "elasticapm.test.find" (2ms, mongodb)
    trace 4231aa
    `- transaction d14208 "GET unknown route" (54.279ms, GET http://localhost:54836/find -> 200)
       `- span 2c12a9 "elasticapm.test.find" (1ms, mongodb)
    trace 87fe2d
    `- transaction 7411b0 "GET unknown route" (55.582ms, GET http://localhost:54836/find -> 200)
       `- span 8aba74 "elasticapm.test.find" (1ms, mongodb)
    trace c86afe
    `- transaction 3217c8 "GET unknown route" (58.234ms, GET http://localhost:54836/find -> 200)
       `- span e36ad6 "elasticapm.test.find" (5ms, mongodb)
    trace 48ec26
    `- transaction b8353e "GET unknown route" (54.772ms, GET http://localhost:54836/find -> 200)
       `- span 7893a2 "elasticapm.test.find" (2ms, mongodb)
    trace 795884
    `- transaction ffc9c3 "GET unknown route" (58.131ms, GET http://localhost:54836/find -> 200)
       `- span fb2ff3 "elasticapm.test.find" (4ms, mongodb)
    trace 1f09b5
    `- transaction 75916d "GET unknown route" (58.739ms, GET http://localhost:54836/find -> 200)
       `- span 8e6dd6 "elasticapm.test.find" (3ms, mongodb)
    trace 7fdfc0
    `- transaction c1dc98 "GET unknown route" (62.114ms, GET http://localhost:54836/find -> 200)
       `- span 13ca2f "elasticapm.test.find" (6ms, mongodb)
    trace ade20c
    `- transaction 56850c "GET unknown route" (61.565ms, GET http://localhost:54836/find -> 200)
       `- span 607537 "elasticapm.test.find" (3ms, mongodb)
    trace 9081f9
    `- transaction de3227 "GET unknown route" (62.153ms, GET http://localhost:54836/find -> 200)
       `- span 69a127 "elasticapm.test.find" (3ms, mongodb)
    trace b43e8f
    `- transaction 0bbe85 "GET unknown route" (79.398ms, GET http://localhost:54836/find -> 200)
       `- span e25bc4 "elasticapm.test.find" (4ms, mongodb)
    trace 3fe4b2
    `- transaction 00b30e "GET unknown route" (81.123ms, GET http://localhost:54836/find -> 200)
       `- span 36ca39 "elasticapm.test.find" (13ms, mongodb)
    trace be9588
    `- transaction 6185cb "GET unknown route" (59.821ms, GET http://localhost:54836/find -> 200)
       `- span 5a887e "elasticapm.test.find" (2ms, mongodb)
    trace d2cf38
    `- transaction b7ec9d "GET unknown route" (59.357ms, GET http://localhost:54836/find -> 200)
       `- span 898259 "elasticapm.test.find" (2ms, mongodb)
    trace 026de2
    `- transaction d82c2b "GET unknown route" (65.095ms, GET http://localhost:54836/find -> 200)
       `- span 1f9030 "elasticapm.test.find" (3ms, mongodb)
    trace 58ce9e
    `- transaction 476897 "GET unknown route" (82.262ms, GET http://localhost:54836/find -> 200)
       `- span 71d9ca "elasticapm.test.find" (2ms, mongodb)
    trace 5bef15
    `- transaction a4906a "GET unknown route" (82.694ms, GET http://localhost:54836/find -> 200)
       `- span 005b4f "elasticapm.test.find" (3ms, mongodb)
    trace 1e41fb
    `- transaction 0548f0 "GET unknown route" (61.705ms, GET http://localhost:54836/find -> 200)
       `- span a2bed9 "elasticapm.test.find" (5ms, mongodb)
    trace 3e5db0
    `- transaction cc334f "GET unknown route" (62.222ms, GET http://localhost:54836/find -> 200)
       `- span eb1627 "elasticapm.test.find" (5ms, mongodb)
    trace 7cb170
    `- transaction cc246b "GET unknown route" (62.771ms, GET http://localhost:54836/find -> 200)
       `- span 601579 "elasticapm.test.find" (5ms, mongodb)
    trace 3c0a34
    `- transaction 9d67b4 "GET unknown route" (63.306ms, GET http://localhost:54836/find -> 200)
       `- span 762a10 "elasticapm.test.find" (5ms, mongodb)
    trace 70ffb2
    `- transaction fe91b9 "GET unknown route" (63.764ms, GET http://localhost:54836/find -> 200)
       `- span 8f4632 "elasticapm.test.find" (5ms, mongodb)
    trace 99ea5f
    `- transaction 723a04 "GET unknown route" (64.326ms, GET http://localhost:54836/find -> 200)
       `- span b59fde "elasticapm.test.find" (4ms, mongodb)
    trace 2c6acd
    `- transaction a20ba1 "GET unknown route" (64.841ms, GET http://localhost:54836/find -> 200)
       `- span 3469d4 "elasticapm.test.find" (4ms, mongodb)
    trace c67583
    `- transaction fc7482 "GET unknown route" (65.378ms, GET http://localhost:54836/find -> 200)
       `- span 687c12 "elasticapm.test.find" (3ms, mongodb)
    trace addecd
    `- transaction e36a8d "GET unknown route" (66.078ms, GET http://localhost:54836/find -> 200)
       `- span 98ebf9 "elasticapm.test.find" (3ms, mongodb)
    trace 76db96
    `- transaction 47699f "GET unknown route" (68.932ms, GET http://localhost:54836/find -> 200)
       `- span 25d105 "elasticapm.test.find" (4ms, mongodb)
    trace 3f38c1
    `- transaction 7428f7 "GET unknown route" (69.516ms, GET http://localhost:54836/find -> 200)
       `- span d2df83 "elasticapm.test.find" (3ms, mongodb)
    trace ce593d
    `- transaction f71f47 "GET unknown route" (70.064ms, GET http://localhost:54836/find -> 200)
       `- span 0ab77f "elasticapm.test.find" (3ms, mongodb)
    trace e41da1
    `- transaction cd8a0d "GET unknown route" (70.56ms, GET http://localhost:54836/find -> 200)
       `- span fedecc "elasticapm.test.find" (3ms, mongodb)
    trace b42ad6
    `- transaction 4b30f7 "GET unknown route" (69.366ms, GET http://localhost:54836/find -> 200)
       `- span 6da396 "elasticapm.test.find" (5ms, mongodb)
    trace 36c1f3
    `- transaction a3767b "GET unknown route" (68.955ms, GET http://localhost:54836/find -> 200)
       `- span d6a8aa "elasticapm.test.find" (4ms, mongodb)
    trace f11615
    `- transaction 2f70b8 "GET unknown route" (3.019ms, GET http://localhost:54836/delete -> 200)
       `- span 2c4a8b "elasticapm.test.delete" (2ms, mongodb)

@trentm
Copy link
Member

trentm commented Mar 18, 2024

Running locally I found the issue does get solved for versions >=4.11.0.

So just skip that test for those early early versions, right? No need to accommodate <4.11.0:

$ npm info mongodb time
...
  '4.10.0': '2022-09-19T15:11:13.273Z',

david-luna
david-luna commented Mar 19, 2024
View reviewed changes
Copy link
Member Author

@david-luna david-luna left a comment

Choose a reason for hiding this comment

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

/test tav mongodb

@david-luna david-luna mentioned this pull request Mar 19, 2024
Closed
6 tasks
@david-luna david-luna merged commit 990442a into main Mar 19, 2024
18 checks passed
@david-luna david-luna deleted the dluna/3834-tune-collection-resolution branch March 19, 2024 22:13
@trentm trentm mentioned this pull request Apr 11, 2024
Merged
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fix <database>.$cmd.getMore span on mongodb
2 participants
@david-luna @trentm