Disable memcached command tag by default

docs/GettingStarted.md

@@ -659,6 +659,7 @@ client.set('abc', 123)
 
 | Key            | Env Var                       | Description                                                                                                                                                                               | Default     |
 |----------------|-------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------------|
+| `command_enabled` | `DD_TRACE_MEMCACHED_COMMAND_ENABLED` | Collect commands as the `memcached.command` tag. Command `keys` can potentially contain sensitive information. | `false` |
 | `service_name` | `DD_TRACE_DALLI_SERVICE_NAME` | Name of application running the `dalli` instrumentation. May be overridden by `global_default_service_name`. [See *Additional Configuration* for more details](#additional-configuration) | `memcached` |
 | `peer_service` | `DD_TRACE_DALLI_PEER_SERVICE` | Name of external service the application connects to                                                                                                                                      | `nil`       |
 

lib/datadog/tracing/contrib/dalli/configuration/settings.rb

@@ -42,6 +42,12 @@ class Settings < Contrib::Configuration::Settings
               o.type :string, nilable: true
               o.env Ext::ENV_PEER_SERVICE
             end
+
+            option :command_enabled do |o|
+              o.type :bool
+              o.env Ext::ENV_COMMAND_ENABLED
+              o.default false
+            end
           end
         end
       end

lib/datadog/tracing/contrib/dalli/ext.rb

@@ -7,9 +7,16 @@ module Dalli
         # Dalli integration constants
         # @public_api Changing resource names, tag names, or environment variables creates breaking changes.
         module Ext
+          # DEV: Even though this is the dalli integration, all spans are named `memcached.*`.
+          # DEV: This happens because such spans have special treatment in backend, with memcached-specific handling.
+          # DEV: If add support for the `memcached` gem (not popular as of 2023), we'll have issues with span naming
+          # DEV: conflicts.
           ENV_ENABLED = 'DD_TRACE_DALLI_ENABLED'
           ENV_ANALYTICS_ENABLED = 'DD_TRACE_DALLI_ANALYTICS_ENABLED'
           ENV_ANALYTICS_SAMPLE_RATE = 'DD_TRACE_DALLI_ANALYTICS_SAMPLE_RATE'
+          # DEV: This is named `*_MEMCACHED_*` because the spans it refer to are `memcached.*` and this variable
+          # DEV: is a global flag that affects all memcached spans generated by any Datadog product.
+          ENV_COMMAND_ENABLED = 'DD_TRACE_MEMCACHED_COMMAND_ENABLED'
           ENV_SERVICE_NAME = 'DD_TRACE_DALLI_SERVICE_NAME'
           ENV_PEER_SERVICE = 'DD_TRACE_DALLI_PEER_SERVICE'
 

lib/datadog/tracing/contrib/dalli/instrumentation.rb

@@ -50,8 +50,10 @@ def request(op, *args)
 
                 span.set_tag(Contrib::Ext::DB::TAG_SYSTEM, Ext::TAG_SYSTEM)
 
-                cmd = Quantize.format_command(op, args)
-                span.set_tag(Ext::TAG_COMMAND, cmd)
+                if datadog_configuration[:command_enabled]
+                  cmd = Quantize.format_command(op, args)
+                  span.set_tag(Ext::TAG_COMMAND, cmd)
+                end
 
                 Contrib::SpanAttributeSchema.set_peer_service!(span, Ext::PEER_SERVICE_SOURCES)
                 super

sig/datadog/tracing/contrib/dalli/ext.rbs

@@ -3,6 +3,8 @@ module Datadog
     module Contrib
       module Dalli
         module Ext
+          ENV_COMMAND_ENABLED: ::String
+
           ENV_ENABLED: "DD_TRACE_DALLI_ENABLED"
 
           ENV_ANALYTICS_ENABLED: "DD_TRACE_DALLI_ANALYTICS_ENABLED"

spec/datadog/tracing/contrib/dalli/instrumentation_spec.rb

@@ -70,7 +70,7 @@
       expect(span.name).to eq('memcached.command')
       expect(span.span_type).to eq('memcached')
       expect(span.resource).to eq('SET')
-      expect(span.get_tag('memcached.command')).to eq('set abc 123 0 0')
+      expect(span).to_not have_tag('memcached.command')
       expect(span.get_tag('out.host')).to eq(test_host)
       expect(span.get_tag('out.port')).to eq(test_port.to_f)
       expect(span.get_tag('db.system')).to eq('memcached')
@@ -79,6 +79,11 @@
       expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION)).to eq('command')
     end
 
+    context 'with command tag captured enabled' do
+      let(:configuration_options) { { command_enabled: true } }
+      it { expect(span.get_tag('memcached.command')).to eq('set abc 123 0 0') }
+    end
+
     it_behaves_like 'a peer service span' do
       let(:peer_service_val) { ENV.fetch('TEST_MEMCACHED_HOST', '127.0.0.1') }
       let(:peer_service_source) { 'peer.hostname' }
@@ -87,12 +92,7 @@
 
   describe 'when multiplexed configuration is provided' do
     let(:service_name) { 'multiplex-service' }
-
-    before do
-      Datadog.configure do |c|
-        c.tracing.instrument :dalli, describes: "#{test_host}:#{test_port}", service_name: service_name
-      end
-    end
+    let(:configuration_options) { { describes: "#{test_host}:#{test_port}", service_name: service_name } }
 
     context 'and #set is called' do
       before do
@@ -106,7 +106,7 @@
         expect(span.name).to eq('memcached.command')
         expect(span.span_type).to eq('memcached')
         expect(span.resource).to eq('SET')
-        expect(span.get_tag('memcached.command')).to eq('set abc 123 0 0')
+        expect(span).to_not have_tag('memcached.command')
         expect(span.get_tag('out.host')).to eq(test_host)
         expect(span.get_tag('out.port')).to eq(test_port.to_f)
 
@@ -116,6 +116,11 @@
         expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION)).to eq('command')
       end
 
+      context 'with command tag captured enabled' do
+        let(:configuration_options) { super().merge(command_enabled: true) }
+        it { expect(span.get_tag('memcached.command')).to eq('set abc 123 0 0') }
+      end
+
       it_behaves_like 'a peer service span' do
         let(:peer_service_val) { ENV.fetch('TEST_MEMCACHED_HOST', '127.0.0.1') }
         let(:peer_service_source) { 'peer.hostname' }