Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redis:Omit command arguments from span.resource by default #3235

Merged
merged 2 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/GettingStarted.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 (for example, `key` in `GET key`) as resource name and tag. If `false`, only the command name is shown (for example, `GET`). | false |


**Configuring trace settings per instance**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
41 changes: 3 additions & 38 deletions lib/datadog/tracing/contrib/redis/instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require_relative 'ext'
require_relative 'quantize'
require_relative 'tags'
require_relative 'trace_middleware'

module Datadog
module Tracing
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/datadog/tracing/contrib/redis/tags.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
79 changes: 46 additions & 33 deletions lib/datadog/tracing/contrib/redis/trace_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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] || {}

Expand Down
6 changes: 3 additions & 3 deletions spec/datadog/tracing/contrib/rails/redis_cache_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions spec/datadog/tracing/contrib/redis/instrumentation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
28 changes: 14 additions & 14 deletions spec/datadog/tracing/contrib/redis/redis_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
Loading