Skip to content

Add operation and collection attributes for MongoDB spans#1398

Merged
iNikem merged 7 commits into
open-telemetry:masterfrom
jamal:db-mongo-attributes
Oct 19, 2020
Merged

Add operation and collection attributes for MongoDB spans#1398
iNikem merged 7 commits into
open-telemetry:masterfrom
jamal:db-mongo-attributes

Conversation

@jamal

@jamal jamal commented Oct 15, 2020

Copy link
Copy Markdown
Contributor

This change introduces db.operation and db.mongodb.collection attributes to the mongo client instrumentation. Because there is no common functionality for db.operation in DatabaseClientTracer we are just overriding onConnect to add mongo specific attributes.

In the future, this should be refactored when db.operation is added across all db instrumetnation.

Fixes #1278

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @jamal!

Looks like CI is failing due to formatting, if you run ./gradlew spotlessApply that will fix it up.

Comment on lines +62 to +66
mongoSpan(it, 0, {
assert it.replaceAll(" ", "") == "{\"create\":\"$collectionName\",\"capped\":\"?\"}" ||
it == "{\"create\": \"$collectionName\", \"capped\": \"?\", \"\$db\": \"?\", \"\$readPreference\": {\"mode\": \"?\"}}"
true
}
}, "create", collectionName)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if we swap args moving operation and collection before the Closure, then could still use the groovy closure syntax

@jamal

jamal commented Oct 17, 2020

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @trask! I've updated the tests based on your recommendation (updated the other ones as well to be consistent). I don't really understand what happened with spotlessApply. When I ran it in Windows it didn't find any issues, but running it on Linux worked. Hopefully this should do it but I'll keep an eye on the build status and will fix any more issues if they come up. Thanks so much!

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @jamal 👍

@iNikem iNikem merged commit 69ea2f7 into open-telemetry:master Oct 19, 2020
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.

Implement db.mongodb.collection semantic attribute

3 participants