From c938e61efa32fa078af0c3fd971a373468cff6e6 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Mar 2026 11:44:44 -0700 Subject: [PATCH 1/6] feat: Redis semantic stability --- instrumentation/redis/Appraisals | 29 +- instrumentation/redis/README.md | 16 + .../instrumentation/redis/instrumentation.rb | 50 ++- .../redis/middlewares/dup/redis_client.rb | 110 ++++++ .../redis/middlewares/old/redis_client.rb | 82 +++++ .../redis/middlewares/redis_client.rb | 80 ----- .../redis/middlewares/stable/redis_client.rb | 94 ++++++ .../redis/patches/dup/redis_v4_client.rb | 110 ++++++ .../redis/patches/old/redis_v4_client.rb | 97 ++++++ .../redis/patches/redis_v4_client.rb | 95 ------ .../redis/patches/stable/redis_v4_client.rb | 103 ++++++ .../instrumentation/redis/dup/client_test.rb | 261 +++++++++++++++ .../redis/instrumentation_test.rb | 5 +- .../redis/{patches => old}/client_test.rb | 9 +- .../redis/stable/client_test.rb | 314 ++++++++++++++++++ .../instrumentation/redis_client_test.rb | 7 +- 16 files changed, 1263 insertions(+), 199 deletions(-) create mode 100644 instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb create mode 100644 instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/old/redis_client.rb delete mode 100644 instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/redis_client.rb create mode 100644 instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb create mode 100644 instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb create mode 100644 instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/old/redis_v4_client.rb delete mode 100644 instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/redis_v4_client.rb create mode 100644 instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb create mode 100644 instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb rename instrumentation/redis/test/opentelemetry/instrumentation/redis/{patches => old}/client_test.rb (98%) create mode 100644 instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb diff --git a/instrumentation/redis/Appraisals b/instrumentation/redis/Appraisals index 07acf3432e..fe234e4694 100644 --- a/instrumentation/redis/Appraisals +++ b/instrumentation/redis/Appraisals @@ -1,16 +1,25 @@ # frozen_string_literal: true +# To facilitate database semantic convention stability migration, we are using +# appraisal to test the different semantic convention modes along with different +# gem versions. For more information on the semantic convention modes, see: +# https://opentelemetry.io/docs/specs/semconv/non-normative/db-migration/ + +semconv_stability = %w[old stable dup] + if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('4') - appraise 'redis-4.x' do - gem 'redis-client', '~> 0.22' - gem 'redis', '~> 4.8' - end -end + semconv_stability.each do |stability| + appraise "redis-4.x-#{stability}" do + gem 'redis-client', '~> 0.22' + gem 'redis', '~> 4.8' + end -appraise 'redis-5.x' do - gem 'redis', '~> 5.0' -end + appraise "redis-5.x-#{stability}" do + gem 'redis', '~> 5.0' + end -appraise 'redis-latest' do - gem 'redis' + appraise "redis-latest-#{stability}" do + gem 'redis' + end + end end diff --git a/instrumentation/redis/README.md b/instrumentation/redis/README.md index d86afacc66..520e50c781 100644 --- a/instrumentation/redis/README.md +++ b/instrumentation/redis/README.md @@ -70,6 +70,22 @@ The `opentelemetry-instrumentation-redis` gem source is [on github][repo-github] The OpenTelemetry Ruby gems are maintained by the OpenTelemetry Ruby special interest group (SIG). You can get involved by joining us on our [GitHub Discussions][discussions-url], [Slack Channel][slack-channel] or attending our weekly meeting. See the [meeting calendar][community-meetings] for dates and times. For more information on this and other language SIGs, see the OpenTelemetry [community page][ruby-sig]. +## Database semantic convention stability + +In the OpenTelemetry ecosystem, database semantic conventions have now reached a stable state. However, the initial Redis instrumentation was introduced before this stability was achieved, which resulted in database attributes being based on an older version of the semantic conventions. + +To facilitate the migration to stable semantic conventions, you can use the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. This variable allows you to opt-in to the new stable conventions, ensuring compatibility and future-proofing your instrumentation. + +When setting the value for `OTEL_SEMCONV_STABILITY_OPT_IN`, you can specify which conventions you wish to adopt: + +- `database` - Emits the stable database and networking conventions and ceases emitting the old conventions previously emitted by the instrumentation. +- `database/dup` - Emits both the old and stable database and networking conventions, enabling a phased rollout of the stable semantic conventions. +- Default behavior (in the absence of either value) is to continue emitting the old database and networking conventions the instrumentation previously emitted. + +During the transition from old to stable conventions, Redis instrumentation code comes in three patch versions: `dup`, `old`, and `stable`. These versions are identical except for the attributes they send. Any changes to Redis instrumentation should consider all three patches. + +For additional information on migration, please refer to our [documentation](https://opentelemetry.io/docs/specs/semconv/non-normative/db-migration/). + ## License Apache 2.0 license. See [LICENSE][license-github] for more information. diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/instrumentation.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/instrumentation.rb index d30462ca6c..a4762e0ccb 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/instrumentation.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/instrumentation.rb @@ -11,8 +11,9 @@ module Redis # instrumentation class Instrumentation < OpenTelemetry::Instrumentation::Base install do |_config| - require_dependencies - patch_client + patch_type = determine_semconv + send(:"require_dependencies_#{patch_type}") + send(:"patch_client_#{patch_type}") end present do @@ -25,14 +26,47 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base private - def require_dependencies - require_relative 'patches/redis_v4_client' if defined?(::Redis) && ::Redis::VERSION < '5' - require_relative 'middlewares/redis_client' if defined?(::RedisClient) + def determine_semconv + stability_opt_in = ENV.fetch('OTEL_SEMCONV_STABILITY_OPT_IN', '') + values = stability_opt_in.split(',').map(&:strip) + + if values.include?('database/dup') + 'dup' + elsif values.include?('database') + 'stable' + else + 'old' + end + end + + def require_dependencies_old + require_relative 'patches/old/redis_v4_client' if defined?(::Redis) && ::Redis::VERSION < '5' + require_relative 'middlewares/old/redis_client' if defined?(::RedisClient) + end + + def require_dependencies_stable + require_relative 'patches/stable/redis_v4_client' if defined?(::Redis) && ::Redis::VERSION < '5' + require_relative 'middlewares/stable/redis_client' if defined?(::RedisClient) + end + + def require_dependencies_dup + require_relative 'patches/dup/redis_v4_client' if defined?(::Redis) && ::Redis::VERSION < '5' + require_relative 'middlewares/dup/redis_client' if defined?(::RedisClient) + end + + def patch_client_old + ::RedisClient.register(Middlewares::Old::RedisClientInstrumentation) if defined?(::RedisClient) + ::Redis::Client.prepend(Patches::Old::RedisV4Client) if defined?(::Redis) && ::Redis::VERSION < '5' + end + + def patch_client_stable + ::RedisClient.register(Middlewares::Stable::RedisClientInstrumentation) if defined?(::RedisClient) + ::Redis::Client.prepend(Patches::Stable::RedisV4Client) if defined?(::Redis) && ::Redis::VERSION < '5' end - def patch_client - ::RedisClient.register(Middlewares::RedisClientInstrumentation) if defined?(::RedisClient) - ::Redis::Client.prepend(Patches::RedisV4Client) if defined?(::Redis) && ::Redis::VERSION < '5' + def patch_client_dup + ::RedisClient.register(Middlewares::Dup::RedisClientInstrumentation) if defined?(::RedisClient) + ::Redis::Client.prepend(Patches::Dup::RedisV4Client) if defined?(::Redis) && ::Redis::VERSION < '5' end end end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb new file mode 100644 index 0000000000..06e36bfdff --- /dev/null +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module Redis + module Middlewares + module Dup + # Default Redis port used to determine whether to include server.port + REDIS_DEFAULT_PORT = 6379 + + # Adapter for redis-client instrumentation interface + module RedisClientInstrumentation + MAX_STATEMENT_LENGTH = 500 + private_constant :MAX_STATEMENT_LENGTH + + def call(command, redis_config) + return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + attributes = span_attributes(redis_config) + + unless instrumentation.config[:db_statement] == :omit + serialized = serialize_commands([command]) + # Both old and new attributes + attributes['db.statement'] = serialized + attributes['db.query.text'] = serialized + end + + span_name = command[0].to_s.upcase + instrumentation.tracer.in_span(span_name, attributes: attributes, kind: :client) do |span| + super + rescue StandardError => e + span.set_attribute('error.type', e.class.name) + raise + end + end + + def call_pipelined(commands, redis_config) + return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + attributes = span_attributes(redis_config) + + unless instrumentation.config[:db_statement] == :omit + serialized = serialize_commands(commands) + # Both old and new attributes + attributes['db.statement'] = serialized + attributes['db.query.text'] = serialized + end + + instrumentation.tracer.in_span('PIPELINED', attributes: attributes, kind: :client) do |span| + super + rescue StandardError => e + span.set_attribute('error.type', e.class.name) + raise + end + end + + private + + def span_attributes(redis_config) + port = redis_config.port + + # Old conventions + attributes = { + 'db.system' => 'redis', + 'net.peer.name' => redis_config.host, + 'net.peer.port' => port + } + + # New stable conventions + attributes['db.system.name'] = 'redis' + attributes['server.address'] = redis_config.host + # Only add server.port if non-default + attributes['server.port'] = port if port && port != Dup::REDIS_DEFAULT_PORT + + attributes['db.redis.database_index'] = redis_config.db unless redis_config.db.zero? + attributes['peer.service'] = instrumentation.config[:peer_service] if instrumentation.config[:peer_service] + attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) + attributes + end + + def serialize_commands(commands) + obfuscate = instrumentation.config[:db_statement] == :obfuscate + + serialized_commands = commands.map do |command| + # If we receive an authentication request command we want to obfuscate it + if obfuscate || command[0].match?(/\A(AUTH|HELLO)\z/i) + command[0].to_s.upcase + (' ?' * (command.size - 1)) + else + command_copy = command.dup + command_copy[0] = command_copy[0].to_s.upcase + command_copy.join(' ') + end + end.join("\n") + serialized_commands = OpenTelemetry::Common::Utilities.truncate(serialized_commands, MAX_STATEMENT_LENGTH) + OpenTelemetry::Common::Utilities.utf8_encode(serialized_commands, binary: true) + end + + def instrumentation + Redis::Instrumentation.instance + end + end + end + end + end + end +end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/old/redis_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/old/redis_client.rb new file mode 100644 index 0000000000..e7b2cb0c7d --- /dev/null +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/old/redis_client.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module Redis + module Middlewares + module Old + # Adapter for redis-client instrumentation interface + module RedisClientInstrumentation + MAX_STATEMENT_LENGTH = 500 + private_constant :MAX_STATEMENT_LENGTH + + def call(command, redis_config) + return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + attributes = span_attributes(redis_config) + + attributes['db.statement'] = serialize_commands([command]) unless instrumentation.config[:db_statement] == :omit + + span_name = command[0].to_s.upcase + instrumentation.tracer.in_span(span_name, attributes: attributes, kind: :client) do + super + end + end + + def call_pipelined(commands, redis_config) + return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + attributes = span_attributes(redis_config) + + attributes['db.statement'] = serialize_commands(commands) unless instrumentation.config[:db_statement] == :omit + + instrumentation.tracer.in_span('PIPELINED', attributes: attributes, kind: :client) do + super + end + end + + private + + def span_attributes(redis_config) + attributes = { + 'db.system' => 'redis', + 'net.peer.name' => redis_config.host, + 'net.peer.port' => redis_config.port + } + + attributes['db.redis.database_index'] = redis_config.db unless redis_config.db.zero? + attributes['peer.service'] = instrumentation.config[:peer_service] if instrumentation.config[:peer_service] + attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) + attributes + end + + def serialize_commands(commands) + obfuscate = instrumentation.config[:db_statement] == :obfuscate + + serialized_commands = commands.map do |command| + # If we receive an authentication request command we want to obfuscate it + if obfuscate || command[0].match?(/\A(AUTH|HELLO)\z/i) + command[0].to_s.upcase + (' ?' * (command.size - 1)) + else + command_copy = command.dup + command_copy[0] = command_copy[0].to_s.upcase + command_copy.join(' ') + end + end.join("\n") + serialized_commands = OpenTelemetry::Common::Utilities.truncate(serialized_commands, MAX_STATEMENT_LENGTH) + OpenTelemetry::Common::Utilities.utf8_encode(serialized_commands, binary: true) + end + + def instrumentation + Redis::Instrumentation.instance + end + end + end + end + end + end +end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/redis_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/redis_client.rb deleted file mode 100644 index 7599cf3f5b..0000000000 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/redis_client.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true - -# Copyright The OpenTelemetry Authors -# -# SPDX-License-Identifier: Apache-2.0 - -module OpenTelemetry - module Instrumentation - module Redis - module Middlewares - # Adapter for redis-client instrumentation interface - module RedisClientInstrumentation - MAX_STATEMENT_LENGTH = 500 - private_constant :MAX_STATEMENT_LENGTH - - def call(command, redis_config) - return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? - - attributes = span_attributes(redis_config) - - attributes['db.statement'] = serialize_commands([command]) unless instrumentation.config[:db_statement] == :omit - - span_name = command[0].to_s.upcase - instrumentation.tracer.in_span(span_name, attributes: attributes, kind: :client) do - super - end - end - - def call_pipelined(commands, redis_config) - return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? - - attributes = span_attributes(redis_config) - - attributes['db.statement'] = serialize_commands(commands) unless instrumentation.config[:db_statement] == :omit - - instrumentation.tracer.in_span('PIPELINED', attributes: attributes, kind: :client) do - super - end - end - - private - - def span_attributes(redis_config) - attributes = { - 'db.system' => 'redis', - 'net.peer.name' => redis_config.host, - 'net.peer.port' => redis_config.port - } - - attributes['db.redis.database_index'] = redis_config.db unless redis_config.db.zero? - attributes['peer.service'] = instrumentation.config[:peer_service] if instrumentation.config[:peer_service] - attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) - attributes - end - - def serialize_commands(commands) - obfuscate = instrumentation.config[:db_statement] == :obfuscate - - serialized_commands = commands.map do |command| - # If we receive an authentication request command we want to obfuscate it - if obfuscate || command[0].match?(/\A(AUTH|HELLO)\z/i) - command[0].to_s.upcase + (' ?' * (command.size - 1)) - else - command_copy = command.dup - command_copy[0] = command_copy[0].to_s.upcase - command_copy.join(' ') - end - end.join("\n") - serialized_commands = OpenTelemetry::Common::Utilities.truncate(serialized_commands, MAX_STATEMENT_LENGTH) - OpenTelemetry::Common::Utilities.utf8_encode(serialized_commands, binary: true) - end - - def instrumentation - Redis::Instrumentation.instance - end - end - end - end - end -end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb new file mode 100644 index 0000000000..277cb786ac --- /dev/null +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module Redis + module Middlewares + module Stable + # Default Redis port used to determine whether to include server.port + REDIS_DEFAULT_PORT = 6379 + + # Adapter for redis-client instrumentation interface + module RedisClientInstrumentation + MAX_STATEMENT_LENGTH = 500 + private_constant :MAX_STATEMENT_LENGTH + + def call(command, redis_config) + return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + attributes = span_attributes(redis_config) + + attributes['db.query.text'] = serialize_commands([command]) unless instrumentation.config[:db_statement] == :omit + + span_name = command[0].to_s.upcase + instrumentation.tracer.in_span(span_name, attributes: attributes, kind: :client) do |span| + super + rescue StandardError => e + span.set_attribute('error.type', e.class.name) + raise + end + end + + def call_pipelined(commands, redis_config) + return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + attributes = span_attributes(redis_config) + + attributes['db.query.text'] = serialize_commands(commands) unless instrumentation.config[:db_statement] == :omit + + instrumentation.tracer.in_span('PIPELINED', attributes: attributes, kind: :client) do |span| + super + rescue StandardError => e + span.set_attribute('error.type', e.class.name) + raise + end + end + + private + + def span_attributes(redis_config) + attributes = { + 'db.system.name' => 'redis', + 'server.address' => redis_config.host + } + + # Only add server.port if non-default + port = redis_config.port + attributes['server.port'] = port if port && port != Stable::REDIS_DEFAULT_PORT + + attributes['db.redis.database_index'] = redis_config.db unless redis_config.db.zero? + attributes['peer.service'] = instrumentation.config[:peer_service] if instrumentation.config[:peer_service] + attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) + attributes + end + + def serialize_commands(commands) + obfuscate = instrumentation.config[:db_statement] == :obfuscate + + serialized_commands = commands.map do |command| + # If we receive an authentication request command we want to obfuscate it + if obfuscate || command[0].match?(/\A(AUTH|HELLO)\z/i) + command[0].to_s.upcase + (' ?' * (command.size - 1)) + else + command_copy = command.dup + command_copy[0] = command_copy[0].to_s.upcase + command_copy.join(' ') + end + end.join("\n") + serialized_commands = OpenTelemetry::Common::Utilities.truncate(serialized_commands, MAX_STATEMENT_LENGTH) + OpenTelemetry::Common::Utilities.utf8_encode(serialized_commands, binary: true) + end + + def instrumentation + Redis::Instrumentation.instance + end + end + end + end + end + end +end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb new file mode 100644 index 0000000000..2f0fc44d44 --- /dev/null +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module Redis + module Patches + module Dup + # Default Redis port used to determine whether to include server.port + REDIS_DEFAULT_PORT = 6379 + + # Module to prepend to Redis::Client for instrumentation + module RedisV4Client + MAX_STATEMENT_LENGTH = 500 + private_constant :MAX_STATEMENT_LENGTH + + def process(commands) + return super unless instrumentation_config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + host = options[:host] + port = options[:port] + + # Old conventions + attributes = { + 'db.system' => 'redis', + 'net.peer.name' => host, + 'net.peer.port' => port + } + + # New stable conventions + attributes['db.system.name'] = 'redis' + attributes['server.address'] = host + # Only add server.port if non-default + attributes['server.port'] = port if port && port != Dup::REDIS_DEFAULT_PORT + + attributes['db.redis.database_index'] = options[:db] unless options[:db].zero? + attributes['peer.service'] = instrumentation_config[:peer_service] if instrumentation_config[:peer_service] + attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) + + unless instrumentation_config[:db_statement] == :omit + parsed_commands = parse_commands(commands) + parsed_commands = OpenTelemetry::Common::Utilities.truncate(parsed_commands, MAX_STATEMENT_LENGTH) + parsed_commands = OpenTelemetry::Common::Utilities.utf8_encode(parsed_commands, binary: true) + # Both old and new attributes + attributes['db.statement'] = parsed_commands + attributes['db.query.text'] = parsed_commands + end + + span_name = if commands.length == 1 + commands[0][0].to_s.upcase + else + 'PIPELINED' + end + + instrumentation_tracer.in_span(span_name, attributes: attributes, kind: :client) do |s| + super.tap do |reply| + if reply.is_a?(::Redis::CommandError) + s.set_attribute('error.type', reply.class.name) + s.record_exception(reply) + s.status = Trace::Status.error(reply.message) + end + end + end + end + + private + + # Examples of commands received for parsing + # Redis#queue [[[:set, "v1", "0"]], [[:incr, "v1"]], [[:get, "v1"]]] + # Redis#pipeline: [[:set, "v1", "0"], [:incr, "v1"], [:get, "v1"]] + # Redis#hmset [[:hmset, "hash", "f1", 1234567890.0987654]] + # Redis#set [[:set, "K", "x"]] + def parse_commands(commands) + commands.map do |command| + # We are checking for the use of Redis#queue command, if we detect the + # extra level of array nesting we return the first element so it + # can be parsed. + command = command[0] if command.is_a?(Array) && command[0].is_a?(Array) + + # If we receive an authentication request command + # we want to short circuit parsing the commands + # and return the obfuscated command + return 'AUTH ?' if command[0] == :auth + + if instrumentation_config[:db_statement] == :obfuscate + command[0].to_s.upcase + (' ?' * (command.size - 1)) + else + command_copy = command.dup + command_copy[0] = command_copy[0].to_s.upcase + command_copy.join(' ') + end + end.join("\n") + end + + def instrumentation_tracer + Redis::Instrumentation.instance.tracer + end + + def instrumentation_config + Redis::Instrumentation.instance.config + end + end + end + end + end + end +end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/old/redis_v4_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/old/redis_v4_client.rb new file mode 100644 index 0000000000..dc4090935c --- /dev/null +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/old/redis_v4_client.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module Redis + module Patches + module Old + # Module to prepend to Redis::Client for instrumentation + module RedisV4Client + MAX_STATEMENT_LENGTH = 500 + private_constant :MAX_STATEMENT_LENGTH + + def process(commands) + return super unless instrumentation_config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + host = options[:host] + port = options[:port] + + attributes = { + 'db.system' => 'redis', + 'net.peer.name' => host, + 'net.peer.port' => port + } + + attributes['db.redis.database_index'] = options[:db] unless options[:db].zero? + attributes['peer.service'] = instrumentation_config[:peer_service] if instrumentation_config[:peer_service] + attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) + + unless instrumentation_config[:db_statement] == :omit + parsed_commands = parse_commands(commands) + parsed_commands = OpenTelemetry::Common::Utilities.truncate(parsed_commands, MAX_STATEMENT_LENGTH) + parsed_commands = OpenTelemetry::Common::Utilities.utf8_encode(parsed_commands, binary: true) + attributes['db.statement'] = parsed_commands + end + + span_name = if commands.length == 1 + commands[0][0].to_s.upcase + else + 'PIPELINED' + end + + instrumentation_tracer.in_span(span_name, attributes: attributes, kind: :client) do |s| + super.tap do |reply| + if reply.is_a?(::Redis::CommandError) + s.record_exception(reply) + s.status = Trace::Status.error(reply.message) + end + end + end + end + + private + + # Examples of commands received for parsing + # Redis#queue [[[:set, "v1", "0"]], [[:incr, "v1"]], [[:get, "v1"]]] + # Redis#pipeline: [[:set, "v1", "0"], [:incr, "v1"], [:get, "v1"]] + # Redis#hmset [[:hmset, "hash", "f1", 1234567890.0987654]] + # Redis#set [[:set, "K", "x"]] + def parse_commands(commands) + commands.map do |command| + # We are checking for the use of Redis#queue command, if we detect the + # extra level of array nesting we return the first element so it + # can be parsed. + command = command[0] if command.is_a?(Array) && command[0].is_a?(Array) + + # If we receive an authentication request command + # we want to short circuit parsing the commands + # and return the obfuscated command + return 'AUTH ?' if command[0] == :auth + + if instrumentation_config[:db_statement] == :obfuscate + command[0].to_s.upcase + (' ?' * (command.size - 1)) + else + command_copy = command.dup + command_copy[0] = command_copy[0].to_s.upcase + command_copy.join(' ') + end + end.join("\n") + end + + def instrumentation_tracer + Redis::Instrumentation.instance.tracer + end + + def instrumentation_config + Redis::Instrumentation.instance.config + end + end + end + end + end + end +end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/redis_v4_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/redis_v4_client.rb deleted file mode 100644 index 6bd473c09e..0000000000 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/redis_v4_client.rb +++ /dev/null @@ -1,95 +0,0 @@ -# frozen_string_literal: true - -# Copyright The OpenTelemetry Authors -# -# SPDX-License-Identifier: Apache-2.0 - -module OpenTelemetry - module Instrumentation - module Redis - module Patches - # Module to prepend to Redis::Client for instrumentation - module RedisV4Client - MAX_STATEMENT_LENGTH = 500 - private_constant :MAX_STATEMENT_LENGTH - - def process(commands) - return super unless instrumentation_config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? - - host = options[:host] - port = options[:port] - - attributes = { - 'db.system' => 'redis', - 'net.peer.name' => host, - 'net.peer.port' => port - } - - attributes['db.redis.database_index'] = options[:db] unless options[:db].zero? - attributes['peer.service'] = instrumentation_config[:peer_service] if instrumentation_config[:peer_service] - attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) - - unless instrumentation_config[:db_statement] == :omit - parsed_commands = parse_commands(commands) - parsed_commands = OpenTelemetry::Common::Utilities.truncate(parsed_commands, MAX_STATEMENT_LENGTH) - parsed_commands = OpenTelemetry::Common::Utilities.utf8_encode(parsed_commands, binary: true) - attributes['db.statement'] = parsed_commands - end - - span_name = if commands.length == 1 - commands[0][0].to_s.upcase - else - 'PIPELINED' - end - - instrumentation_tracer.in_span(span_name, attributes: attributes, kind: :client) do |s| - super.tap do |reply| - if reply.is_a?(::Redis::CommandError) - s.record_exception(reply) - s.status = Trace::Status.error(reply.message) - end - end - end - end - - private - - # Examples of commands received for parsing - # Redis#queue [[[:set, "v1", "0"]], [[:incr, "v1"]], [[:get, "v1"]]] - # Redis#pipeline: [[:set, "v1", "0"], [:incr, "v1"], [:get, "v1"]] - # Redis#hmset [[:hmset, "hash", "f1", 1234567890.0987654]] - # Redis#set [[:set, "K", "x"]] - def parse_commands(commands) - commands.map do |command| - # We are checking for the use of Redis#queue command, if we detect the - # extra level of array nesting we return the first element so it - # can be parsed. - command = command[0] if command.is_a?(Array) && command[0].is_a?(Array) - - # If we receive an authentication request command - # we want to short circuit parsing the commands - # and return the obfuscated command - return 'AUTH ?' if command[0] == :auth - - if instrumentation_config[:db_statement] == :obfuscate - command[0].to_s.upcase + (' ?' * (command.size - 1)) - else - command_copy = command.dup - command_copy[0] = command_copy[0].to_s.upcase - command_copy.join(' ') - end - end.join("\n") - end - - def instrumentation_tracer - Redis::Instrumentation.instance.tracer - end - - def instrumentation_config - Redis::Instrumentation.instance.config - end - end - end - end - end -end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb new file mode 100644 index 0000000000..23756b9feb --- /dev/null +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module Redis + module Patches + module Stable + # Default Redis port used to determine whether to include server.port + REDIS_DEFAULT_PORT = 6379 + + # Module to prepend to Redis::Client for instrumentation + module RedisV4Client + MAX_STATEMENT_LENGTH = 500 + private_constant :MAX_STATEMENT_LENGTH + + def process(commands) + return super unless instrumentation_config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + + host = options[:host] + port = options[:port] + + attributes = { + 'db.system.name' => 'redis', + 'server.address' => host + } + + # Only add server.port if non-default + attributes['server.port'] = port if port && port != Stable::REDIS_DEFAULT_PORT + + attributes['db.redis.database_index'] = options[:db] unless options[:db].zero? + attributes['peer.service'] = instrumentation_config[:peer_service] if instrumentation_config[:peer_service] + attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) + + unless instrumentation_config[:db_statement] == :omit + parsed_commands = parse_commands(commands) + parsed_commands = OpenTelemetry::Common::Utilities.truncate(parsed_commands, MAX_STATEMENT_LENGTH) + parsed_commands = OpenTelemetry::Common::Utilities.utf8_encode(parsed_commands, binary: true) + attributes['db.query.text'] = parsed_commands + end + + span_name = if commands.length == 1 + commands[0][0].to_s.upcase + else + 'PIPELINED' + end + + instrumentation_tracer.in_span(span_name, attributes: attributes, kind: :client) do |s| + super.tap do |reply| + if reply.is_a?(::Redis::CommandError) + s.set_attribute('error.type', reply.class.name) + s.record_exception(reply) + s.status = Trace::Status.error(reply.message) + end + end + end + end + + private + + # Examples of commands received for parsing + # Redis#queue [[[:set, "v1", "0"]], [[:incr, "v1"]], [[:get, "v1"]]] + # Redis#pipeline: [[:set, "v1", "0"], [:incr, "v1"], [:get, "v1"]] + # Redis#hmset [[:hmset, "hash", "f1", 1234567890.0987654]] + # Redis#set [[:set, "K", "x"]] + def parse_commands(commands) + commands.map do |command| + # We are checking for the use of Redis#queue command, if we detect the + # extra level of array nesting we return the first element so it + # can be parsed. + command = command[0] if command.is_a?(Array) && command[0].is_a?(Array) + + # If we receive an authentication request command + # we want to short circuit parsing the commands + # and return the obfuscated command + return 'AUTH ?' if command[0] == :auth + + if instrumentation_config[:db_statement] == :obfuscate + command[0].to_s.upcase + (' ?' * (command.size - 1)) + else + command_copy = command.dup + command_copy[0] = command_copy[0].to_s.upcase + command_copy.join(' ') + end + end.join("\n") + end + + def instrumentation_tracer + Redis::Instrumentation.instance.tracer + end + + def instrumentation_config + Redis::Instrumentation.instance.config + end + end + end + end + end + end +end diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb new file mode 100644 index 0000000000..a97aed805e --- /dev/null +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb @@ -0,0 +1,261 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +require_relative '../../../../../lib/opentelemetry/instrumentation/redis' +require_relative '../../../../../lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client' + +# Tests for dup semantic convention mode (both old and stable attributes) +describe OpenTelemetry::Instrumentation::Redis::Patches::Dup::RedisV4Client do + let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } + let(:exporter) { EXPORTER } + let(:password) { 'passw0rd' } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } + let(:last_span) { exporter.finished_spans.last } + + def redis_with_auth(redis_options = {}) + redis_options[:password] ||= password + redis_options[:host] ||= redis_host + redis_options[:port] ||= redis_port + Redis.new(redis_options) + end + + def redis_version + Gem.loaded_specs['redis']&.version + end + + def redis_version_major + redis_version&.segments&.first + end + + def redis_gte_5? + redis_version_major&.>=(5) + end + + before do + skip unless ENV['BUNDLE_GEMFILE']&.include?('dup') + + ENV['OTEL_SEMCONV_STABILITY_OPT_IN'] = 'database/dup' + config = { db_statement: :include } + instrumentation.install(config) + exporter.reset + end + + after { instrumentation.instance_variable_set(:@installed, false) } + + describe '#process' do + it 'before request' do + _(exporter.finished_spans.size).must_equal 0 + end + + it 'after authorization with Redis server includes both old and new attributes' do + Redis.new(host: redis_host, port: redis_port).auth(password) + + _(last_span.name).must_equal 'AUTH' + # Old attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal 'AUTH ?' + _(last_span.attributes['net.peer.name']).must_equal redis_host + _(last_span.attributes['net.peer.port']).must_equal redis_port + # New attributes + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'AUTH ?' + _(last_span.attributes['server.address']).must_equal redis_host + # server.port only included if non-default (6379) + _(last_span.attributes['server.port']).must_equal redis_port if redis_port != 6379 + end + + it 'after requests includes both old and new attributes' do + redis = redis_with_auth + _(redis.set('K', 'x')).must_equal 'OK' + _(redis.get('K')).must_equal 'x' + + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + # Old attributes + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.statement']).must_equal('SET K x') + _(set_span.attributes['net.peer.name']).must_equal redis_host + _(set_span.attributes['net.peer.port']).must_equal redis_port + # New attributes + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('SET K x') + _(set_span.attributes['server.address']).must_equal redis_host + + get_span = exporter.finished_spans.last + _(get_span.name).must_equal 'GET' + # Old attributes + _(get_span.attributes['db.system']).must_equal 'redis' + _(get_span.attributes['db.statement']).must_equal 'GET K' + _(get_span.attributes['net.peer.name']).must_equal redis_host + # New attributes + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.query.text']).must_equal 'GET K' + _(get_span.attributes['server.address']).must_equal redis_host + end + + it 'reflects db index' do + skip if redis_gte_5? + + redis = redis_with_auth(db: 1) + redis.get('K') + + _(exporter.finished_spans.size).must_equal 3 + + select_span = exporter.finished_spans[1] + _(select_span.name).must_equal 'SELECT' + # Both attributes + _(select_span.attributes['db.statement']).must_equal('SELECT 1') + _(select_span.attributes['db.query.text']).must_equal('SELECT 1') + _(select_span.attributes['db.system']).must_equal 'redis' + _(select_span.attributes['db.system.name']).must_equal 'redis' + _(select_span.attributes['db.redis.database_index']).must_equal 1 + + get_span = exporter.finished_spans.last + _(get_span.name).must_equal 'GET' + _(get_span.attributes['db.system']).must_equal 'redis' + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.statement']).must_equal('GET K') + _(get_span.attributes['db.query.text']).must_equal('GET K') + _(get_span.attributes['db.redis.database_index']).must_equal 1 + end + + it 'reflects db index v5' do + skip unless redis_gte_5? + + redis = redis_with_auth(db: 1) + redis.get('K') + + _(exporter.finished_spans.size).must_equal 2 + select_span = exporter.finished_spans.first + get_span = exporter.finished_spans.last + _(select_span.name).must_equal 'PIPELINED' + # Both attributes + _(select_span.attributes['db.statement']).must_equal("AUTH ?\nSELECT 1") + _(select_span.attributes['db.query.text']).must_equal("AUTH ?\nSELECT 1") + _(select_span.attributes['db.system']).must_equal 'redis' + _(select_span.attributes['db.system.name']).must_equal 'redis' + _(select_span.attributes['db.redis.database_index']).must_equal 1 + + _(get_span.name).must_equal 'GET' + _(get_span.attributes['db.system']).must_equal 'redis' + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.statement']).must_equal('GET K') + _(get_span.attributes['db.query.text']).must_equal('GET K') + _(get_span.attributes['db.redis.database_index']).must_equal 1 + end + + it 'records exceptions with error.type' do + skip if redis_gte_5? + + expect do + redis = redis_with_auth + redis.call 'THIS_IS_NOT_A_REDIS_FUNC', 'THIS_IS_NOT_A_VALID_ARG' + end.must_raise Redis::CommandError + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'THIS_IS_NOT_A_REDIS_FUNC' + # Both attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['db.query.text']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['error.type']).must_equal 'Redis::CommandError' + _(last_span.status.code).must_equal( + OpenTelemetry::Trace::Status::ERROR + ) + end + + it 'records exceptions with error.type v5' do + skip unless redis_gte_5? + + expect do + redis = redis_with_auth + redis.call 'THIS_IS_NOT_A_REDIS_FUNC', 'THIS_IS_NOT_A_VALID_ARG' + end.must_raise Redis::CommandError + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'THIS_IS_NOT_A_REDIS_FUNC' + # Both attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['db.query.text']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['error.type']).must_equal 'RedisClient::CommandError' + _(last_span.status.code).must_equal( + OpenTelemetry::Trace::Status::ERROR + ) + end + + it 'traces pipelined commands' do + redis = redis_with_auth + redis.pipelined do |r| + r.set('v1', '0') + r.incr('v1') + r.get('v1') + end + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'PIPELINED' + # Both attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal "SET v1 0\nINCR v1\nGET v1" + _(last_span.attributes['db.query.text']).must_equal "SET v1 0\nINCR v1\nGET v1" + _(last_span.attributes['net.peer.name']).must_equal redis_host + _(last_span.attributes['server.address']).must_equal redis_host + end + + describe 'when db_statement is :omit' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :omit) + end + + it 'omits both db.statement and db.query.text attributes' do + skip if redis_gte_5? + + redis = redis_with_auth + _(redis.set('K', 'xyz')).must_equal 'OK' + _(exporter.finished_spans.size).must_equal 2 + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.statement') + _(set_span.attributes).wont_include('db.query.text') + end + + it 'omits both db.statement and db.query.text attributes v5' do + skip unless redis_gte_5? + + redis = redis_with_auth + _(redis.set('K', 'xyz')).must_equal 'OK' + _(exporter.finished_spans.size).must_equal 2 + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.statement') + _(set_span.attributes).wont_include('db.query.text') + end + end + end +end unless ENV['OMIT_SERVICES'] diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/instrumentation_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/instrumentation_test.rb index c1714876fc..f6a77fc4c7 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/instrumentation_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/instrumentation_test.rb @@ -7,7 +7,6 @@ require 'test_helper' require_relative '../../../../lib/opentelemetry/instrumentation/redis' -require_relative '../../../../lib/opentelemetry/instrumentation/redis/patches/redis_v4_client' describe OpenTelemetry::Instrumentation::Redis::Instrumentation do let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } @@ -22,6 +21,10 @@ end describe '#install' do + before do + skip unless ENV['BUNDLE_GEMFILE']&.include?('old') + end + it 'accepts argument' do _(instrumentation.install({})).must_equal(true) instrumentation.instance_variable_set(:@installed, false) diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/patches/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb similarity index 98% rename from instrumentation/redis/test/opentelemetry/instrumentation/redis/patches/client_test.rb rename to instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb index 261970614c..52716389d1 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/patches/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb @@ -7,15 +7,16 @@ require 'test_helper' require_relative '../../../../../lib/opentelemetry/instrumentation/redis' -require_relative '../../../../../lib/opentelemetry/instrumentation/redis/patches/redis_v4_client' +require_relative '../../../../../lib/opentelemetry/instrumentation/redis/patches/old/redis_v4_client' -describe OpenTelemetry::Instrumentation::Redis::Patches::RedisV4Client do +# Tests for old semantic convention attributes (db.system, net.peer.name, net.peer.port, db.statement) +describe OpenTelemetry::Instrumentation::Redis::Patches::Old::RedisV4Client do # NOTE: These tests should be run for redis v4 and redis v5, even though the patches won't be installed on v5. # Perhaps these tests should live in a different file? let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV['TEST_REDIS_HOST'] } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } @@ -42,6 +43,8 @@ def redis_gte_5? end before do + skip unless ENV['BUNDLE_GEMFILE']&.include?('old') + # ensure obfuscation is off if it was previously set in a different test config = { db_statement: :include } instrumentation.install(config) diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb new file mode 100644 index 0000000000..4d58575fa1 --- /dev/null +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb @@ -0,0 +1,314 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +require_relative '../../../../../lib/opentelemetry/instrumentation/redis' +require_relative '../../../../../lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client' + +# Tests for stable semantic convention attributes (db.system.name, server.address, server.port, db.query.text) +describe OpenTelemetry::Instrumentation::Redis::Patches::Stable::RedisV4Client do + let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } + let(:exporter) { EXPORTER } + let(:password) { 'passw0rd' } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } + let(:last_span) { exporter.finished_spans.last } + + def redis_with_auth(redis_options = {}) + redis_options[:password] ||= password + redis_options[:host] ||= redis_host + redis_options[:port] ||= redis_port + Redis.new(redis_options) + end + + def redis_version + Gem.loaded_specs['redis']&.version + end + + def redis_version_major + redis_version&.segments&.first + end + + def redis_gte_5? + redis_version_major&.>=(5) + end + + before do + skip unless ENV['BUNDLE_GEMFILE']&.include?('stable') + + ENV['OTEL_SEMCONV_STABILITY_OPT_IN'] = 'database' + config = { db_statement: :include } + instrumentation.install(config) + exporter.reset + end + + after { instrumentation.instance_variable_set(:@installed, false) } + + describe '#process' do + it 'before request' do + _(exporter.finished_spans.size).must_equal 0 + end + + it 'after authorization with Redis server' do + Redis.new(host: redis_host, port: redis_port).auth(password) + + _(last_span.name).must_equal 'AUTH' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'AUTH ?' + _(last_span.attributes['server.address']).must_equal redis_host + # server.port only included if non-default (6379) + if redis_port == 6379 + _(last_span.attributes['server.port']).must_be_nil + else + _(last_span.attributes['server.port']).must_equal redis_port + end + end + + it 'after requests' do + redis = redis_with_auth + _(redis.set('K', 'x')).must_equal 'OK' + _(redis.get('K')).must_equal 'x' + + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('SET K x') + _(set_span.attributes['server.address']).must_equal redis_host + + get_span = exporter.finished_spans.last + _(get_span.name).must_equal 'GET' + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.query.text']).must_equal 'GET K' + _(get_span.attributes['server.address']).must_equal redis_host + end + + it 'reflects db index' do + skip if redis_gte_5? + + redis = redis_with_auth(db: 1) + redis.get('K') + + _(exporter.finished_spans.size).must_equal 3 + + select_span = exporter.finished_spans[1] + _(select_span.name).must_equal 'SELECT' + _(select_span.attributes['db.query.text']).must_equal('SELECT 1') + _(select_span.attributes['db.system.name']).must_equal 'redis' + _(select_span.attributes['server.address']).must_equal redis_host + _(select_span.attributes['db.redis.database_index']).must_equal 1 + + get_span = exporter.finished_spans.last + _(get_span.name).must_equal 'GET' + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.query.text']).must_equal('GET K') + _(get_span.attributes['db.redis.database_index']).must_equal 1 + end + + it 'reflects db index v5' do + skip unless redis_gte_5? + + redis = redis_with_auth(db: 1) + redis.get('K') + + _(exporter.finished_spans.size).must_equal 2 + select_span = exporter.finished_spans.first + get_span = exporter.finished_spans.last + _(select_span.name).must_equal 'PIPELINED' + _(select_span.attributes['db.query.text']).must_equal("AUTH ?\nSELECT 1") + _(select_span.attributes['db.system.name']).must_equal 'redis' + _(select_span.attributes['server.address']).must_equal redis_host + _(select_span.attributes['db.redis.database_index']).must_equal 1 + + _(get_span.name).must_equal 'GET' + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.query.text']).must_equal('GET K') + _(get_span.attributes['db.redis.database_index']).must_equal 1 + end + + it 'records exceptions' do + skip if redis_gte_5? + + expect do + redis = redis_with_auth + redis.call 'THIS_IS_NOT_A_REDIS_FUNC', 'THIS_IS_NOT_A_VALID_ARG' + end.must_raise Redis::CommandError + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'THIS_IS_NOT_A_REDIS_FUNC' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['server.address']).must_equal redis_host + _(last_span.attributes['error.type']).must_equal 'Redis::CommandError' + _(last_span.status.code).must_equal( + OpenTelemetry::Trace::Status::ERROR + ) + end + + it 'records exceptions v5' do + skip unless redis_gte_5? + + expect do + redis = redis_with_auth + redis.call 'THIS_IS_NOT_A_REDIS_FUNC', 'THIS_IS_NOT_A_VALID_ARG' + end.must_raise Redis::CommandError + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'THIS_IS_NOT_A_REDIS_FUNC' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['server.address']).must_equal redis_host + _(last_span.attributes['error.type']).must_equal 'RedisClient::CommandError' + _(last_span.status.code).must_equal( + OpenTelemetry::Trace::Status::ERROR + ) + end + + it 'traces pipelined commands' do + redis = redis_with_auth + redis.pipelined do |r| + r.set('v1', '0') + r.incr('v1') + r.get('v1') + end + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'PIPELINED' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal "SET v1 0\nINCR v1\nGET v1" + _(last_span.attributes['server.address']).must_equal redis_host + end + + it 'records server.address and server.port for non-default port' do + skip if redis_gte_5? + + client = Redis.new(host: 'example.com', port: 8321, timeout: 0.01) + _ { client.auth(password) }.must_raise Redis::CannotConnectError + + _(last_span.name).must_equal 'AUTH' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'AUTH ?' + _(last_span.attributes['server.address']).must_equal 'example.com' + _(last_span.attributes['server.port']).must_equal 8321 + end + + describe 'when db_statement is :omit' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :omit) + end + + it 'omits db.query.text attribute' do + skip if redis_gte_5? + + redis = redis_with_auth + _(redis.set('K', 'xyz')).must_equal 'OK' + _(redis.get('K')).must_equal 'xyz' + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[0] + _(set_span.name).must_equal('AUTH') + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + + set_span = exporter.finished_spans[2] + _(set_span.name).must_equal 'GET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + end + + it 'omits db.query.text attribute v5' do + skip unless redis_gte_5? + + redis = redis_with_auth + _(redis.set('K', 'xyz')).must_equal 'OK' + _(redis.get('K')).must_equal 'xyz' + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[0] + _(set_span.name).must_equal('PIPELINED') + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + + set_span = exporter.finished_spans[2] + _(set_span.name).must_equal 'GET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + end + end + + describe 'when db_statement is :obfuscate' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :obfuscate) + end + + it 'obfuscates arguments in db.query.text' do + skip if redis_gte_5? + + redis = redis_with_auth + _(redis.set('K', 'xyz')).must_equal 'OK' + _(redis.get('K')).must_equal 'xyz' + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[0] + _(set_span.name).must_equal('AUTH') + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('AUTH ?') + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('SET ? ?') + + set_span = exporter.finished_spans[2] + _(set_span.name).must_equal 'GET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('GET ?') + end + + it 'obfuscates arguments in db.query.text v5' do + skip unless redis_gte_5? + + redis = redis_with_auth + _(redis.set('K', 'xyz')).must_equal 'OK' + _(redis.get('K')).must_equal 'xyz' + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[0] + _(set_span.name).must_equal('PIPELINED') + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('AUTH ?') + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('SET ? ?') + + set_span = exporter.finished_spans[2] + _(set_span.name).must_equal 'GET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('GET ?') + end + end + end +end unless ENV['OMIT_SERVICES'] diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis_client_test.rb index 253ce331ce..f5141d1803 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis_client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis_client_test.rb @@ -7,9 +7,10 @@ require 'test_helper' require_relative '../../../lib/opentelemetry/instrumentation/redis' -require_relative '../../../lib/opentelemetry/instrumentation/redis/middlewares/redis_client' +require_relative '../../../lib/opentelemetry/instrumentation/redis/middlewares/old/redis_client' -describe OpenTelemetry::Instrumentation::Redis::Middlewares::RedisClientInstrumentation do +# Tests for old semantic convention attributes via RedisClient middleware +describe OpenTelemetry::Instrumentation::Redis::Middlewares::Old::RedisClientInstrumentation do let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } @@ -30,6 +31,8 @@ def redis_with_auth(redis_options = {}) end before do + skip unless ENV['BUNDLE_GEMFILE']&.include?('old') + # ensure obfuscation is off if it was previously set in a different test config = { db_statement: :include } instrumentation.install(config) From 8bf2155957c16f63a7a31419d31427c8ef7bf4ab Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Mar 2026 11:56:18 -0700 Subject: [PATCH 2/6] Update tests --- instrumentation/redis/Appraisals | 17 +- .../redis/dup/redis_client_test.rb | 406 ++++++++++++++++++ .../redis/stable/redis_client_test.rb | 368 ++++++++++++++++ 3 files changed, 783 insertions(+), 8 deletions(-) create mode 100644 instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb create mode 100644 instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb diff --git a/instrumentation/redis/Appraisals b/instrumentation/redis/Appraisals index fe234e4694..9a934d517d 100644 --- a/instrumentation/redis/Appraisals +++ b/instrumentation/redis/Appraisals @@ -7,19 +7,20 @@ semconv_stability = %w[old stable dup] -if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('4') - semconv_stability.each do |stability| +semconv_stability.each do |stability| + # redis-4.x requires redis-client which has Ruby version constraints + if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('4') appraise "redis-4.x-#{stability}" do gem 'redis-client', '~> 0.22' gem 'redis', '~> 4.8' end + end - appraise "redis-5.x-#{stability}" do - gem 'redis', '~> 5.0' - end + appraise "redis-5.x-#{stability}" do + gem 'redis', '~> 5.0' + end - appraise "redis-latest-#{stability}" do - gem 'redis' - end + appraise "redis-latest-#{stability}" do + gem 'redis' end end diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb new file mode 100644 index 0000000000..4a3602504d --- /dev/null +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb @@ -0,0 +1,406 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +require_relative '../../../../../lib/opentelemetry/instrumentation/redis' +require_relative '../../../../../lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client' + +# Tests for dup semantic convention mode (both old and stable attributes) via RedisClient middleware +describe OpenTelemetry::Instrumentation::Redis::Middlewares::Dup::RedisClientInstrumentation do + let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } + let(:exporter) { EXPORTER } + let(:password) { 'passw0rd' } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } + let(:last_span) { exporter.finished_spans.last } + + # Instantiate the Redis client with the correct password. Note that this + # will generate one extra span on connect because the Redis client will + # send an AUTH command before doing anything else. + def redis_with_auth(redis_options = {}) + redis_options[:password] ||= password + redis_options[:host] ||= redis_host + redis_options[:port] ||= redis_port + RedisClient.new(**redis_options).tap do |client| + client.send(:raw_connection) # force lazy client to connect + end + end + + before do + skip unless ENV['BUNDLE_GEMFILE']&.include?('dup') + + ENV['OTEL_SEMCONV_STABILITY_OPT_IN'] = 'database/dup' + config = { db_statement: :include } + instrumentation.install(config) + exporter.reset + end + + # Force re-install of instrumentation + after { instrumentation.instance_variable_set(:@installed, false) } + + describe '#process' do + it 'before request' do + _(exporter.finished_spans.size).must_equal 0 + end + + it 'accepts peer service name from config' do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(peer_service: 'readonly:redis') + redis_with_auth + + _(last_span.attributes['peer.service']).must_equal 'readonly:redis' + end + + it 'context attributes take priority' do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(peer_service: 'readonly:redis') + redis = redis_with_auth + + OpenTelemetry::Instrumentation::Redis.with_attributes('peer.service' => 'foo') do + redis.call('set', 'K', 'x') + end + + _(last_span.attributes['peer.service']).must_equal 'foo' + end + + it 'after authorization with Redis server includes both old and new attributes' do + client = redis_with_auth + + _(client.connected?).must_equal(true) + + _(last_span.name).must_equal 'PIPELINED' + # Old attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal 'HELLO ? ? ? ?' + _(last_span.attributes['net.peer.name']).must_equal redis_host + _(last_span.attributes['net.peer.port']).must_equal redis_port + # New attributes + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'HELLO ? ? ? ?' + _(last_span.attributes['server.address']).must_equal redis_host + # server.port only included if non-default (6379) + _(last_span.attributes['server.port']).must_equal redis_port if redis_port != 6379 + end + + it 'after calling auth lowercase' do + client = redis_with_auth + client.call('auth', password) + + _(last_span.name).must_equal 'AUTH' + # Old attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal 'AUTH ?' + _(last_span.attributes['net.peer.name']).must_equal redis_host + _(last_span.attributes['net.peer.port']).must_equal redis_port + # New attributes + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'AUTH ?' + _(last_span.attributes['server.address']).must_equal redis_host + end + + it 'after calling AUTH uppercase' do + client = redis_with_auth + client.call('AUTH', password) + + _(last_span.name).must_equal 'AUTH' + # Old attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal 'AUTH ?' + _(last_span.attributes['net.peer.name']).must_equal redis_host + _(last_span.attributes['net.peer.port']).must_equal redis_port + # New attributes + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'AUTH ?' + _(last_span.attributes['server.address']).must_equal redis_host + end + + it 'after requests includes both old and new attributes' do + redis = redis_with_auth + _(redis.call('set', 'K', 'x')).must_equal 'OK' + _(redis.call('get', 'K')).must_equal 'x' + + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + # Old attributes + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.statement']).must_equal('SET K x') + _(set_span.attributes['net.peer.name']).must_equal redis_host + _(set_span.attributes['net.peer.port']).must_equal redis_port + # New attributes + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('SET K x') + _(set_span.attributes['server.address']).must_equal redis_host + + get_span = exporter.finished_spans.last + _(get_span.name).must_equal 'GET' + # Old attributes + _(get_span.attributes['db.system']).must_equal 'redis' + _(get_span.attributes['db.statement']).must_equal 'GET K' + _(get_span.attributes['net.peer.name']).must_equal redis_host + # New attributes + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.query.text']).must_equal 'GET K' + _(get_span.attributes['server.address']).must_equal redis_host + end + + it 'reflects db index' do + redis = redis_with_auth(db: 1) + redis.call('get', 'K') + + _(exporter.finished_spans.size).must_equal 2 + + prelude_span = exporter.finished_spans.first + _(prelude_span.name).must_equal 'PIPELINED' + # Both old and new attributes + _(prelude_span.attributes['db.system']).must_equal 'redis' + _(prelude_span.attributes['db.system.name']).must_equal 'redis' + _(prelude_span.attributes['db.statement']).must_equal("HELLO ? ? ? ?\nSELECT 1") + _(prelude_span.attributes['db.query.text']).must_equal("HELLO ? ? ? ?\nSELECT 1") + _(prelude_span.attributes['net.peer.name']).must_equal redis_host + _(prelude_span.attributes['server.address']).must_equal redis_host + + get_span = exporter.finished_spans.last + _(get_span.name).must_equal 'GET' + _(get_span.attributes['db.system']).must_equal 'redis' + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.statement']).must_equal('GET K') + _(get_span.attributes['db.query.text']).must_equal('GET K') + _(get_span.attributes['db.redis.database_index']).must_equal 1 + _(get_span.attributes['net.peer.name']).must_equal redis_host + _(get_span.attributes['server.address']).must_equal redis_host + end + + it 'merges context attributes' do + redis = redis_with_auth + OpenTelemetry::Instrumentation::Redis.with_attributes('peer.service' => 'foo') do + redis.call('set', 'K', 'x') + end + + _(exporter.finished_spans.size).must_equal 2 + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.statement']).must_equal('SET K x') + _(set_span.attributes['db.query.text']).must_equal('SET K x') + _(set_span.attributes['peer.service']).must_equal 'foo' + _(set_span.attributes['net.peer.name']).must_equal redis_host + _(set_span.attributes['server.address']).must_equal redis_host + end + + it 'records exceptions with error.type' do + expect do + redis = redis_with_auth + redis.call 'THIS_IS_NOT_A_REDIS_FUNC', 'THIS_IS_NOT_A_VALID_ARG' + end.must_raise RedisClient::CommandError + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'THIS_IS_NOT_A_REDIS_FUNC' + # Both old and new attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['db.query.text']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['net.peer.name']).must_equal redis_host + _(last_span.attributes['server.address']).must_equal redis_host + _(last_span.attributes['error.type']).must_equal 'RedisClient::CommandError' + _(last_span.status.code).must_equal( + OpenTelemetry::Trace::Status::ERROR + ) + end + + it 'connect is uninstrumented' do + error = _ { redis_with_auth(host: 'example.com', port: 8321, timeout: 0.01) }.must_raise StandardError + # Ruby 4 changed the timeout error class + # Prior to that the client library would wrap the timeout in a RedisClient::CannotConnectError + _([IO::TimeoutError, RedisClient::CannotConnectError]).must_include error.class + + _(last_span).must_be_nil + end + + it 'traces pipelined commands' do + redis = redis_with_auth + redis.pipelined do |r| + r.call('set', 'v1', '0') + r.call('incr', 'v1') + r.call('get', 'v1') + end + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'PIPELINED' + # Both old and new attributes + _(last_span.attributes['db.system']).must_equal 'redis' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.statement']).must_equal "SET v1 0\nINCR v1\nGET v1" + _(last_span.attributes['db.query.text']).must_equal "SET v1 0\nINCR v1\nGET v1" + _(last_span.attributes['net.peer.name']).must_equal redis_host + _(last_span.attributes['server.address']).must_equal redis_host + end + + it 'records floats' do + redis = redis_with_auth + redis.call('hmset', 'hash', 'f1', 1_234_567_890.0987654321) + + _(last_span.name).must_equal 'HMSET' + _(last_span.attributes['db.statement']).must_equal 'HMSET hash f1 1234567890.0987654' + _(last_span.attributes['db.query.text']).must_equal 'HMSET hash f1 1234567890.0987654' + end + + it 'records empty string' do + redis = redis_with_auth + redis.call('set', 'K', '') + + _(last_span.name).must_equal 'SET' + _(last_span.attributes['db.statement']).must_equal 'SET K ' + _(last_span.attributes['db.query.text']).must_equal 'SET K ' + end + + it 'truncates long statements' do + redis = redis_with_auth + the_long_value = 'y' * 100 + redis.pipelined do |pipeline| + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + end + + expected_statement = <<~HEREDOC.chomp + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy... + HEREDOC + + _(last_span.name).must_equal 'PIPELINED' + _(last_span.attributes['db.statement'].size).must_equal 500 + _(last_span.attributes['db.statement']).must_equal expected_statement + _(last_span.attributes['db.query.text'].size).must_equal 500 + _(last_span.attributes['db.query.text']).must_equal expected_statement + end + + it 'encodes invalid byte sequences' do + redis = redis_with_auth + + # \255 is off-limits https://en.wikipedia.org/wiki/UTF-8#Codepage_layout + redis.call('set', 'K', "x\255") + + _(last_span.name).must_equal 'SET' + _(last_span.attributes['db.statement']).must_equal 'SET K x' + _(last_span.attributes['db.query.text']).must_equal 'SET K x' + end + + describe 'when trace_root_spans is disabled' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(trace_root_spans: false) + end + + it 'traces redis spans with a parent' do + redis = redis_with_auth + OpenTelemetry.tracer_provider.tracer('tester').in_span('a root!') do + redis.call('set', 'a', 'b') + end + + redis_span = exporter.finished_spans.find { |s| s.name == 'SET' } + _(redis_span.name).must_equal 'SET' + _(redis_span.attributes['db.statement']).must_equal 'SET ? ?' + _(redis_span.attributes['db.query.text']).must_equal 'SET ? ?' + end + + it 'does not trace redis spans without a parent' do + redis = redis_with_auth + redis.call('set', 'a', 'b') + + _(exporter.finished_spans.size).must_equal 0 + end + end + + describe 'when db_statement is :omit' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :omit) + end + + it 'omits both db.statement and db.query.text attributes' do + redis = redis_with_auth + _(redis.call('set', 'K', 'xyz')).must_equal 'OK' + _(redis.call('get', 'K')).must_equal 'xyz' + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[0] + _(set_span.name).must_equal 'PIPELINED' # AUTH + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.statement') + _(set_span.attributes).wont_include('db.query.text') + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.statement') + _(set_span.attributes).wont_include('db.query.text') + + set_span = exporter.finished_spans[2] + _(set_span.name).must_equal 'GET' + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.statement') + _(set_span.attributes).wont_include('db.query.text') + end + end + + describe 'when db_statement is :obfuscate' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :obfuscate) + end + + it 'obfuscates arguments in both db.statement and db.query.text' do + redis = redis_with_auth + _(redis.call('set', 'K', 'xyz')).must_equal 'OK' + _(redis.call('get', 'K')).must_equal 'xyz' + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[0] + _(set_span.name).must_equal 'PIPELINED' + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.statement']).must_equal('HELLO ? ? ? ?') + _(set_span.attributes['db.query.text']).must_equal('HELLO ? ? ? ?') + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.statement']).must_equal('SET ? ?') + _(set_span.attributes['db.query.text']).must_equal('SET ? ?') + + set_span = exporter.finished_spans[2] + _(set_span.name).must_equal 'GET' + _(set_span.attributes['db.system']).must_equal 'redis' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.statement']).must_equal('GET ?') + _(set_span.attributes['db.query.text']).must_equal('GET ?') + end + end + end +end unless ENV['OMIT_SERVICES'] diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb new file mode 100644 index 0000000000..2b210fc887 --- /dev/null +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb @@ -0,0 +1,368 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +require_relative '../../../../../lib/opentelemetry/instrumentation/redis' +require_relative '../../../../../lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client' + +# Tests for stable semantic convention attributes via RedisClient middleware +describe OpenTelemetry::Instrumentation::Redis::Middlewares::Stable::RedisClientInstrumentation do + let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } + let(:exporter) { EXPORTER } + let(:password) { 'passw0rd' } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } + let(:last_span) { exporter.finished_spans.last } + + # Instantiate the Redis client with the correct password. Note that this + # will generate one extra span on connect because the Redis client will + # send an AUTH command before doing anything else. + def redis_with_auth(redis_options = {}) + redis_options[:password] ||= password + redis_options[:host] ||= redis_host + redis_options[:port] ||= redis_port + RedisClient.new(**redis_options).tap do |client| + client.send(:raw_connection) # force lazy client to connect + end + end + + before do + skip unless ENV['BUNDLE_GEMFILE']&.include?('stable') + + ENV['OTEL_SEMCONV_STABILITY_OPT_IN'] = 'database' + config = { db_statement: :include } + instrumentation.install(config) + exporter.reset + end + + # Force re-install of instrumentation + after { instrumentation.instance_variable_set(:@installed, false) } + + describe '#process' do + it 'before request' do + _(exporter.finished_spans.size).must_equal 0 + end + + it 'accepts peer service name from config' do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(peer_service: 'readonly:redis') + redis_with_auth + + _(last_span.attributes['peer.service']).must_equal 'readonly:redis' + end + + it 'context attributes take priority' do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(peer_service: 'readonly:redis') + redis = redis_with_auth + + OpenTelemetry::Instrumentation::Redis.with_attributes('peer.service' => 'foo') do + redis.call('set', 'K', 'x') + end + + _(last_span.attributes['peer.service']).must_equal 'foo' + end + + it 'after authorization with Redis server uses stable attributes' do + client = redis_with_auth + + _(client.connected?).must_equal(true) + + _(last_span.name).must_equal 'PIPELINED' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'HELLO ? ? ? ?' + _(last_span.attributes['server.address']).must_equal redis_host + # server.port only included if non-default (6379) + if redis_port == 6379 + _(last_span.attributes['server.port']).must_be_nil + else + _(last_span.attributes['server.port']).must_equal redis_port + end + # Old attributes should NOT be present + _(last_span.attributes).wont_include('db.system') + _(last_span.attributes).wont_include('db.statement') + _(last_span.attributes).wont_include('net.peer.name') + _(last_span.attributes).wont_include('net.peer.port') + end + + it 'after calling auth lowercase' do + client = redis_with_auth + client.call('auth', password) + + _(last_span.name).must_equal 'AUTH' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'AUTH ?' + _(last_span.attributes['server.address']).must_equal redis_host + end + + it 'after calling AUTH uppercase' do + client = redis_with_auth + client.call('AUTH', password) + + _(last_span.name).must_equal 'AUTH' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal 'AUTH ?' + _(last_span.attributes['server.address']).must_equal redis_host + end + + it 'after requests' do + redis = redis_with_auth + _(redis.call('set', 'K', 'x')).must_equal 'OK' + _(redis.call('get', 'K')).must_equal 'x' + + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('SET K x') + _(set_span.attributes['server.address']).must_equal redis_host + + get_span = exporter.finished_spans.last + _(get_span.name).must_equal 'GET' + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.query.text']).must_equal 'GET K' + _(get_span.attributes['server.address']).must_equal redis_host + end + + it 'reflects db index' do + redis = redis_with_auth(db: 1) + redis.call('get', 'K') + + _(exporter.finished_spans.size).must_equal 2 + + prelude_span = exporter.finished_spans.first + _(prelude_span.name).must_equal 'PIPELINED' + _(prelude_span.attributes['db.system.name']).must_equal 'redis' + _(prelude_span.attributes['db.query.text']).must_equal("HELLO ? ? ? ?\nSELECT 1") + _(prelude_span.attributes['server.address']).must_equal redis_host + + get_span = exporter.finished_spans.last + _(get_span.name).must_equal 'GET' + _(get_span.attributes['db.system.name']).must_equal 'redis' + _(get_span.attributes['db.query.text']).must_equal('GET K') + _(get_span.attributes['db.redis.database_index']).must_equal 1 + _(get_span.attributes['server.address']).must_equal redis_host + end + + it 'merges context attributes' do + redis = redis_with_auth + OpenTelemetry::Instrumentation::Redis.with_attributes('peer.service' => 'foo') do + redis.call('set', 'K', 'x') + end + + _(exporter.finished_spans.size).must_equal 2 + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal('SET K x') + _(set_span.attributes['peer.service']).must_equal 'foo' + _(set_span.attributes['server.address']).must_equal redis_host + end + + it 'records exceptions with error.type' do + expect do + redis = redis_with_auth + redis.call 'THIS_IS_NOT_A_REDIS_FUNC', 'THIS_IS_NOT_A_VALID_ARG' + end.must_raise RedisClient::CommandError + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'THIS_IS_NOT_A_REDIS_FUNC' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal( + 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' + ) + _(last_span.attributes['server.address']).must_equal redis_host + _(last_span.attributes['error.type']).must_equal 'RedisClient::CommandError' + _(last_span.status.code).must_equal( + OpenTelemetry::Trace::Status::ERROR + ) + end + + it 'connect is uninstrumented' do + error = _ { redis_with_auth(host: 'example.com', port: 8321, timeout: 0.01) }.must_raise StandardError + # Ruby 4 changed the timeout error class + # Prior to that the client library would wrap the timeout in a RedisClient::CannotConnectError + _([IO::TimeoutError, RedisClient::CannotConnectError]).must_include error.class + + _(last_span).must_be_nil + end + + it 'traces pipelined commands' do + redis = redis_with_auth + redis.pipelined do |r| + r.call('set', 'v1', '0') + r.call('incr', 'v1') + r.call('get', 'v1') + end + + _(exporter.finished_spans.size).must_equal 2 + _(last_span.name).must_equal 'PIPELINED' + _(last_span.attributes['db.system.name']).must_equal 'redis' + _(last_span.attributes['db.query.text']).must_equal "SET v1 0\nINCR v1\nGET v1" + _(last_span.attributes['server.address']).must_equal redis_host + end + + it 'records floats' do + redis = redis_with_auth + redis.call('hmset', 'hash', 'f1', 1_234_567_890.0987654321) + + _(last_span.name).must_equal 'HMSET' + _(last_span.attributes['db.query.text']).must_equal 'HMSET hash f1 1234567890.0987654' + end + + it 'records empty string' do + redis = redis_with_auth + redis.call('set', 'K', '') + + _(last_span.name).must_equal 'SET' + _(last_span.attributes['db.query.text']).must_equal 'SET K ' + end + + it 'truncates long db.query.text' do + redis = redis_with_auth + the_long_value = 'y' * 100 + redis.pipelined do |pipeline| + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + pipeline.call(:set, 'v1', the_long_value) + end + + expected_query_text = <<~HEREDOC.chomp + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy + SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy... + HEREDOC + + _(last_span.name).must_equal 'PIPELINED' + _(last_span.attributes['db.query.text'].size).must_equal 500 + _(last_span.attributes['db.query.text']).must_equal expected_query_text + end + + it 'encodes invalid byte sequences for db.query.text' do + redis = redis_with_auth + + # \255 is off-limits https://en.wikipedia.org/wiki/UTF-8#Codepage_layout + redis.call('set', 'K', "x\255") + + _(last_span.name).must_equal 'SET' + _(last_span.attributes['db.query.text']).must_equal 'SET K x' + end + + it 'records server.port for non-default port' do + client = redis_with_auth + client.call('set', 'K', 'x') + + set_span = exporter.finished_spans[1] + _(set_span.attributes['server.address']).must_equal redis_host + # server.port should only be present if non-default + if redis_port != 6379 + _(set_span.attributes['server.port']).must_equal redis_port + else + _(set_span.attributes['server.port']).must_be_nil + end + end + + describe 'when trace_root_spans is disabled' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(trace_root_spans: false) + end + + it 'traces redis spans with a parent' do + redis = redis_with_auth + OpenTelemetry.tracer_provider.tracer('tester').in_span('a root!') do + redis.call('set', 'a', 'b') + end + + redis_span = exporter.finished_spans.find { |s| s.name == 'SET' } + _(redis_span.name).must_equal 'SET' + _(redis_span.attributes['db.query.text']).must_equal 'SET ? ?' + end + + it 'does not trace redis spans without a parent' do + redis = redis_with_auth + redis.call('set', 'a', 'b') + + _(exporter.finished_spans.size).must_equal 0 + end + end + + describe 'when db_statement is :omit' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :omit) + end + + it 'omits db.query.text attribute' do + redis = redis_with_auth + _(redis.call('set', 'K', 'xyz')).must_equal 'OK' + _(redis.call('get', 'K')).must_equal 'xyz' + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[0] + _(set_span.name).must_equal 'PIPELINED' # AUTH + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + + set_span = exporter.finished_spans[2] + _(set_span.name).must_equal 'GET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes).wont_include('db.query.text') + end + end + + describe 'when db_statement is :obfuscate' do + before do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(db_statement: :obfuscate) + end + + it 'obfuscates arguments in db.query.text' do + redis = redis_with_auth + _(redis.call('set', 'K', 'xyz')).must_equal 'OK' + _(redis.call('get', 'K')).must_equal 'xyz' + _(exporter.finished_spans.size).must_equal 3 + + set_span = exporter.finished_spans[0] + _(set_span.name).must_equal 'PIPELINED' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal( + 'HELLO ? ? ? ?' + ) + + set_span = exporter.finished_spans[1] + _(set_span.name).must_equal 'SET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal( + 'SET ? ?' + ) + + set_span = exporter.finished_spans[2] + _(set_span.name).must_equal 'GET' + _(set_span.attributes['db.system.name']).must_equal 'redis' + _(set_span.attributes['db.query.text']).must_equal( + 'GET ?' + ) + end + end + end +end unless ENV['OMIT_SERVICES'] From 451caa09116187d759096e229007f23debe02f6d Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Mar 2026 13:44:19 -0700 Subject: [PATCH 3/6] Update test --- .../instrumentation/redis/stable/redis_client_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb index 2b210fc887..2309bd4dbe 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb @@ -269,10 +269,10 @@ def redis_with_auth(redis_options = {}) set_span = exporter.finished_spans[1] _(set_span.attributes['server.address']).must_equal redis_host # server.port should only be present if non-default - if redis_port != 6379 - _(set_span.attributes['server.port']).must_equal redis_port - else + if redis_port == 6379 _(set_span.attributes['server.port']).must_be_nil + else + _(set_span.attributes['server.port']).must_equal redis_port end end From a3eb9c25c690fa94ecf8d6301038ca27e370738d Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Wed, 18 Mar 2026 08:33:04 -0700 Subject: [PATCH 4/6] Update semconv --- .../redis/middlewares/dup/redis_client.rb | 42 ++++++++++++++++--- .../redis/middlewares/stable/redis_client.rb | 39 +++++++++++++---- .../redis/patches/dup/redis_v4_client.rb | 29 +++++++++++-- .../redis/patches/stable/redis_v4_client.rb | 25 +++++++++-- .../instrumentation/redis/dup/client_test.rb | 18 ++++++-- .../redis/dup/redis_client_test.rb | 18 ++++---- .../redis/stable/client_test.rb | 24 ++++++----- .../redis/stable/redis_client_test.rb | 18 ++++---- 8 files changed, 162 insertions(+), 51 deletions(-) diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb index 06e36bfdff..e23e2f8f2e 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb @@ -20,7 +20,9 @@ module RedisClientInstrumentation def call(command, redis_config) return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + op_name = command[0].to_s.upcase attributes = span_attributes(redis_config) + attributes['db.operation.name'] = op_name unless instrumentation.config[:db_statement] == :omit serialized = serialize_commands([command]) @@ -29,11 +31,10 @@ def call(command, redis_config) attributes['db.query.text'] = serialized end - span_name = command[0].to_s.upcase - instrumentation.tracer.in_span(span_name, attributes: attributes, kind: :client) do |span| + instrumentation.tracer.in_span(op_name, attributes: attributes, kind: :client) do |span| super rescue StandardError => e - span.set_attribute('error.type', e.class.name) + set_error_attributes(span, e) raise end end @@ -42,6 +43,8 @@ def call_pipelined(commands, redis_config) return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? attributes = span_attributes(redis_config) + attributes['db.operation.name'] = 'PIPELINE' + attributes['db.operation.batch.size'] = commands.size unless instrumentation.config[:db_statement] == :omit serialized = serialize_commands(commands) @@ -50,10 +53,10 @@ def call_pipelined(commands, redis_config) attributes['db.query.text'] = serialized end - instrumentation.tracer.in_span('PIPELINED', attributes: attributes, kind: :client) do |span| + instrumentation.tracer.in_span('PIPELINE', attributes: attributes, kind: :client) do |span| super rescue StandardError => e - span.set_attribute('error.type', e.class.name) + set_error_attributes(span, e) raise end end @@ -76,12 +79,39 @@ def span_attributes(redis_config) # Only add server.port if non-default attributes['server.port'] = port if port && port != Dup::REDIS_DEFAULT_PORT - attributes['db.redis.database_index'] = redis_config.db unless redis_config.db.zero? + unless redis_config.db.zero? + # Old convention + attributes['db.redis.database_index'] = redis_config.db + # New stable convention (db.namespace as string) + attributes['db.namespace'] = redis_config.db.to_s + end attributes['peer.service'] = instrumentation.config[:peer_service] if instrumentation.config[:peer_service] attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) attributes end + def set_error_attributes(span, error) + error_type = extract_error_type(error) + span.set_attribute('error.type', error_type) + span.set_attribute('db.response.status_code', error_type) if redis_error?(error) + span.record_exception(error) + span.status = OpenTelemetry::Trace::Status.error(error.message) + end + + def extract_error_type(error) + # Redis errors start with an error prefix like ERR, WRONGTYPE, CLUSTERDOWN + # Extract this prefix for db.response.status_code and error.type + if redis_error?(error) && error.message + prefix = error.message.split.first + return prefix if prefix && prefix == prefix.upcase && prefix.match?(/\A[A-Z]+\z/) + end + error.class.name + end + + def redis_error?(error) + error.is_a?(::RedisClient::CommandError) + end + def serialize_commands(commands) obfuscate = instrumentation.config[:db_statement] == :obfuscate diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb index 277cb786ac..9c68081498 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb @@ -20,15 +20,15 @@ module RedisClientInstrumentation def call(command, redis_config) return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? + op_name = command[0].to_s.upcase attributes = span_attributes(redis_config) - + attributes['db.operation.name'] = op_name attributes['db.query.text'] = serialize_commands([command]) unless instrumentation.config[:db_statement] == :omit - span_name = command[0].to_s.upcase - instrumentation.tracer.in_span(span_name, attributes: attributes, kind: :client) do |span| + instrumentation.tracer.in_span(op_name, attributes: attributes, kind: :client) do |span| super rescue StandardError => e - span.set_attribute('error.type', e.class.name) + set_error_attributes(span, e) raise end end @@ -37,13 +37,14 @@ def call_pipelined(commands, redis_config) return super unless instrumentation.config[:trace_root_spans] || OpenTelemetry::Trace.current_span.context.valid? attributes = span_attributes(redis_config) - + attributes['db.operation.name'] = 'PIPELINE' + attributes['db.operation.batch.size'] = commands.size attributes['db.query.text'] = serialize_commands(commands) unless instrumentation.config[:db_statement] == :omit - instrumentation.tracer.in_span('PIPELINED', attributes: attributes, kind: :client) do |span| + instrumentation.tracer.in_span('PIPELINE', attributes: attributes, kind: :client) do |span| super rescue StandardError => e - span.set_attribute('error.type', e.class.name) + set_error_attributes(span, e) raise end end @@ -60,12 +61,34 @@ def span_attributes(redis_config) port = redis_config.port attributes['server.port'] = port if port && port != Stable::REDIS_DEFAULT_PORT - attributes['db.redis.database_index'] = redis_config.db unless redis_config.db.zero? + attributes['db.namespace'] = redis_config.db.to_s unless redis_config.db.zero? attributes['peer.service'] = instrumentation.config[:peer_service] if instrumentation.config[:peer_service] attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) attributes end + def set_error_attributes(span, error) + error_type = extract_error_type(error) + span.set_attribute('error.type', error_type) + span.set_attribute('db.response.status_code', error_type) if redis_error?(error) + span.record_exception(error) + span.status = OpenTelemetry::Trace::Status.error(error.message) + end + + def extract_error_type(error) + # Redis errors start with an error prefix like ERR, WRONGTYPE, CLUSTERDOWN + # Extract this prefix for db.response.status_code and error.type + if redis_error?(error) && error.message + prefix = error.message.split.first + return prefix if prefix && prefix == prefix.upcase && prefix.match?(/\A[A-Z]+\z/) + end + error.class.name + end + + def redis_error?(error) + error.is_a?(::RedisClient::CommandError) + end + def serialize_commands(commands) obfuscate = instrumentation.config[:db_statement] == :obfuscate diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb index 2f0fc44d44..0345c71967 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb @@ -36,7 +36,12 @@ def process(commands) # Only add server.port if non-default attributes['server.port'] = port if port && port != Dup::REDIS_DEFAULT_PORT - attributes['db.redis.database_index'] = options[:db] unless options[:db].zero? + unless options[:db].zero? + # Old convention + attributes['db.redis.database_index'] = options[:db] + # New stable convention (db.namespace as string) + attributes['db.namespace'] = options[:db].to_s + end attributes['peer.service'] = instrumentation_config[:peer_service] if instrumentation_config[:peer_service] attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) @@ -50,15 +55,21 @@ def process(commands) end span_name = if commands.length == 1 - commands[0][0].to_s.upcase + op_name = commands[0][0].to_s.upcase + attributes['db.operation.name'] = op_name + op_name else - 'PIPELINED' + attributes['db.operation.name'] = 'PIPELINE' + attributes['db.operation.batch.size'] = commands.length + 'PIPELINE' end instrumentation_tracer.in_span(span_name, attributes: attributes, kind: :client) do |s| super.tap do |reply| if reply.is_a?(::Redis::CommandError) - s.set_attribute('error.type', reply.class.name) + error_type = extract_error_type(reply) + s.set_attribute('error.type', error_type) + s.set_attribute('db.response.status_code', error_type) s.record_exception(reply) s.status = Trace::Status.error(reply.message) end @@ -102,6 +113,16 @@ def instrumentation_tracer def instrumentation_config Redis::Instrumentation.instance.config end + + def extract_error_type(error) + # Redis errors start with an error prefix like ERR, WRONGTYPE, CLUSTERDOWN + # Extract this prefix for db.response.status_code and error.type + if error.message + prefix = error.message.split.first + return prefix if prefix && prefix == prefix.upcase && prefix.match?(/\A[A-Z]+\z/) + end + error.class.name + end end end end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb index 23756b9feb..db0ecb68a6 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb @@ -31,7 +31,8 @@ def process(commands) # Only add server.port if non-default attributes['server.port'] = port if port && port != Stable::REDIS_DEFAULT_PORT - attributes['db.redis.database_index'] = options[:db] unless options[:db].zero? + # db.namespace is the database index as a string (replaces db.redis.database_index in stable) + attributes['db.namespace'] = options[:db].to_s unless options[:db].zero? attributes['peer.service'] = instrumentation_config[:peer_service] if instrumentation_config[:peer_service] attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) @@ -43,15 +44,21 @@ def process(commands) end span_name = if commands.length == 1 - commands[0][0].to_s.upcase + op_name = commands[0][0].to_s.upcase + attributes['db.operation.name'] = op_name + op_name else - 'PIPELINED' + attributes['db.operation.name'] = 'PIPELINE' + attributes['db.operation.batch.size'] = commands.length + 'PIPELINE' end instrumentation_tracer.in_span(span_name, attributes: attributes, kind: :client) do |s| super.tap do |reply| if reply.is_a?(::Redis::CommandError) - s.set_attribute('error.type', reply.class.name) + error_type = extract_error_type(reply) + s.set_attribute('error.type', error_type) + s.set_attribute('db.response.status_code', error_type) s.record_exception(reply) s.status = Trace::Status.error(reply.message) end @@ -95,6 +102,16 @@ def instrumentation_tracer def instrumentation_config Redis::Instrumentation.instance.config end + + def extract_error_type(error) + # Redis errors start with an error prefix like ERR, WRONGTYPE, CLUSTERDOWN + # Extract this prefix for db.response.status_code and error.type + if error.message + prefix = error.message.split.first + return prefix if prefix && prefix == prefix.upcase && prefix.match?(/\A[A-Z]+\z/) + end + error.class.name + end end end end diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb index a97aed805e..1bfd2a827b 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb @@ -116,7 +116,9 @@ def redis_gte_5? _(select_span.attributes['db.query.text']).must_equal('SELECT 1') _(select_span.attributes['db.system']).must_equal 'redis' _(select_span.attributes['db.system.name']).must_equal 'redis' + # Both old and new namespace attributes _(select_span.attributes['db.redis.database_index']).must_equal 1 + _(select_span.attributes['db.namespace']).must_equal '1' get_span = exporter.finished_spans.last _(get_span.name).must_equal 'GET' @@ -125,6 +127,7 @@ def redis_gte_5? _(get_span.attributes['db.statement']).must_equal('GET K') _(get_span.attributes['db.query.text']).must_equal('GET K') _(get_span.attributes['db.redis.database_index']).must_equal 1 + _(get_span.attributes['db.namespace']).must_equal '1' end it 'reflects db index v5' do @@ -136,13 +139,15 @@ def redis_gte_5? _(exporter.finished_spans.size).must_equal 2 select_span = exporter.finished_spans.first get_span = exporter.finished_spans.last - _(select_span.name).must_equal 'PIPELINED' + _(select_span.name).must_equal 'PIPELINE' # Both attributes _(select_span.attributes['db.statement']).must_equal("AUTH ?\nSELECT 1") _(select_span.attributes['db.query.text']).must_equal("AUTH ?\nSELECT 1") _(select_span.attributes['db.system']).must_equal 'redis' _(select_span.attributes['db.system.name']).must_equal 'redis' + # Both old and new namespace attributes _(select_span.attributes['db.redis.database_index']).must_equal 1 + _(select_span.attributes['db.namespace']).must_equal '1' _(get_span.name).must_equal 'GET' _(get_span.attributes['db.system']).must_equal 'redis' @@ -150,6 +155,7 @@ def redis_gte_5? _(get_span.attributes['db.statement']).must_equal('GET K') _(get_span.attributes['db.query.text']).must_equal('GET K') _(get_span.attributes['db.redis.database_index']).must_equal 1 + _(get_span.attributes['db.namespace']).must_equal '1' end it 'records exceptions with error.type' do @@ -171,7 +177,9 @@ def redis_gte_5? _(last_span.attributes['db.query.text']).must_equal( 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' ) - _(last_span.attributes['error.type']).must_equal 'Redis::CommandError' + # Redis error prefix is extracted for error.type and db.response.status_code + _(last_span.attributes['error.type']).must_equal 'ERR' + _(last_span.attributes['db.response.status_code']).must_equal 'ERR' _(last_span.status.code).must_equal( OpenTelemetry::Trace::Status::ERROR ) @@ -196,7 +204,9 @@ def redis_gte_5? _(last_span.attributes['db.query.text']).must_equal( 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' ) - _(last_span.attributes['error.type']).must_equal 'RedisClient::CommandError' + # Redis error prefix is extracted for error.type and db.response.status_code + _(last_span.attributes['error.type']).must_equal 'ERR' + _(last_span.attributes['db.response.status_code']).must_equal 'ERR' _(last_span.status.code).must_equal( OpenTelemetry::Trace::Status::ERROR ) @@ -211,7 +221,7 @@ def redis_gte_5? end _(exporter.finished_spans.size).must_equal 2 - _(last_span.name).must_equal 'PIPELINED' + _(last_span.name).must_equal 'PIPELINE' # Both attributes _(last_span.attributes['db.system']).must_equal 'redis' _(last_span.attributes['db.system.name']).must_equal 'redis' diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb index 4a3602504d..9934c32f18 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb @@ -72,7 +72,7 @@ def redis_with_auth(redis_options = {}) _(client.connected?).must_equal(true) - _(last_span.name).must_equal 'PIPELINED' + _(last_span.name).must_equal 'PIPELINE' # Old attributes _(last_span.attributes['db.system']).must_equal 'redis' _(last_span.attributes['db.statement']).must_equal 'HELLO ? ? ? ?' @@ -156,7 +156,7 @@ def redis_with_auth(redis_options = {}) _(exporter.finished_spans.size).must_equal 2 prelude_span = exporter.finished_spans.first - _(prelude_span.name).must_equal 'PIPELINED' + _(prelude_span.name).must_equal 'PIPELINE' # Both old and new attributes _(prelude_span.attributes['db.system']).must_equal 'redis' _(prelude_span.attributes['db.system.name']).must_equal 'redis' @@ -171,7 +171,9 @@ def redis_with_auth(redis_options = {}) _(get_span.attributes['db.system.name']).must_equal 'redis' _(get_span.attributes['db.statement']).must_equal('GET K') _(get_span.attributes['db.query.text']).must_equal('GET K') + # Both old and new namespace attributes _(get_span.attributes['db.redis.database_index']).must_equal 1 + _(get_span.attributes['db.namespace']).must_equal '1' _(get_span.attributes['net.peer.name']).must_equal redis_host _(get_span.attributes['server.address']).must_equal redis_host end @@ -214,7 +216,9 @@ def redis_with_auth(redis_options = {}) ) _(last_span.attributes['net.peer.name']).must_equal redis_host _(last_span.attributes['server.address']).must_equal redis_host - _(last_span.attributes['error.type']).must_equal 'RedisClient::CommandError' + # Redis error prefix is extracted for error.type and db.response.status_code + _(last_span.attributes['error.type']).must_equal 'ERR' + _(last_span.attributes['db.response.status_code']).must_equal 'ERR' _(last_span.status.code).must_equal( OpenTelemetry::Trace::Status::ERROR ) @@ -238,7 +242,7 @@ def redis_with_auth(redis_options = {}) end _(exporter.finished_spans.size).must_equal 2 - _(last_span.name).must_equal 'PIPELINED' + _(last_span.name).must_equal 'PIPELINE' # Both old and new attributes _(last_span.attributes['db.system']).must_equal 'redis' _(last_span.attributes['db.system.name']).must_equal 'redis' @@ -289,7 +293,7 @@ def redis_with_auth(redis_options = {}) SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy... HEREDOC - _(last_span.name).must_equal 'PIPELINED' + _(last_span.name).must_equal 'PIPELINE' _(last_span.attributes['db.statement'].size).must_equal 500 _(last_span.attributes['db.statement']).must_equal expected_statement _(last_span.attributes['db.query.text'].size).must_equal 500 @@ -346,7 +350,7 @@ def redis_with_auth(redis_options = {}) _(exporter.finished_spans.size).must_equal 3 set_span = exporter.finished_spans[0] - _(set_span.name).must_equal 'PIPELINED' # AUTH + _(set_span.name).must_equal 'PIPELINE' # AUTH _(set_span.attributes['db.system']).must_equal 'redis' _(set_span.attributes['db.system.name']).must_equal 'redis' _(set_span.attributes).wont_include('db.statement') @@ -381,7 +385,7 @@ def redis_with_auth(redis_options = {}) _(exporter.finished_spans.size).must_equal 3 set_span = exporter.finished_spans[0] - _(set_span.name).must_equal 'PIPELINED' + _(set_span.name).must_equal 'PIPELINE' _(set_span.attributes['db.system']).must_equal 'redis' _(set_span.attributes['db.system.name']).must_equal 'redis' _(set_span.attributes['db.statement']).must_equal('HELLO ? ? ? ?') diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb index 4d58575fa1..f23b3ec0ef 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb @@ -101,13 +101,13 @@ def redis_gte_5? _(select_span.attributes['db.query.text']).must_equal('SELECT 1') _(select_span.attributes['db.system.name']).must_equal 'redis' _(select_span.attributes['server.address']).must_equal redis_host - _(select_span.attributes['db.redis.database_index']).must_equal 1 + _(select_span.attributes['db.namespace']).must_equal '1' get_span = exporter.finished_spans.last _(get_span.name).must_equal 'GET' _(get_span.attributes['db.system.name']).must_equal 'redis' _(get_span.attributes['db.query.text']).must_equal('GET K') - _(get_span.attributes['db.redis.database_index']).must_equal 1 + _(get_span.attributes['db.namespace']).must_equal '1' end it 'reflects db index v5' do @@ -119,16 +119,16 @@ def redis_gte_5? _(exporter.finished_spans.size).must_equal 2 select_span = exporter.finished_spans.first get_span = exporter.finished_spans.last - _(select_span.name).must_equal 'PIPELINED' + _(select_span.name).must_equal 'PIPELINE' _(select_span.attributes['db.query.text']).must_equal("AUTH ?\nSELECT 1") _(select_span.attributes['db.system.name']).must_equal 'redis' _(select_span.attributes['server.address']).must_equal redis_host - _(select_span.attributes['db.redis.database_index']).must_equal 1 + _(select_span.attributes['db.namespace']).must_equal '1' _(get_span.name).must_equal 'GET' _(get_span.attributes['db.system.name']).must_equal 'redis' _(get_span.attributes['db.query.text']).must_equal('GET K') - _(get_span.attributes['db.redis.database_index']).must_equal 1 + _(get_span.attributes['db.namespace']).must_equal '1' end it 'records exceptions' do @@ -146,7 +146,9 @@ def redis_gte_5? 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' ) _(last_span.attributes['server.address']).must_equal redis_host - _(last_span.attributes['error.type']).must_equal 'Redis::CommandError' + # Redis error prefix is extracted for error.type and db.response.status_code + _(last_span.attributes['error.type']).must_equal 'ERR' + _(last_span.attributes['db.response.status_code']).must_equal 'ERR' _(last_span.status.code).must_equal( OpenTelemetry::Trace::Status::ERROR ) @@ -167,7 +169,9 @@ def redis_gte_5? 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' ) _(last_span.attributes['server.address']).must_equal redis_host - _(last_span.attributes['error.type']).must_equal 'RedisClient::CommandError' + # Redis error prefix is extracted for error.type and db.response.status_code + _(last_span.attributes['error.type']).must_equal 'ERR' + _(last_span.attributes['db.response.status_code']).must_equal 'ERR' _(last_span.status.code).must_equal( OpenTelemetry::Trace::Status::ERROR ) @@ -182,7 +186,7 @@ def redis_gte_5? end _(exporter.finished_spans.size).must_equal 2 - _(last_span.name).must_equal 'PIPELINED' + _(last_span.name).must_equal 'PIPELINE' _(last_span.attributes['db.system.name']).must_equal 'redis' _(last_span.attributes['db.query.text']).must_equal "SET v1 0\nINCR v1\nGET v1" _(last_span.attributes['server.address']).must_equal redis_host @@ -240,7 +244,7 @@ def redis_gte_5? _(exporter.finished_spans.size).must_equal 3 set_span = exporter.finished_spans[0] - _(set_span.name).must_equal('PIPELINED') + _(set_span.name).must_equal('PIPELINE') _(set_span.attributes['db.system.name']).must_equal 'redis' _(set_span.attributes).wont_include('db.query.text') @@ -295,7 +299,7 @@ def redis_gte_5? _(exporter.finished_spans.size).must_equal 3 set_span = exporter.finished_spans[0] - _(set_span.name).must_equal('PIPELINED') + _(set_span.name).must_equal('PIPELINE') _(set_span.attributes['db.system.name']).must_equal 'redis' _(set_span.attributes['db.query.text']).must_equal('AUTH ?') diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb index 2309bd4dbe..557315bd85 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb @@ -72,7 +72,7 @@ def redis_with_auth(redis_options = {}) _(client.connected?).must_equal(true) - _(last_span.name).must_equal 'PIPELINED' + _(last_span.name).must_equal 'PIPELINE' _(last_span.attributes['db.system.name']).must_equal 'redis' _(last_span.attributes['db.query.text']).must_equal 'HELLO ? ? ? ?' _(last_span.attributes['server.address']).must_equal redis_host @@ -136,7 +136,7 @@ def redis_with_auth(redis_options = {}) _(exporter.finished_spans.size).must_equal 2 prelude_span = exporter.finished_spans.first - _(prelude_span.name).must_equal 'PIPELINED' + _(prelude_span.name).must_equal 'PIPELINE' _(prelude_span.attributes['db.system.name']).must_equal 'redis' _(prelude_span.attributes['db.query.text']).must_equal("HELLO ? ? ? ?\nSELECT 1") _(prelude_span.attributes['server.address']).must_equal redis_host @@ -145,7 +145,7 @@ def redis_with_auth(redis_options = {}) _(get_span.name).must_equal 'GET' _(get_span.attributes['db.system.name']).must_equal 'redis' _(get_span.attributes['db.query.text']).must_equal('GET K') - _(get_span.attributes['db.redis.database_index']).must_equal 1 + _(get_span.attributes['db.namespace']).must_equal '1' _(get_span.attributes['server.address']).must_equal redis_host end @@ -178,7 +178,9 @@ def redis_with_auth(redis_options = {}) 'THIS_IS_NOT_A_REDIS_FUNC THIS_IS_NOT_A_VALID_ARG' ) _(last_span.attributes['server.address']).must_equal redis_host - _(last_span.attributes['error.type']).must_equal 'RedisClient::CommandError' + # Redis error prefix is extracted for error.type and db.response.status_code + _(last_span.attributes['error.type']).must_equal 'ERR' + _(last_span.attributes['db.response.status_code']).must_equal 'ERR' _(last_span.status.code).must_equal( OpenTelemetry::Trace::Status::ERROR ) @@ -202,7 +204,7 @@ def redis_with_auth(redis_options = {}) end _(exporter.finished_spans.size).must_equal 2 - _(last_span.name).must_equal 'PIPELINED' + _(last_span.name).must_equal 'PIPELINE' _(last_span.attributes['db.system.name']).must_equal 'redis' _(last_span.attributes['db.query.text']).must_equal "SET v1 0\nINCR v1\nGET v1" _(last_span.attributes['server.address']).must_equal redis_host @@ -247,7 +249,7 @@ def redis_with_auth(redis_options = {}) SET v1 yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy... HEREDOC - _(last_span.name).must_equal 'PIPELINED' + _(last_span.name).must_equal 'PIPELINE' _(last_span.attributes['db.query.text'].size).must_equal 500 _(last_span.attributes['db.query.text']).must_equal expected_query_text end @@ -314,7 +316,7 @@ def redis_with_auth(redis_options = {}) _(exporter.finished_spans.size).must_equal 3 set_span = exporter.finished_spans[0] - _(set_span.name).must_equal 'PIPELINED' # AUTH + _(set_span.name).must_equal 'PIPELINE' # AUTH _(set_span.attributes['db.system.name']).must_equal 'redis' _(set_span.attributes).wont_include('db.query.text') @@ -343,7 +345,7 @@ def redis_with_auth(redis_options = {}) _(exporter.finished_spans.size).must_equal 3 set_span = exporter.finished_spans[0] - _(set_span.name).must_equal 'PIPELINED' + _(set_span.name).must_equal 'PIPELINE' _(set_span.attributes['db.system.name']).must_equal 'redis' _(set_span.attributes['db.query.text']).must_equal( 'HELLO ? ? ? ?' From 541416076c99356f9b9e0b46d1098dd6adee2741 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Wed, 25 Mar 2026 13:18:49 -0700 Subject: [PATCH 5/6] update tests --- .../redis/middlewares/dup/redis_client.rb | 6 +-- .../redis/middlewares/stable/redis_client.rb | 9 +--- .../redis/patches/dup/redis_v4_client.rb | 6 +-- .../redis/patches/stable/redis_v4_client.rb | 7 +-- .../instrumentation/redis/dup/client_test.rb | 2 +- .../redis/dup/redis_client_test.rb | 2 +- .../instrumentation/redis/old/client_test.rb | 2 +- .../redis/stable/client_test.rb | 11 ++--- .../redis/stable/redis_client_test.rb | 43 +++---------------- 9 files changed, 17 insertions(+), 71 deletions(-) diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb index e23e2f8f2e..ef950bbdb2 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/dup/redis_client.rb @@ -9,9 +9,6 @@ module Instrumentation module Redis module Middlewares module Dup - # Default Redis port used to determine whether to include server.port - REDIS_DEFAULT_PORT = 6379 - # Adapter for redis-client instrumentation interface module RedisClientInstrumentation MAX_STATEMENT_LENGTH = 500 @@ -76,8 +73,7 @@ def span_attributes(redis_config) # New stable conventions attributes['db.system.name'] = 'redis' attributes['server.address'] = redis_config.host - # Only add server.port if non-default - attributes['server.port'] = port if port && port != Dup::REDIS_DEFAULT_PORT + attributes['server.port'] = port if port unless redis_config.db.zero? # Old convention diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb index 9c68081498..ebb5841200 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/middlewares/stable/redis_client.rb @@ -9,9 +9,6 @@ module Instrumentation module Redis module Middlewares module Stable - # Default Redis port used to determine whether to include server.port - REDIS_DEFAULT_PORT = 6379 - # Adapter for redis-client instrumentation interface module RedisClientInstrumentation MAX_STATEMENT_LENGTH = 500 @@ -57,12 +54,8 @@ def span_attributes(redis_config) 'server.address' => redis_config.host } - # Only add server.port if non-default - port = redis_config.port - attributes['server.port'] = port if port && port != Stable::REDIS_DEFAULT_PORT - + attributes['server.port'] = redis_config.port if redis_config.port attributes['db.namespace'] = redis_config.db.to_s unless redis_config.db.zero? - attributes['peer.service'] = instrumentation.config[:peer_service] if instrumentation.config[:peer_service] attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) attributes end diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb index 0345c71967..d46c6b6c6d 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/dup/redis_v4_client.rb @@ -9,9 +9,6 @@ module Instrumentation module Redis module Patches module Dup - # Default Redis port used to determine whether to include server.port - REDIS_DEFAULT_PORT = 6379 - # Module to prepend to Redis::Client for instrumentation module RedisV4Client MAX_STATEMENT_LENGTH = 500 @@ -33,8 +30,7 @@ def process(commands) # New stable conventions attributes['db.system.name'] = 'redis' attributes['server.address'] = host - # Only add server.port if non-default - attributes['server.port'] = port if port && port != Dup::REDIS_DEFAULT_PORT + attributes['server.port'] = port if port unless options[:db].zero? # Old convention diff --git a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb index db0ecb68a6..0c2672b073 100644 --- a/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb +++ b/instrumentation/redis/lib/opentelemetry/instrumentation/redis/patches/stable/redis_v4_client.rb @@ -9,9 +9,6 @@ module Instrumentation module Redis module Patches module Stable - # Default Redis port used to determine whether to include server.port - REDIS_DEFAULT_PORT = 6379 - # Module to prepend to Redis::Client for instrumentation module RedisV4Client MAX_STATEMENT_LENGTH = 500 @@ -28,12 +25,10 @@ def process(commands) 'server.address' => host } - # Only add server.port if non-default - attributes['server.port'] = port if port && port != Stable::REDIS_DEFAULT_PORT + attributes['server.port'] = port if port # db.namespace is the database index as a string (replaces db.redis.database_index in stable) attributes['db.namespace'] = options[:db].to_s unless options[:db].zero? - attributes['peer.service'] = instrumentation_config[:peer_service] if instrumentation_config[:peer_service] attributes.merge!(OpenTelemetry::Instrumentation::Redis.attributes) unless instrumentation_config[:db_statement] == :omit diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb index 1bfd2a827b..46781206d8 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb @@ -14,7 +14,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_host) { ENV['TEST_REDIS_HOST'] } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb index 9934c32f18..f30dee4050 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb @@ -14,7 +14,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_host) { ENV['TEST_REDIS_HOST'] } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb index 52716389d1..d4a2c051b0 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb @@ -16,7 +16,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_host) { ENV['TEST_REDIS_HOST'] } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb index f23b3ec0ef..c7890944f6 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb @@ -14,7 +14,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_host) { ENV['TEST_REDIS_HOST'] } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } @@ -60,12 +60,7 @@ def redis_gte_5? _(last_span.attributes['db.system.name']).must_equal 'redis' _(last_span.attributes['db.query.text']).must_equal 'AUTH ?' _(last_span.attributes['server.address']).must_equal redis_host - # server.port only included if non-default (6379) - if redis_port == 6379 - _(last_span.attributes['server.port']).must_be_nil - else - _(last_span.attributes['server.port']).must_equal redis_port - end + _(last_span.attributes['server.port']).must_equal redis_port end it 'after requests' do @@ -192,7 +187,7 @@ def redis_gte_5? _(last_span.attributes['server.address']).must_equal redis_host end - it 'records server.address and server.port for non-default port' do + it 'records server.address and server.port' do skip if redis_gte_5? client = Redis.new(host: 'example.com', port: 8321, timeout: 0.01) diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb index 557315bd85..0490781508 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb @@ -14,7 +14,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } + let(:redis_host) { ENV['TEST_REDIS_HOST'] } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } @@ -47,26 +47,6 @@ def redis_with_auth(redis_options = {}) _(exporter.finished_spans.size).must_equal 0 end - it 'accepts peer service name from config' do - instrumentation.instance_variable_set(:@installed, false) - instrumentation.install(peer_service: 'readonly:redis') - redis_with_auth - - _(last_span.attributes['peer.service']).must_equal 'readonly:redis' - end - - it 'context attributes take priority' do - instrumentation.instance_variable_set(:@installed, false) - instrumentation.install(peer_service: 'readonly:redis') - redis = redis_with_auth - - OpenTelemetry::Instrumentation::Redis.with_attributes('peer.service' => 'foo') do - redis.call('set', 'K', 'x') - end - - _(last_span.attributes['peer.service']).must_equal 'foo' - end - it 'after authorization with Redis server uses stable attributes' do client = redis_with_auth @@ -76,17 +56,13 @@ def redis_with_auth(redis_options = {}) _(last_span.attributes['db.system.name']).must_equal 'redis' _(last_span.attributes['db.query.text']).must_equal 'HELLO ? ? ? ?' _(last_span.attributes['server.address']).must_equal redis_host - # server.port only included if non-default (6379) - if redis_port == 6379 - _(last_span.attributes['server.port']).must_be_nil - else - _(last_span.attributes['server.port']).must_equal redis_port - end + _(last_span.attributes['server.port']).must_equal redis_port # Old attributes should NOT be present _(last_span.attributes).wont_include('db.system') _(last_span.attributes).wont_include('db.statement') _(last_span.attributes).wont_include('net.peer.name') _(last_span.attributes).wont_include('net.peer.port') + _(last_span.attributes).wont_include('peer.service') end it 'after calling auth lowercase' do @@ -151,7 +127,7 @@ def redis_with_auth(redis_options = {}) it 'merges context attributes' do redis = redis_with_auth - OpenTelemetry::Instrumentation::Redis.with_attributes('peer.service' => 'foo') do + OpenTelemetry::Instrumentation::Redis.with_attributes('custom.attribute' => 'foo') do redis.call('set', 'K', 'x') end @@ -161,7 +137,7 @@ def redis_with_auth(redis_options = {}) _(set_span.name).must_equal 'SET' _(set_span.attributes['db.system.name']).must_equal 'redis' _(set_span.attributes['db.query.text']).must_equal('SET K x') - _(set_span.attributes['peer.service']).must_equal 'foo' + _(set_span.attributes['custom.attribute']).must_equal 'foo' _(set_span.attributes['server.address']).must_equal redis_host end @@ -264,18 +240,13 @@ def redis_with_auth(redis_options = {}) _(last_span.attributes['db.query.text']).must_equal 'SET K x' end - it 'records server.port for non-default port' do + it 'records server.port' do client = redis_with_auth client.call('set', 'K', 'x') set_span = exporter.finished_spans[1] _(set_span.attributes['server.address']).must_equal redis_host - # server.port should only be present if non-default - if redis_port == 6379 - _(set_span.attributes['server.port']).must_be_nil - else - _(set_span.attributes['server.port']).must_equal redis_port - end + _(set_span.attributes['server.port']).must_equal redis_port end describe 'when trace_root_spans is disabled' do From 8f14f96db131abba94f3be2fa3492e859370a6a7 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Wed, 25 Mar 2026 13:26:00 -0700 Subject: [PATCH 6/6] Rubocop --- .../test/opentelemetry/instrumentation/redis/dup/client_test.rb | 2 +- .../instrumentation/redis/dup/redis_client_test.rb | 2 +- .../test/opentelemetry/instrumentation/redis/old/client_test.rb | 2 +- .../opentelemetry/instrumentation/redis/stable/client_test.rb | 2 +- .../instrumentation/redis/stable/redis_client_test.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb index 46781206d8..1bfd2a827b 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/client_test.rb @@ -14,7 +14,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV['TEST_REDIS_HOST'] } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb index f30dee4050..9934c32f18 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/dup/redis_client_test.rb @@ -14,7 +14,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV['TEST_REDIS_HOST'] } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb index d4a2c051b0..52716389d1 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/old/client_test.rb @@ -16,7 +16,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV['TEST_REDIS_HOST'] } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb index c7890944f6..4dc5af30f6 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/client_test.rb @@ -14,7 +14,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV['TEST_REDIS_HOST'] } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last } diff --git a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb index 0490781508..a6877d680f 100644 --- a/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb +++ b/instrumentation/redis/test/opentelemetry/instrumentation/redis/stable/redis_client_test.rb @@ -14,7 +14,7 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Redis::Instrumentation.instance } let(:exporter) { EXPORTER } let(:password) { 'passw0rd' } - let(:redis_host) { ENV['TEST_REDIS_HOST'] } + let(:redis_host) { ENV.fetch('TEST_REDIS_HOST', nil) } let(:redis_port) { ENV['TEST_REDIS_PORT'].to_i } let(:last_span) { exporter.finished_spans.last }