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

MongoDB driver upgrade #86

Closed

Conversation

pyrotechnics-io
Copy link

Upgrade to the official MongoDB driver

@CLAassistant
Copy link

CLAassistant commented Jul 21, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ hbaste
❌ Harsh


Harsh seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@paologallinaharbur
Copy link
Member

Thanks!

Just for context @pyrotechnics-io was testing out a poc to replace the driver, he solved many issue we are going to face following this path. The PR it is to be intended as a diff and not as if it was ready to be merged

@pyrotechnics-io
Copy link
Author

pyrotechnics-io commented Jul 21, 2022

Thanks!

Just for context @pyrotechnics-io was testing out a poc to replace the driver, he solved many issue we are going to face following this path. The PR it is to be intended as a diff and not as if it was ready to be merged

Agreed - this code isnt meant to be merged but to perhaps help with the final replacement.
Some significant aspects about this diff are:

  1. There were a lot of unit tests referring to mgo/globalsign. Some of them aren't relevant to the new driver but some are. All of them refer to FakeSession which will need to be replaced for use with the official driver/session.
  2. The approach followed here was to try and keep the calling code (to the driver) within the collectors as untouched as possible. It also intended to keep the binary command line options similarly untouched for backward compatibility. What this required was to make a facade on the new driver to make it look like global/mgo. The intention was, this allows us to replace mgo first, put in unit/integration tests on the new driver, and then eventually refactor/remove the facade and adapt the calling code as well. But so far only the mgo driver has been replaced and the unit/integration tests have yet to be adapted. The eventual refactoring is still further afar.
  3. This fork above in theory should address Add support for SCRAM-SHA-256 authentication #67, MongoDB Atlas #70, Move to the official mongodb client #71, Can't connect to Mongodb 5.x #76, Fail to connect to Mongos on Sharded cluster in K8s #78. However, in part because of Fail to connect to Mongos on Sharded cluster in K8s #78, the new agent produces significantly more output than before in the integration test. If the additional data is because Fail to connect to Mongos on Sharded cluster in K8s #78 was fixed and/or is producing data that isn't useful to us in general, is something that will need to be looked at.

@carlossscastro
Copy link
Contributor

This PR has been closed as NR is working on a new version of the MongoDb Integration

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.

5 participants