-
Notifications
You must be signed in to change notification settings - Fork 623
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: adding instrumentation support for mongo-driver/v2 #6539
base: main
Are you sure you want to change the base?
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6539 +/- ##
======================================
Coverage 75.7% 75.8%
======================================
Files 207 210 +3
Lines 19403 19521 +118
======================================
+ Hits 14695 14800 +105
- Misses 4273 4282 +9
- Partials 435 439 +4
🚀 New features to boost your workflow:
|
cc @prestonvasquez for review? |
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.
@AlaricWhitney Thank you for putting this together. I wanted to let you know that I'll be reviewing this in stages, as my schedule allows.
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/doc.go
Outdated
Show resolved
Hide resolved
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/doc.go
Outdated
Show resolved
Hide resolved
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/doc.go
Outdated
Show resolved
Hide resolved
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/version.go
Outdated
Show resolved
Hide resolved
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/version.go
Show resolved
Hide resolved
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/test/version_test.go
Outdated
Show resolved
Hide resolved
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/test/doc.go
Outdated
Show resolved
Hide resolved
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/version.go
Outdated
Show resolved
Hide resolved
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/test/version.go
Outdated
Show resolved
Hide resolved
can this pr merge? I need use it in my project |
Not until all review comments have been closed an it has been approved. |
…/doc.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/doc.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/doc.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/version.go Co-authored-by: Damien Mathieu <42@dmathieu.com>
…/version.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…tation for go.mongodb.org/mongo-driver/v2
…/example_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/mongo_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/mongo_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/mongo_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/mongo_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/mongo_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/mongo_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/test/mongo_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
…/example_test.go Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
@dmathieu Is there anyone particular I should add as the codeowner of this? I'm not 100% sure what the appropriate process is. |
@dmathieu @prestonvasquez This should be ready for review again. For the |
Keep it blank for now. Though we should probably add @prestonvasquez, if he's ok with it. |
@dmathieu @AlaricWhitney SGTM, thanks! |
|
||
// Version is the current release version of the mongo-go-driver V2 instrumentation. | ||
func Version() string { | ||
return "0.60.0" |
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.
cc @MrAlias
This kinds of goes in the same way as this comment: #6961 (comment)
We should therefore adopt the same behavior. So let's keep commit hashes rather than releases for now (you just need to remove the entries in versions.yaml
).
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.
Sounds good. I'll remove the entries from versions.yaml. I'm unsure what needs to be done about the commit hashes, as I don't see any action taken yet with the referenced PR.
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.
You should still add it to excluded modules in versions.yaml
, so it doesn't get released. But it gets dependency upgrades on a new release.
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.
got it. I added it to the excluded modules.
…rg/mongo-driver/v2/mongo/otelmongo exclusion
Adding the mongo-driver/v2 instrumentation.
This is a copy/paste of the v1 instrumentation, and was modified to adhere to the mongo-driver/v2 requirements.
Notable differences:
mtest
was removed in v2. Replaced withdrivertest
in the unit testing.This addresses PR #6419