Redis:Omit command arguments from span.resource by default

docs/GettingStarted.md

@@ -1641,7 +1641,7 @@ redis.set 'foo', 'bar'
 |----------------|-------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------|
 | `service_name` | `DD_TRACE_REDIS_SERVICE_NAME` | Name of application running the `redis` instrumentation. May be overridden by `global_default_service_name`. [See *Additional Configuration* for more details](#additional-configuration) | `redis` |
 | `peer_service` | `DD_TRACE_REDIS_PEER_SERVICE` | Name of external service the application connects to                                                                                                                                      | `nil`   |
-| `command_args` | `DD_REDIS_COMMAND_ARGS`       | Show the command arguments (e.g. `key` in `GET key`) as resource name and tag                                                                                                             | true    |
+| `command_args` | `DD_REDIS_COMMAND_ARGS`       | Show the command arguments (e.g. `key` in `GET key`) as resource name and tag. If `false`, only the command name is shown (e.g. `GET`). | false |
 
 
 **Configuring trace settings per instance**

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

@@ -32,7 +32,7 @@ class Settings < Contrib::Configuration::Settings
             option :command_args do |o|
               o.type :bool
               o.env Ext::ENV_COMMAND_ARGS
-              o.default true
+              o.default false
             end
 
             option :service_name do |o|

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

@@ -3,6 +3,7 @@
 require_relative 'ext'
 require_relative 'quantize'
 require_relative 'tags'
+require_relative 'trace_middleware'
 
 module Datadog
   module Tracing
@@ -17,31 +18,11 @@ def self.included(base)
           # InstanceMethods - implementing instrumentation
           module InstanceMethods
             def call(*args, &block)
-              show_command_args = command_args?
-
-              Tracing.trace(Contrib::Redis::Ext::SPAN_COMMAND) do |span|
-                span.service = service_name
-                span.span_type = Contrib::Redis::Ext::TYPE
-                span.resource = get_command(args, show_command_args)
-                Contrib::Redis::Tags.set_common_tags(self, span, show_command_args)
-
-                super
-              end
+              TraceMiddleware.call(self, args[0], service_name, command_args?) { super }
             end
 
             def call_pipeline(*args, &block)
-              show_command_args = command_args?
-
-              Tracing.trace(Contrib::Redis::Ext::SPAN_COMMAND) do |span|
-                span.service = service_name
-                span.span_type = Contrib::Redis::Ext::TYPE
-                commands = get_pipeline_commands(args, show_command_args)
-                span.resource = commands.any? ? commands.join("\n") : '(none)'
-                span.set_metric Contrib::Redis::Ext::METRIC_PIPELINE_LEN, commands.length
-                Contrib::Redis::Tags.set_common_tags(self, span, show_command_args)
-
-                super
-              end
+              TraceMiddleware.call_pipelined(self, args[0].commands, service_name, command_args?) { super }
             end
 
             private
@@ -59,22 +40,6 @@ def service_name
                 datadog_configuration[:service_name]
             end
 
-            def get_command(args, show_command_args)
-              if show_command_args
-                Contrib::Redis::Quantize.format_command_args(*args)
-              else
-                Contrib::Redis::Quantize.get_verb(*args)
-              end
-            end
-
-            def get_pipeline_commands(args, show_command_args)
-              if show_command_args
-                args[0].commands.map { |c| Contrib::Redis::Quantize.format_command_args(c) }
-              else
-                args[0].commands.map { |c| Contrib::Redis::Quantize.get_verb(c) }
-              end
-            end
-
             def datadog_configuration
               Datadog.configuration.tracing[:redis, options]
             end

lib/datadog/tracing/contrib/redis/tags.rb

@@ -12,7 +12,7 @@ module Redis
         # Tags handles generic common tags assignment.
         module Tags
           class << self
-            def set_common_tags(client, span, show_command_args)
+            def set_common_tags(client, span, raw_command)
               if datadog_configuration[:peer_service]
                 span.set_tag(
                   Tracing::Metadata::Ext::TAG_PEER_SERVICE,
@@ -42,7 +42,7 @@ def set_common_tags(client, span, show_command_args)
 
               span.set_tag Ext::TAG_DATABASE_INDEX, client.db.to_s
               span.set_tag Ext::TAG_DB, client.db
-              span.set_tag Ext::TAG_RAW_COMMAND, span.resource if show_command_args
+              span.set_tag Ext::TAG_RAW_COMMAND, raw_command
 
               Contrib::SpanAttributeSchema.set_peer_service!(span, Ext::PEER_SERVICE_SOURCES)
             end

lib/datadog/tracing/contrib/redis/trace_middleware.rb

@@ -9,55 +9,68 @@ module Contrib
       module Redis
         # Instrumentation for Redis 5+
         module TraceMiddleware
-          def call(commands, redis_config)
-            Tracing.trace(Contrib::Redis::Ext::SPAN_COMMAND) do |span|
-              datadog_configuration = resolve(redis_config)
-              resource = get_command(commands, datadog_configuration[:command_args])
+          # Instruments {RedisClient::ConnectionMixin#call}.
+          def call(command, redis_config)
+            config = resolve(redis_config)
+            TraceMiddleware.call(redis_config, command, config[:service_name], config[:command_args]) { super }
+          end
+
+          # Instruments {RedisClient::ConnectionMixin#call_pipelined}.
+          def call_pipelined(commands, redis_config)
+            config = resolve(redis_config)
+            TraceMiddleware.call_pipelined(redis_config, commands, config[:service_name], config[:command_args]) { super }
+          end
 
-              span.service = datadog_configuration[:service_name]
-              span.span_type = Contrib::Redis::Ext::TYPE
-              span.resource = resource
+          class << self
+            def call(client, command, service_name, command_args)
+              Tracing.trace(Redis::Ext::SPAN_COMMAND, type: Redis::Ext::TYPE, service: service_name) do |span|
+                raw_command = get_command(command, true)
+                span.resource = command_args ? raw_command : get_command(command, false)
 
-              Contrib::Redis::Tags.set_common_tags(redis_config, span, datadog_configuration[:command_args])
+                Contrib::Redis::Tags.set_common_tags(client, span, raw_command)
 
-              super
+                yield
+              end
             end
-          end
 
-          def call_pipelined(commands, redis_config)
-            Tracing.trace(Contrib::Redis::Ext::SPAN_COMMAND) do |span|
-              datadog_configuration = resolve(redis_config)
-              pipelined_commands = get_pipeline_commands(commands, datadog_configuration[:command_args])
+            def call_pipelined(client, commands, service_name, command_args)
+              Tracing.trace(Redis::Ext::SPAN_COMMAND, type: Redis::Ext::TYPE, service: service_name) do |span|
+                raw_command = get_pipeline_commands(commands, true)
+                span.resource = command_args ? raw_command : get_pipeline_commands(commands, false)
 
-              span.service = datadog_configuration[:service_name]
-              span.span_type = Contrib::Redis::Ext::TYPE
-              span.resource = pipelined_commands.join("\n")
-              span.set_metric Contrib::Redis::Ext::METRIC_PIPELINE_LEN, pipelined_commands.length
+                span.set_metric Contrib::Redis::Ext::METRIC_PIPELINE_LEN, commands.length
 
-              Contrib::Redis::Tags.set_common_tags(redis_config, span, datadog_configuration[:command_args])
+                Contrib::Redis::Tags.set_common_tags(client, span, raw_command)
 
-              super
+                yield
+              end
             end
-          end
 
-          private
+            private
 
-          def get_command(commands, boolean)
-            if boolean
-              Contrib::Redis::Quantize.format_command_args(commands)
-            else
-              Contrib::Redis::Quantize.get_verb(commands)
+            # Quantizes a single Redis command
+            def get_command(command, command_args)
+              if command_args
+                Contrib::Redis::Quantize.format_command_args(command)
+              else
+                Contrib::Redis::Quantize.get_verb(command)
+              end
             end
-          end
 
-          def get_pipeline_commands(commands, boolean)
-            if boolean
-              commands.map { |c| Contrib::Redis::Quantize.format_command_args(c) }
-            else
-              commands.map { |c| Contrib::Redis::Quantize.get_verb(c) }
+            # Quantizes a multi-command Redis pipeline execution
+            def get_pipeline_commands(commands, command_args)
+              list = if command_args
+                       commands.map { |c| Contrib::Redis::Quantize.format_command_args(c) }
+                     else
+                       commands.map { |c| Contrib::Redis::Quantize.get_verb(c) }
+                     end
+
+              list.empty? ? '(none)' : list.join("\n")
             end
           end
 
+          private
+
           def resolve(redis_config)
             custom = redis_config.custom[:datadog] || {}
 

spec/datadog/tracing/contrib/rails/redis_cache_spec.rb

@@ -83,7 +83,7 @@
 
       expect(redis.name).to eq('redis.command')
       expect(redis.span_type).to eq('redis')
-      expect(redis.resource).to eq('GET custom-key')
+      expect(redis.resource).to eq('GET')
       expect(redis.get_tag('redis.raw_command')).to eq('GET custom-key')
       expect(redis.service).to eq('redis')
       # the following ensures span will be correctly displayed (parent/child of the same trace)
@@ -167,7 +167,7 @@
 
       expect(redis.name).to eq('redis.command')
       expect(redis.span_type).to eq('redis')
-      expect(redis.resource).to match(/SET custom-key .*ActiveSupport.*/)
+      expect(redis.resource).to eq('SET')
       expect(redis.get_tag('redis.raw_command')).to match(/SET custom-key .*ActiveSupport.*/)
       expect(redis.service).to eq('redis')
       # the following ensures span will be correctly displayed (parent/child of the same trace)
@@ -196,7 +196,7 @@
 
       expect(del.name).to eq('redis.command')
       expect(del.span_type).to eq('redis')
-      expect(del.resource).to eq('DEL custom-key')
+      expect(del.resource).to eq('DEL')
       expect(del.get_tag('redis.raw_command')).to eq('DEL custom-key')
       expect(del.service).to eq('redis')
       # the following ensures span will be correctly displayed (parent/child of the same trace)

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

@@ -80,7 +80,7 @@
 
         # Select the designated database first
         expect(select_db_span).to be_a_redis_span.with(
-          resource: "SELECT #{test_database}",
+          resource: 'SELECT',
           service: 'multiplex-service',
           raw_command: "SELECT #{test_database}",
           host: test_host,
@@ -89,7 +89,7 @@
         )
 
         expect(span).to be_a_redis_span.with(
-          resource: 'SET abc 123',
+          resource: 'SET',
           service: 'multiplex-service',
           raw_command: 'SET abc 123',
           host: test_host,
@@ -130,7 +130,7 @@
 
         # Select the designated database first
         expect(select_db_span).to be_a_redis_span.with(
-          resource: "SELECT #{test_database}",
+          resource: 'SELECT',
           service: 'multiplex-service',
           raw_command: "SELECT #{test_database}",
           host: test_host,
@@ -139,7 +139,7 @@
         )
 
         expect(span).to be_a_redis_span.with(
-          resource: 'SET abc 123',
+          resource: 'SET',
           service: 'multiplex-service',
           raw_command: 'SET abc 123',
           host: test_host,

spec/datadog/tracing/contrib/redis/redis_spec.rb

@@ -49,22 +49,22 @@
       context 'with default settings' do
         let(:configuration_options) { {} }
 
-        it_behaves_like 'redis instrumentation', command_args: true
-        it_behaves_like 'an authenticated redis instrumentation', command_args: true
+        it_behaves_like 'redis instrumentation'
+        it_behaves_like 'an authenticated redis instrumentation'
       end
 
       context 'with service_name as `standard`' do
         let(:configuration_options) { { service_name: 'standard' } }
 
-        it_behaves_like 'redis instrumentation', service_name: 'standard', command_args: true
-        it_behaves_like 'an authenticated redis instrumentation', service_name: 'standard', command_args: true
+        it_behaves_like 'redis instrumentation', service_name: 'standard'
+        it_behaves_like 'an authenticated redis instrumentation', service_name: 'standard'
       end
 
-      context 'with command_args as `false`' do
-        let(:configuration_options) { { command_args: false } }
+      context 'with command_args as `true`' do
+        let(:configuration_options) { { command_args: true } }
 
-        it_behaves_like 'redis instrumentation'
-        it_behaves_like 'an authenticated redis instrumentation'
+        it_behaves_like 'redis instrumentation', command_args: true
+        it_behaves_like 'an authenticated redis instrumentation', command_args: true
       end
     end
 
@@ -82,23 +82,23 @@
           )
         end
 
-        it_behaves_like 'redis instrumentation', service_name: 'custom', command_args: true
-        it_behaves_like 'an authenticated redis instrumentation', service_name: 'custom', command_args: true
+        it_behaves_like 'redis instrumentation', service_name: 'custom'
+        it_behaves_like 'an authenticated redis instrumentation', service_name: 'custom'
       end
 
-      context 'with command_args as `false`' do
+      context 'with command_args as `true`' do
         let(:redis_options) do
           default_redis_options.merge(
             custom: {
               datadog: {
-                command_args: false
+                command_args: true
               }
             }
           )
         end
 
-        it_behaves_like 'redis instrumentation'
-        it_behaves_like 'an authenticated redis instrumentation'
+        it_behaves_like 'redis instrumentation', command_args: true
+        it_behaves_like 'an authenticated redis instrumentation', command_args: true
       end
     end
   end