Revert "contrib/bradfitz/gomemcache: trace item info for memcached op…

…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.
DataDog · May 18, 2020 · 3999161 · 3999161
1 parent 28e8330
commit 3999161
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 52 deletions.
diff --git a/contrib/bradfitz/gomemcache/memcache/memcache.go b/contrib/bradfitz/gomemcache/memcache/memcache.go
@@ -80,11 +80,6 @@ func (c *Client) startSpan(resourceName string) ddtrace.Span {
 func (c *Client) Add(item *memcache.Item) error {
 	span := c.startSpan("Add")
 	err := c.Client.Add(item)
-	span.SetTag("item.key", item.Key)
-	if c.cfg.withValueTags {
-		span.SetTag("item.value", item.Value)
-	}
-	span.SetTag("item.expiration", item.Expiration)
 	span.Finish(tracer.WithError(err))
 	return err
 }
@@ -93,11 +88,6 @@ func (c *Client) Add(item *memcache.Item) error {
 func (c *Client) CompareAndSwap(item *memcache.Item) error {
 	span := c.startSpan("CompareAndSwap")
 	err := c.Client.CompareAndSwap(item)
-	span.SetTag("item.key", item.Key)
-	if c.cfg.withValueTags {
-		span.SetTag("item.value", item.Value)
-	}
-	span.SetTag("item.expiration", item.Expiration)
 	span.Finish(tracer.WithError(err))
 	return err
 }
@@ -106,11 +96,6 @@ func (c *Client) CompareAndSwap(item *memcache.Item) error {
 func (c *Client) Decrement(key string, delta uint64) (newValue uint64, err error) {
 	span := c.startSpan("Decrement")
 	newValue, err = c.Client.Decrement(key, delta)
-	span.SetTag("item.key", key)
-	if c.cfg.withValueTags {
-		span.SetTag("item.value.before", newValue-delta)
-		span.SetTag("item.value.after", newValue)
-	}
 	span.Finish(tracer.WithError(err))
 	return newValue, err
 }
@@ -119,7 +104,6 @@ func (c *Client) Decrement(key string, delta uint64) (newValue uint64, err error
 func (c *Client) Delete(key string) error {
 	span := c.startSpan("Delete")
 	err := c.Client.Delete(key)
-	span.SetTag("item.key", key)
 	span.Finish(tracer.WithError(err))
 	return err
 }
@@ -144,7 +128,6 @@ func (c *Client) FlushAll() error {
 func (c *Client) Get(key string) (item *memcache.Item, err error) {
 	span := c.startSpan("Get")
 	item, err = c.Client.Get(key)
-	span.SetTag("item.key", key)
 	span.Finish(tracer.WithError(err))
 	return item, err
 }
@@ -153,7 +136,6 @@ func (c *Client) Get(key string) (item *memcache.Item, err error) {
 func (c *Client) GetMulti(keys []string) (map[string]*memcache.Item, error) {
 	span := c.startSpan("GetMulti")
 	items, err := c.Client.GetMulti(keys)
-	span.SetTag("item.keys", keys)
 	span.Finish(tracer.WithError(err))
 	return items, err
 }
@@ -162,11 +144,6 @@ func (c *Client) GetMulti(keys []string) (map[string]*memcache.Item, error) {
 func (c *Client) Increment(key string, delta uint64) (newValue uint64, err error) {
 	span := c.startSpan("Increment")
 	newValue, err = c.Client.Increment(key, delta)
-	span.SetTag("item.key", key)
-	if c.cfg.withValueTags {
-		span.SetTag("item.value.before", newValue-delta)
-		span.SetTag("item.value.after", newValue)
-	}
 	span.Finish(tracer.WithError(err))
 	return newValue, err
 }
@@ -175,11 +152,6 @@ func (c *Client) Increment(key string, delta uint64) (newValue uint64, err error
 func (c *Client) Replace(item *memcache.Item) error {
 	span := c.startSpan("Replace")
 	err := c.Client.Replace(item)
-	span.SetTag("item.key", item.Key)
-	if c.cfg.withValueTags {
-		span.SetTag("item.value", item.Value)
-	}
-	span.SetTag("item.expiration", item.Expiration)
 	span.Finish(tracer.WithError(err))
 	return err
 }
@@ -188,11 +160,6 @@ func (c *Client) Replace(item *memcache.Item) error {
 func (c *Client) Set(item *memcache.Item) error {
 	span := c.startSpan("Set")
 	err := c.Client.Set(item)
-	span.SetTag("item.key", item.Key)
-	if c.cfg.withValueTags {
-		span.SetTag("item.value", item.Value)
-	}
-	span.SetTag("item.expiration", item.Expiration)
 	span.Finish(tracer.WithError(err))
 	return err
 }
@@ -201,8 +168,6 @@ func (c *Client) Set(item *memcache.Item) error {
 func (c *Client) Touch(key string, seconds int32) error {
 	span := c.startSpan("Touch")
 	err := c.Client.Touch(key, seconds)
-	span.SetTag("item.key", key)
-	span.SetTag("item.expiration", seconds)
 	span.Finish(tracer.WithError(err))
 	return err
 }
diff --git a/contrib/bradfitz/gomemcache/memcache/memcache_test.go b/contrib/bradfitz/gomemcache/memcache/memcache_test.go
@@ -38,7 +38,7 @@ func TestMemcacheIntegration(t *testing.T) {
 }
 
 func testMemcache(t *testing.T, addr string) {
-	client := WrapClient(memcache.New(addr), WithServiceName("test-memcache"), WithValueTags())
+	client := WrapClient(memcache.New(addr), WithServiceName("test-memcache"))
 	defer client.DeleteAll()
 
 	validateMemcacheSpan := func(t *testing.T, span mocktracer.Span, resourceName string) {
@@ -76,9 +76,8 @@ func testMemcache(t *testing.T, addr string) {
 		err := client.
 			WithContext(ctx).
 			Add(&memcache.Item{
-				Key:        "key2",
-				Value:      []byte("value2"),
-				Expiration: 10,
+				Key:   "key2",
+				Value: []byte("value2"),
 			})
 		assert.Nil(t, err)
 
@@ -90,9 +89,6 @@ func testMemcache(t *testing.T, addr string) {
 		assert.Equal(t, span, spans[1])
 		assert.Equal(t, spans[1].TraceID(), spans[0].TraceID(),
 			"memcache span should be part of the parent trace")
-		assert.Equal(t, "key2", spans[0].Tag("item.key"))
-		assert.Equal(t, []byte("value2"), spans[0].Tag("item.value"))
-		assert.Equal(t, int32(10), spans[0].Tag("item.expiration"))
 	})
 }
 

diff --git a/contrib/bradfitz/gomemcache/memcache/option.go b/contrib/bradfitz/gomemcache/memcache/option.go
@@ -17,7 +17,6 @@ const (
 type clientConfig struct {
 	serviceName   string
 	analyticsRate float64
-	withValueTags bool
 }
 
 // ClientOption represents an option that can be passed to Dial.
@@ -58,11 +57,3 @@ func WithAnalyticsRate(rate float64) ClientOption {
 		}
 	}
 }
-
-// WithValueTags specifies whether values assigned to keys in memcache operations
-// should be added to spans as tags.
-func WithValueTags() ClientOption {
-	return func(cfg *clientConfig) {
-		cfg.withValueTags = true
-	}
-}
diff --git a/internal/version/version.go b/internal/version/version.go
@@ -8,4 +8,4 @@ package version
 // Tag specifies the current release tag. It needs to be manually
 // updated. A test checks that the value of Tag never points to a
 // git tag that is older than HEAD.
-const Tag = "v1.24.0"
+const Tag = "v1.24.1"