Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(mongodb4): added mongodb4 instrumentation #794
feat(mongodb4): added mongodb4 instrumentation #794
Changes from 17 commits
af49bac
6de0a73
28dd80f
7a2734b
af247e4
f171e3d
87efbc6
af3f981
7262944
8333e54
177338f
33a7294
d4ea2c6
009f16f
b1d7aad
652ae27
a65c4b3
6dbc6d5
ca4d020
fe9bb27
196d77e
4fa8252
a0ab0a4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change means we are running PullRequests CI tests only for v4, and stop running v3 tests on PRs.
opentelemetry-js-contrib/package.json
Line 23 in 81b3190
something we might want to consider...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct. I can't think about any other option. Suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a particularly good one
We can run
tav
here as well, possibly with just 2 versions. But that sounds clumsy and complex.Let's wait for more minds on this one @rauno56 @dyladan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, The tests are only can run locally (we need mongodb server). So anyway i dont think i would change much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I usually spin up a mongo server locally with docker and run the tests against it.
CI tests also start a mongo server here, so the tests can pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We talked about this in the SIG. We recommend to test the most popular major version (in this case v3) and test other versions with TAV.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are installing mongodb v4 as dev dependency, which means that v3 tests will not pass, only in tav, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about documenting it somehow?
Maybe we can write a comment at the top of the v3 tests file with instructions on how to run them locally.
The next person who will try to run v3 test will see all tests failing and be surprised
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added doc :)