Report only known collection names in MongoClientTracer (#1625) #1662
Conversation
jyemin
commented
Nov 17, 2020
jyemin
commented
- Use allow-list of commands that are known to use collection name as the value of the command
- Special-case getMore command, which uses a different field for the collection name
…ry#1625) * Use allow-list of commands that are known to use collection name as the value of the command * Special-case getMore command, which uses a different field for the collection name
jyemin
requested review from
anuraaga,
iNikem,
jkwatson,
mateuszrzeszutek,
trask and
tylerbenson
as code owners
|
| @@ -130,10 +133,24 @@ private static BsonValue scrub(BsonValue origin) { | |||
| return scrubbed; | |||
| } | |||
|
|
|||
| private static final Set<String> COMMANDS_WITH_COLLECTION_NAME_AS_VALUE = | |||
| new HashSet<>(asList( | |||
| "aggregate", "count", "distinct", "mapReduce", "geoSearch", "delete", "find", "killCursors", | |||
There was a problem hiding this comment.
While a n allow-list is in some ways not future proof, in practice I think it's safe as the trend in MongoDB has been to use new initiating stages in aggregation pipelines to add new functionality (e.g. the $changeStream stage.
| if (collectionValue != null && collectionValue.isString()) { | ||
| return collectionValue.asString().getValue(); | ||
| if (event.getCommandName().equals("getMore")) { | ||
| if (event.getCommand().containsKey("collection")) { |
There was a problem hiding this comment.
|
I signed the CLA just now, so I'm not sure why it's still saying that I'm not authorized |
|
Ya, the bot doesn't update the comment, which is confusing 🙁 I can see the EasyCLA check passed tho, so u are good 👍 |
| "findAndModify", | ||
| "insert", | ||
| "update", | ||
| "listIndexes")); |
There was a problem hiding this comment.
does it make sense to include create in this list?
There was a problem hiding this comment.
I considered it. It doesn't do any harm, but I left it out because in MongoDB AFAIK it's not so common to create collections explicitly, as MongoDB will create them implicitly on first insert.
But happy to add it (and drop as well, I guess) if there's support.
There was a problem hiding this comment.
Added create, drop, and createIndexes
…on name as their values
|
I can't tell why the build is failing. |
