-
Notifications
You must be signed in to change notification settings - Fork 692
feat(mongo): add SpanNameResolver for customizable span naming #7986
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
base: main
Are you sure you want to change the base?
Conversation
fc23cbf
to
0dd6c01
Compare
instrumentation/go.mongodb.org/mongo-driver/v2/mongo/otelmongo/config.go
Outdated
Show resolved
Hide resolved
0dd6c01
to
e528e47
Compare
e528e47
to
807588d
Compare
This will need a changelog entry. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7986 +/- ##
=======================================
+ Coverage 78.5% 78.6% +0.1%
=======================================
Files 184 184
Lines 14673 14683 +10
=======================================
+ Hits 11520 11543 +23
+ Misses 2802 2789 -13
Partials 351 351
🚀 New features to boost your workflow:
|
I added one |
Co-authored-by: Damien Mathieu <[email protected]>
8c36929
to
8a811ee
Compare
df66b8e
to
ac9e992
Compare
@dmathieu could you please merge? :) |
We require 2 approvals to merge. |
Ping @prestonvasquez :) |
|
||
// SpanNameFormatterFunc is a function that resolves the span name given the | ||
// collection and command name. | ||
type SpanNameFormatterFunc func(collection, command string) string |
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.
I cannot be certain, currently for reference only:
Why can't this part func(collection, command string) string
be func(collection string, evt *event.CommandStartedEvent) string
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.
Another option would be to pass a formatter data struct like so:
type FormatterData struct {
*event.CommandStartedEvent
CollectionName string
}
But tbh I don't really see what added benefits that would bring, but I am also not opposed to implementing it
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.
I don't know enough about mongo to say what we need.
But with HTTP instrumentations, we pass the *http.Request
object, so folks can use anything within that struct to build the span name.
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.
@dmathieu should I change the formatter function to accept an event alongside the collection name?
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.
I still feel like a formatter data struct is more extendable
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.
@prestonvasquez is currently on leave. I have emailed his coworkers. Let's wait a bit for their input.
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.
@dmathieu Hi! Are there any updates on this?
I'd also would like to ask if there are any updates regarding this, as I am in need for this change, as well. Thx! |
This PR adds a SpanNameResolver function to the monitor options in order to customize the span name to the users needs. This is especially relevant when dealing with multiple database instances, having to prefix spans, or generally needing to customize the span name.