Extract Mongo library instrumentation #2789
Conversation
…nstrumentation into mongo-library-instrumentation
anuraaga
requested review from
iNikem,
jkwatson,
mateuszrzeszutek,
pavolloffay,
trask and
tylerbenson
as code owners
...ongo-3.1/library/src/main/java/io/opentelemetry/instrumentation/mongo/v3_1/MongoTracing.java
Outdated
Show resolved
Hide resolved
...src/main/groovy/io/opentelemetry/instrumentation/mongo/v3_1/AbstractMongo31ClientTest.groovy
Show resolved
Hide resolved
| for (CommandListener commandListener : commandListeners) { | ||
| if (commandListener instanceof TracingCommandListener) { | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
this may have been protecting against calling build() twice on the same builder
There was a problem hiding this comment.
Ah good point, restored dedupe. Strangely a new test I added doesn't fail even without these though, haven't dug into why that might be.
There was a problem hiding this comment.
a new test I added doesn't fail even without these though
oh no, maybe it's not needed after all(?)
There was a problem hiding this comment.
I wonder if our workForTraces doesn't actually check for extra traces? I thought it did
There was a problem hiding this comment.
try putting a sleep before it to give the second trace time to get there
There was a problem hiding this comment.
There isn't really anywhere to put a sleep - build just causes two listeners to be registered, which I could confirm in the debugger and Thread.dumpStack. Weird but not sure I'll dig too much into it
…nstrumentation into mongo-library-instrumentation
| @Override | ||
| void createCollectionCallingBuildTwice(String dbName, String collectionName) { | ||
| def options = MongoClientOptions.builder().description("some-description") | ||
| configureMongoClientOptions(options) | ||
| options.build() | ||
| MongoDatabase db = new MongoClient(new ServerAddress("localhost", port), options.build()).getDatabase(dbName) | ||
| db.createCollection(collectionName) | ||
| } |
| } | ||
|
|
||
| then: | ||
| assertTraces(1) { |
There was a problem hiding this comment.
try putting the sleep before this line, but agree not worth a lot of time, thx for adding the test and verifying manually 👍
No description provided.