diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md index 3e0dfcb07f0..0cc454cb754 100644 --- a/docs/GettingStarted.md +++ b/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** diff --git a/lib/datadog/tracing/contrib/redis/configuration/settings.rb b/lib/datadog/tracing/contrib/redis/configuration/settings.rb index f05e7643a3b..ecea0b5e359 100644 --- a/lib/datadog/tracing/contrib/redis/configuration/settings.rb +++ b/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| diff --git a/lib/datadog/tracing/contrib/redis/instrumentation.rb b/lib/datadog/tracing/contrib/redis/instrumentation.rb index 6b8e8d4a018..712ce8754d6 100644 --- a/lib/datadog/tracing/contrib/redis/instrumentation.rb +++ b/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 diff --git a/lib/datadog/tracing/contrib/redis/tags.rb b/lib/datadog/tracing/contrib/redis/tags.rb index bfd0002b79f..6e8feb49029 100644 --- a/lib/datadog/tracing/contrib/redis/tags.rb +++ b/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 diff --git a/lib/datadog/tracing/contrib/redis/trace_middleware.rb b/lib/datadog/tracing/contrib/redis/trace_middleware.rb index d569e12dd89..77aa6abf351 100644 --- a/lib/datadog/tracing/contrib/redis/trace_middleware.rb +++ b/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] || {} diff --git a/spec/datadog/tracing/contrib/rails/redis_cache_spec.rb b/spec/datadog/tracing/contrib/rails/redis_cache_spec.rb index da91a1a1801..8854a0aec23 100644 --- a/spec/datadog/tracing/contrib/rails/redis_cache_spec.rb +++ b/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) diff --git a/spec/datadog/tracing/contrib/redis/instrumentation_spec.rb b/spec/datadog/tracing/contrib/redis/instrumentation_spec.rb index 7276b6c8e81..c4f14ff741a 100644 --- a/spec/datadog/tracing/contrib/redis/instrumentation_spec.rb +++ b/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, diff --git a/spec/datadog/tracing/contrib/redis/redis_spec.rb b/spec/datadog/tracing/contrib/redis/redis_spec.rb index 45c118d2740..855b308e00b 100644 --- a/spec/datadog/tracing/contrib/redis/redis_spec.rb +++ b/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 diff --git a/spec/datadog/tracing/contrib/redis/shared_examples.rb b/spec/datadog/tracing/contrib/redis/shared_examples.rb index 92dc528fc92..b66e5f6ebda 100644 --- a/spec/datadog/tracing/contrib/redis/shared_examples.rb +++ b/spec/datadog/tracing/contrib/redis/shared_examples.rb @@ -31,11 +31,10 @@ expect(span.service).to eq(options[:service_name] || 'redis') if options[:command_args] expect(span.resource).to eq('SET FOO bar') - expect(span.get_tag('redis.raw_command')).to eq('SET FOO bar') else expect(span.resource).to eq('SET') - expect(span.get_tag('redis.raw_command')).to be_nil end + expect(span.get_tag('redis.raw_command')).to eq('SET FOO bar') end it_behaves_like 'a redis span with common tags' @@ -58,11 +57,10 @@ if options[:command_args] expect(span.resource).to eq('GET FOO') - expect(span.get_tag('redis.raw_command')).to eq('GET FOO') else expect(span.resource).to eq('GET') - expect(span.get_tag('redis.raw_command')).to be_nil end + expect(span.get_tag('redis.raw_command')).to eq('GET FOO') end it_behaves_like 'a redis span with common tags' @@ -102,11 +100,10 @@ it do if options[:command_args] expect(span.resource).to eq('SET FOO bar') - expect(span.get_tag('redis.raw_command')).to eq('SET FOO bar') else expect(span.resource).to eq('SET') - expect(span.get_tag('redis.raw_command')).to be_nil end + expect(span.get_tag('redis.raw_command')).to eq('SET FOO bar') end end end @@ -147,11 +144,10 @@ expect(span.service).to eq(options[:service_name] || 'redis') if options[:command_args] expect(span.resource).to eq("SET v1 0\nSET v2 0\nINCR v1\nINCR v2\nINCR v2") - expect(span.get_tag('redis.raw_command')).to eq("SET v1 0\nSET v2 0\nINCR v1\nINCR v2\nINCR v2") else expect(span.resource).to eq("SET\nSET\nINCR\nINCR\nINCR") - expect(span.get_tag('redis.raw_command')).to be_nil end + expect(span.get_tag('redis.raw_command')).to eq("SET v1 0\nSET v2 0\nINCR v1\nINCR v2\nINCR v2") end it_behaves_like 'a redis span with common tags' @@ -192,11 +188,7 @@ expect(span.service).to eq(options[:service_name] || 'redis') expect(span.resource).to eq('(none)') - if options[:command_args] - expect(span.get_tag('redis.raw_command')).to eq('(none)') - else - expect(span.get_tag('redis.raw_command')).to be_nil - end + expect(span.get_tag('redis.raw_command')).to eq('(none)') end it_behaves_like 'a redis span with common tags' @@ -233,12 +225,11 @@ if options[:command_args] expect(span.resource).to eq('THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG') - expect(span.get_tag('redis.raw_command')).to eq('THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG') else expect(span.resource).to eq('THIS_IS_NOT_A_REDIS_FUNC') - expect(span.get_tag('redis.raw_command')).to be_nil end + expect(span.get_tag('redis.raw_command')).to eq('THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG') expect(span.status).to eq(1) expect(span.get_tag('error.message')).to match(/ERR unknown command/) expect(span.get_tag('error.type')).to match(/CommandError/) @@ -267,11 +258,10 @@ if options[:command_args] expect(span.resource).to eq("SET K #{'x' * 47}...") - expect(span.get_tag('redis.raw_command')).to eq("SET K #{'x' * 47}...") else expect(span.resource).to eq('SET') - expect(span.get_tag('redis.raw_command')).to be_nil end + expect(span.get_tag('redis.raw_command')).to eq("SET K #{'x' * 47}...") end it_behaves_like 'a redis span with common tags' @@ -299,11 +289,10 @@ expect(span.service).to eq(options[:service_name] || 'redis') if options[:command_args] expect(span.resource).to eq('GET K') - expect(span.get_tag('redis.raw_command')).to eq('GET K') else expect(span.resource).to eq('GET') - expect(span.get_tag('redis.raw_command')).to be_nil end + expect(span.get_tag('redis.raw_command')).to eq('GET K') end it_behaves_like 'a redis span with common tags' @@ -340,11 +329,10 @@ if options[:command_args] expect(span.resource).to eq('AUTH ?') - expect(span.get_tag('redis.raw_command')).to eq('AUTH ?') else expect(span.resource).to eq('AUTH') - expect(span.get_tag('redis.raw_command')).to be_nil end + expect(span.get_tag('redis.raw_command')).to eq('AUTH ?') end end @@ -394,7 +382,7 @@ end end - context 'with redis optins' do + context 'with redis options' do let(:redis) do Redis.new( redis_options.merge(