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.

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

Prevent mongodb instrumentation from applying to async/reactive clients #476

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

jasonjkeller
Copy link
Contributor

@jasonjkeller jasonjkeller commented Oct 7, 2021

The mongodb-3.0, mongodb-3.1, and mongodb-3.7 instrumentation modules could incorrectly apply to the async/reactive mongo drivers which they are not intended to do. This can cause issues such as segment timeouts and incorrect metrics due to asynchronous behavior that the instrumentation isn't intended to handle.

This is a followup to #341

This change means that some customers will lose visibility into mongodb calls where the async/reactive mongo drivers were being erroneously instrumented.

It also resolves an issue where both the mongodb-3.1 and mongodb-3.7 could apply.

Issue #198 is intended to add instrumentation for the async/reactive mongo drivers.


Tested with an app that was using a few different versions of the reactive client:

  • mongodb-driver-reactivestreams:1.12.0 with mongodb-driver-core:3.11.2
  • mongodb-driver-reactivestreams:4.2.3 with mongodb-driver-core:4.2.3

As expected none of the mongodb instrumentation modules loaded:

Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-2.12/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-2.14/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-3.0/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-3.1/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-3.7/1

Tested with an app that was using the sync client:

  • mongodb-driver-sync:4.1.1 with mongodb-driver-core:4.1.1

As expected, only the mongodb-3.7 instrumentation loaded:

Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-2.12/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-2.14/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-3.0/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-3.1/1

Supportability/WeaveInstrumentation/Loaded/com.newrelic.instrumentation.mongodb-3.7/1
Supportability/WeaveInstrumentation/WeaveClass/com.newrelic.instrumentation.mongodb-3.7/com/mongodb/MongoClientSettings$Builder
Supportability/WeaveInstrumentation/WeaveClass/com.newrelic.instrumentation.mongodb-3.7/com/mongodb/MongoClientSettings

Tested with an app that was using the sync client:

  • mongodb-driver-sync:3.6.3 with mongodb-driver-core:3.6.3

As expected, only the mongodb-3.1 instrumentation loaded:

Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-2.12/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-2.14/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-3.0/1
Supportability/WeaveInstrumentation/Skipped/com.newrelic.instrumentation.mongodb-3.7/1

Supportability/WeaveInstrumentation/Loaded/com.newrelic.instrumentation.mongodb-3.1/1
Supportability/WeaveInstrumentation/WeaveClass/com.newrelic.instrumentation.mongodb-3.1/com/mongodb/MongoClientOptions
Supportability/WeaveInstrumentation/WeaveClass/com.newrelic.instrumentation.mongodb-3.1/com/mongodb/MongoClientOptions$Builder

AITs pass: https://javaagent-build.pdx.vm.datanerd.us/job/AIT_Configurable/127/

Instrumentation tests pass for all three modified instrumentation modules.

Copy link
Contributor

@tbradellis tbradellis left a comment

Choose a reason for hiding this comment

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

lgtm
AND...All checks have passed (so pretty). 👍 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants