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.

Sign up for GitHub

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

Jump to bottom

contrib/bradfitz/gomemcache: trace item info for memcached operations #642

Merged
merged 4 commits into from
Apr 22, 2020
Merged

contrib/bradfitz/gomemcache: trace item info for memcached operations #642

merged 4 commits into from
Apr 22, 2020

Conversation

mingrammer
Copy link
Contributor

Currently, the memcached tracer is just creating a span and does not
trace anything except env tag. This commit adds some tags of valuable
information for each memcached operations.

Fixes #640

@mingrammer
contrib/bradfitz/gomemcache: trace item info for memcached operations
f827d1d 
Currently, the memcached tracer is just creating a span and does not
trace anything except env tag. This commit adds some tags of valuable
information for each memcached operations.

Fixes DataDog#640
@mingrammer
Copy link
Contributor Author

mingrammer commented Apr 22, 2020 •
edited
Loading

There is a milestone issue.

gbbr
gbbr reviewed Apr 22, 2020
View reviewed changes
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

This makes sense and I think it's a great change. I do have one concern though.

contrib/bradfitz/gomemcache/memcache/memcache.go Outdated Show resolved Hide resolved
@gbbr gbbr added this to the 1.24.0 milestone Apr 22, 2020
gbbr
gbbr reviewed Apr 22, 2020
View reviewed changes
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turnaround. This is mostly perfect! Just one small request

contrib/bradfitz/gomemcache/memcache/option.go Outdated Show resolved Hide resolved
gbbr
gbbr approved these changes Apr 22, 2020
View reviewed changes
Copy link
Contributor

@gbbr gbbr left a comment

Choose a reason for hiding this comment

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

Very nice! Thank you! We’ll include this in the next release!

@mingrammer
Copy link
Contributor Author

Thank you :)

@gbbr gbbr merged commit 3f45f6d into DataDog:v1 Apr 22, 2020
@mingrammer mingrammer deleted the memcached-tags branch April 22, 2020 13:50
knusbaum added a commit that referenced this pull request May 18, 2020
@knusbaum
Revert "contrib/bradfitz/gomemcache: trace item info for memcached op…
1afe50d 
…erations (#642)"

This reverts commit 3f45f6d.
knusbaum added a commit that referenced this pull request May 18, 2020
@knusbaum
Revert "contrib/bradfitz/gomemcache: trace item info for memcached op…
adf7779 
…erations (#642)"

This reverts commit 3f45f6d.
knusbaum added a commit that referenced this pull request May 18, 2020
@knusbaum
Revert "contrib/bradfitz/gomemcache: trace item info for memcached op…
3999161 
…erations (#642)" (#663)

This reverts commit 3f45f6d.

This can be problematic for multiple reasons such as producing unexpectedly large quantities of data, and possible security issues. We do not want to provide APIs with unexpected or potentially dangerous behavior.
knusbaum added a commit that referenced this pull request May 18, 2020
@knusbaum
Revert "contrib/bradfitz/gomemcache: trace item info for memcached op…
a007850 
…erations (#642)"

This reverts commit 3f45f6d.
This was referenced May 18, 2020
Merged
Closed
knusbaum added a commit that referenced this pull request May 19, 2020
@knusbaum
Revert "contrib/bradfitz/gomemcache: trace item info for memcached op…
6c15fe8 
…erations (#642)"

This reverts commit 3f45f6d.
gbbr pushed a commit that referenced this pull request Jun 5, 2020
@knusbaum
Revert "contrib/bradfitz/gomemcache: trace item info for memcached op…
9dc9e22 
…erations (#642)" (#664)

This reverts commit 3f45f6d.
mingrammer added a commit to mingrammer/dd-trace-go that referenced this pull request Dec 22, 2020
@mingrammer
contrib/bradfitz/gomemcache: trace item info for memcached operations (…
9087972 
…DataDog#642)

This commit adds more tags to each memcached operation, along with an option to also add assignment values.

Fixes DataDog#640
mingrammer pushed a commit to mingrammer/dd-trace-go that referenced this pull request Dec 22, 2020
@knusbaum @mingrammer
Revert "contrib/bradfitz/gomemcache: trace item info for memcached op…
c091b36 
…erations (DataDog#642)" (DataDog#664)

This reverts commit 3f45f6d.
@sjmtan sjmtan mentioned this pull request Sep 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gbbr gbbr gbbr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.

contrib/bradfitz/gomemcache: trace the processing keys
2 participants
@mingrammer @gbbr