Add config to disable obfuscation in memcached queries (#2725)

pablomartinezbernardo · web-flow · commit 231d5bd0d184 · 2024-06-21T12:38:53.000+02:00
Add config to disable obfuscation in memcached queries
diff --git a/ext/configuration.h b/ext/configuration.h
@@ -184,6 +184,7 @@ enum ddtrace_sampling_rules_format {
     CONFIG(STRING, DD_VERSION, "", .ini_change = ddtrace_alter_dd_version,                                     \
            .env_config_fallback = ddtrace_conf_otel_resource_attributes_version)                               \
     CONFIG(STRING, DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP, DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP_DEFAULT) \
+    CONFIG(BOOL, DD_TRACE_MEMCACHED_OBFUSCATION, "true")                                                       \
     CONFIG(BOOL, DD_TRACE_CLIENT_IP_ENABLED, "false")                                                          \
     CONFIG(CUSTOM(STRING), DD_TRACE_CLIENT_IP_HEADER, "", .parser = ddtrace_parse_client_ip_header_config)     \
     CONFIG(BOOL, DD_TRACE_FORKED_PROCESS, "true")                                                              \
diff --git a/src/DDTrace/Integrations/Memcache/MemcacheIntegration.php b/src/DDTrace/Integrations/Memcache/MemcacheIntegration.php
@@ -97,7 +97,9 @@ public function traceCommand($command)
             }
             if (!is_array($args[0])) {
                 $integration->setServerTags($span, $this);
-                $span->meta['memcache.query'] = $command . ' ' . Obfuscation::toObfuscatedString($args[0]);
+                $queryParams = dd_trace_env_config("DD_TRACE_MEMCACHED_OBFUSCATION") ?
+                    Obfuscation::toObfuscatedString($args[0]) : $args[0];
+                $span->meta['memcache.query'] = $command . ' ' . $queryParams;
             }
             $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;
             $integration->markForTraceAnalytics($span, $command);
diff --git a/src/DDTrace/Integrations/Memcached/MemcachedIntegration.php b/src/DDTrace/Integrations/Memcached/MemcachedIntegration.php
@@ -125,7 +125,7 @@ function (SpanData $span, $args, $retval) use ($integration, $command) {
                 }
                 if (!is_array($args[0])) {
                     $integration->setServerTags($span, $this);
-                    $span->meta['memcached.query'] = $command . ' ' . Obfuscation::toObfuscatedString($args[0]);
+                    $span->meta['memcached.query'] = $command . ' ' . $integration->obfuscateIfNeeded($args[0]);
                 }
                 $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;
 
@@ -147,7 +147,7 @@ function (SpanData $span, $args, $retval) use ($integration, $command) {
                 }
                 if (!is_array($args[0])) {
                     $integration->setServerTags($span, $this);
-                    $span->meta['memcached.query'] = $command . ' ' . Obfuscation::toObfuscatedString($args[0]);
+                    $span->meta['memcached.query'] = $command . ' ' . $integration->obfuscateIfNeeded($args[0]);
                     $span->meta['memcached.server_key'] = $args[0];
                 }
                 $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;
@@ -169,7 +169,7 @@ function (SpanData $span, $args, $retval) use ($integration, $command) {
                     $span->metrics[Tag::DB_ROW_COUNT] = isset($retval) ? (is_array($retval) ? count($retval) : 1) : 0;
                 }
                 $integration->setServerTags($span, $this);
-                $span->meta['memcached.query'] = $command . ' ' . Obfuscation::toObfuscatedString($args[0], ',');
+                $span->meta['memcached.query'] = $command . ' ' . $integration->obfuscateIfNeeded($args[0], ',');
                 $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;
                 $integration->markForTraceAnalytics($span, $command);
             }
@@ -189,7 +189,7 @@ function (SpanData $span, $args, $retval) use ($integration, $command) {
                 }
                 $span->meta['memcached.server_key'] = $args[0];
                 $integration->setServerTags($span, $this);
-                $query = "$command " . Obfuscation::toObfuscatedString($args[1], ',');
+                $query = "$command " . $integration->obfuscateIfNeeded($args[1], ',');
                 $span->meta['memcached.query'] = $query;
                 $span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;
                 $integration->markForTraceAnalytics($span, $command);
@@ -263,4 +263,18 @@ public function markForTraceAnalytics(SpanData $span, $command)
             $this->addTraceAnalyticsIfEnabled($span);
         }
     }
+
+    /*
+     * Return either the obfuscated params or the params themselves, depending on the env var.
+     */
+    public function obfuscateIfNeeded($params, $glue = ' ')
+    {
+        if (dd_trace_env_config("DD_TRACE_MEMCACHED_OBFUSCATION")) {
+            return Obfuscation::toObfuscatedString($params, $glue);
+        } elseif (is_array($params)) {
+            return implode($glue, $params);
+        } else {
+            return $params;
+        }
+    }
 }
diff --git a/tests/Integrations/Memcache/MemcacheTest.php b/tests/Integrations/Memcache/MemcacheTest.php
@@ -37,6 +37,7 @@ protected function envsToCleanUpAtTearDown()
             'DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED',
             'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED',
             'DD_SERVICE',
+            'DD_TRACE_MEMCACHED_OBFUSCATION',
         ];
     }
 
@@ -55,6 +56,22 @@ public function testAdd()
         ]);
     }
 
+    public function testAddNoObfuscation()
+    {
+        $this->putEnvAndReloadConfig(['DD_TRACE_MEMCACHED_OBFUSCATION=false']);
+        $traces = $this->isolateTracer(function () {
+            $this->client->add('key', 'value');
+        });
+        $this->assertSpans($traces, [
+            SpanAssertion::build('Memcache.add', 'memcache', 'memcached', 'add')
+                ->withExactTags(array_merge(self::baseTags(), [
+                    'memcache.query' => 'add ' . 'key',
+                    'memcache.command' => 'add',
+                    Tag::SPAN_KIND => 'client',
+                ]))
+        ]);
+    }
+
     public function testAppend()
     {
         $traces = $this->isolateTracer(function () {
diff --git a/tests/Integrations/Memcached/MemcachedTest.php b/tests/Integrations/Memcached/MemcachedTest.php
@@ -36,6 +36,7 @@ protected function envsToCleanUpAtTearDown()
             'DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED',
             'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED',
             'DD_SERVICE',
+            'DD_TRACE_MEMCACHED_OBFUSCATION',
         ];
     }
 
@@ -530,6 +531,30 @@ public function testGetMulti()
         ]);
     }
 
+    public function testGetMultiNoObfuscation()
+    {
+        $this->putEnvAndReloadConfig(['DD_TRACE_MEMCACHED_OBFUSCATION=false']);
+        $traces = $this->isolateTracer(function () {
+            $this->client->add('key1', 'value1');
+            $this->client->add('key2', 'value2');
+
+            $this->assertEquals(['key1' => 'value1', 'key2' => 'value2'], $this->client->getMulti(['key1', 'key2']));
+        });
+        $this->assertSpans($traces, [
+            SpanAssertion::exists('Memcached.add'),
+            SpanAssertion::exists('Memcached.add'),
+            SpanAssertion::build('Memcached.getMulti', 'memcached', 'memcached', 'getMulti')
+                ->withExactTags(array_merge($this->baseTags(), [
+                    'memcached.query' => 'getMulti key1,key2',
+                    'memcached.command' => 'getMulti',
+                ]))->withExactMetrics([
+                    Tag::DB_ROW_COUNT => 2,
+                    '_dd.agent_psr' => 1.0,
+                    '_sampling_priority_v1' => 1.0,
+                ]),
+        ]);
+    }
+
     public function testGetMultiNotAllExist()
     {
         $traces = $this->isolateTracer(function () {

Original file line number	Diff line number	Diff line change
`@@ -97,7 +97,9 @@ public function traceCommand($command)`
`97`	`97`	`}`
`98`	`98`	`if (!is_array($args[0])) {`
`99`	`99`	`$integration->setServerTags($span, $this);`
`100`		`- $span->meta['memcache.query'] = $command . ' ' . Obfuscation::toObfuscatedString($args[0]);`
	`100`	`+ $queryParams = dd_trace_env_config("DD_TRACE_MEMCACHED_OBFUSCATION") ?`
	`101`	`+ Obfuscation::toObfuscatedString($args[0]) : $args[0];`
	`102`	`+ $span->meta['memcache.query'] = $command . ' ' . $queryParams;`
`101`	`103`	`}`
`102`	`104`	`$span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;`
`103`	`105`	`$integration->markForTraceAnalytics($span, $command);`
Original file line number	Diff line number	Diff line change
`@@ -125,7 +125,7 @@ function (SpanData $span, $args, $retval) use ($integration, $command) {`
`125`	`125`	`}`
`126`	`126`	`if (!is_array($args[0])) {`
`127`	`127`	`$integration->setServerTags($span, $this);`
`128`		`- $span->meta['memcached.query'] = $command . ' ' . Obfuscation::toObfuscatedString($args[0]);`
	`128`	`+ $span->meta['memcached.query'] = $command . ' ' . $integration->obfuscateIfNeeded($args[0]);`
`129`	`129`	`}`
`130`	`130`	`$span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;`
`131`	`131`
`@@ -147,7 +147,7 @@ function (SpanData $span, $args, $retval) use ($integration, $command) {`
`147`	`147`	`}`
`148`	`148`	`if (!is_array($args[0])) {`
`149`	`149`	`$integration->setServerTags($span, $this);`
`150`		`- $span->meta['memcached.query'] = $command . ' ' . Obfuscation::toObfuscatedString($args[0]);`
	`150`	`+ $span->meta['memcached.query'] = $command . ' ' . $integration->obfuscateIfNeeded($args[0]);`
`151`	`151`	`$span->meta['memcached.server_key'] = $args[0];`
`152`	`152`	`}`
`153`	`153`	`$span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;`
`@@ -169,7 +169,7 @@ function (SpanData $span, $args, $retval) use ($integration, $command) {`
`169`	`169`	`$span->metrics[Tag::DB_ROW_COUNT] = isset($retval) ? (is_array($retval) ? count($retval) : 1) : 0;`
`170`	`170`	`}`
`171`	`171`	`$integration->setServerTags($span, $this);`
`172`		`- $span->meta['memcached.query'] = $command . ' ' . Obfuscation::toObfuscatedString($args[0], ',');`
	`172`	`+ $span->meta['memcached.query'] = $command . ' ' . $integration->obfuscateIfNeeded($args[0], ',');`
`173`	`173`	`$span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;`
`174`	`174`	`$integration->markForTraceAnalytics($span, $command);`
`175`	`175`	`}`
`@@ -189,7 +189,7 @@ function (SpanData $span, $args, $retval) use ($integration, $command) {`
`189`	`189`	`}`
`190`	`190`	`$span->meta['memcached.server_key'] = $args[0];`
`191`	`191`	`$integration->setServerTags($span, $this);`
`192`		`- $query = "$command " . Obfuscation::toObfuscatedString($args[1], ',');`
	`192`	`+ $query = "$command " . $integration->obfuscateIfNeeded($args[1], ',');`
`193`	`193`	`$span->meta['memcached.query'] = $query;`
`194`	`194`	`$span->peerServiceSources = DatabaseIntegrationHelper::PEER_SERVICE_SOURCES;`
`195`	`195`	`$integration->markForTraceAnalytics($span, $command);`
`@@ -263,4 +263,18 @@ public function markForTraceAnalytics(SpanData $span, $command)`
`263`	`263`	`$this->addTraceAnalyticsIfEnabled($span);`
`264`	`264`	`}`
`265`	`265`	`}`
	`266`	`+`
	`267`	`+ /*`
	`268`	`+ * Return either the obfuscated params or the params themselves, depending on the env var.`
	`269`	`+ */`
	`270`	`+ public function obfuscateIfNeeded($params, $glue = ' ')`
	`271`	`+ {`
	`272`	`+ if (dd_trace_env_config("DD_TRACE_MEMCACHED_OBFUSCATION")) {`
	`273`	`+ return Obfuscation::toObfuscatedString($params, $glue);`
	`274`	`+ } elseif (is_array($params)) {`
	`275`	`+ return implode($glue, $params);`
	`276`	`+ } else {`
	`277`	`+ return $params;`
	`278`	`+ }`
	`279`	`+ }`
`266`	`280`	`}`