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

Add operation and collection attributes for MongoDB spans #1398

Merged
merged 7 commits into from
Oct 19, 2020

Conversation

jamal
Copy link
Contributor

@jamal jamal commented Oct 15, 2020

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

Copy link
Member

@trask trask left a comment

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
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
Copy link
Contributor Author

jamal commented Oct 17, 2020

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!

Copy link
Member

@trask trask left a comment

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