From a7456668d95f1e07f5f8ea16b49e53dfe75cc0ec Mon Sep 17 00:00:00 2001 From: zvkemp Date: Wed, 18 Dec 2024 17:20:53 -0500 Subject: [PATCH] wip --- .../lib/opentelemetry/instrumentation/base.rb | 38 +++++++++++++++--- .../sidekiq/instrumentation.rb | 13 ++++++ .../middlewares/client/tracer_middleware.rb | 40 +++++++++++++------ 3 files changed, 73 insertions(+), 18 deletions(-) diff --git a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb index bcaeff2cb..dc2abd3ba 100644 --- a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb +++ b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb @@ -208,7 +208,8 @@ def initialize(name, version, install_blk, present_blk, @installed = false @options = options @tracer = OpenTelemetry::Trace::Tracer.new # default no-op tracer - @meter = OpenTelemetry::Metrics::Meter.new # default no-op meter + + @meter = OpenTelemetry::Metrics::Meter.new if defined?(OpenTelemetry::Meter) # default no-op meter end # rubocop:enable Metrics/ParameterLists @@ -221,15 +222,15 @@ def install(config = {}) return true if installed? @config = config_options(config) + + set_metrics_enabled + return false unless installable?(config) instance_exec(@config, &@install_blk) @tracer = OpenTelemetry.tracer_provider.tracer(name, version) - + @meter = OpenTelemetry.meter_provider.meter(name, version: version) if metrics_enabled? @installed = true - - return unless config[:metrics] - @meter = OpenTelemetry.meter_provider.meter(name, version: version) end # Whether or not this instrumentation is installable in the current process. Will @@ -269,8 +270,25 @@ def enabled?(config = nil) true end + # This is based on a variety of factors, and should be invalidated when @config changes. + # It should be explicitly set via `set_metrics_enabled` for now. + def metrics_enabled? + !!@metrics_enabled + end + + # @api private + def with_meter + yield @meter if metrics_enabled? + end + private + def set_metrics_enabled + return unless defined?(OpenTelemetry::Metrics) + + @metrics_enabled = !!@config[:metrics] || metrics_enabled_by_env_var? + end + # The config_options method is responsible for validating that the user supplied # config hash is valid. # Unknown configuration keys are not included in the final config hash. @@ -334,6 +352,16 @@ def enabled_by_env_var? ENV[var_name] != 'false' end + def metrics_enabled_by_env_var? + var_name = name.dup + var_name.upcase! + var_name.gsub!('::', '_') + var_name.gsub!('OPENTELEMETRY_', 'OTEL_RUBY_') + var_name << 'METRICS_ENABLED' + + ENV[var_name] != 'false' + end + # Checks to see if the user has passed any environment variables that set options # for instrumentation. By convention, the environment variable will be the name # of the instrumentation, uppercased, with '::' replaced by underscores, diff --git a/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/instrumentation.rb b/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/instrumentation.rb index 38ab2eb61..eff8e8e29 100644 --- a/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/instrumentation.rb +++ b/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/instrumentation.rb @@ -109,8 +109,21 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base option :peer_service, default: nil, validate: :string option :metrics, default: false, validate: :boolean + # FIXME: upstream + def get_counter(name, description: nil) + return unless metrics_enabled? + + binding.pry + # FIXME: structural keys + # FIXME: mutex counter creation (& reads?) + INSTRUMENTS[[name, description]] ||= meter.create_counter(name, description: description) + end + private + # FIXME: upstream + INSTRUMENTS = {} + def gem_version Gem::Version.new(::Sidekiq::VERSION) end diff --git a/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/middlewares/client/tracer_middleware.rb b/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/middlewares/client/tracer_middleware.rb index 821b4d74c..99b39b077 100644 --- a/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/middlewares/client/tracer_middleware.rb +++ b/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/middlewares/client/tracer_middleware.rb @@ -35,36 +35,50 @@ def call(_worker_class, job, _queue, _redis_pool) yield end.tap do # FIXME: is it possible to detect failures here? Does sidekiq bubble them up the middlewares? - if instrumentation_config[:metrics] - begin - counter_attributes = { - 'messaging.operation.name' => 'enqueue', # FIXME: metrics semconv - 'messaging.system' => 'sidekiq', # FIXME: metrics semconv - 'messaging.destination.name' => job['queue'] # FIXME: metrics semconv + with_meter do |meter| + counter_attributes = metrics_attributes(job).merge( + { + 'messaging.operation.name' => 'enqueue' # FIXME: metrics semconv # server.address => # FIXME: required if available # messaging.destination.partition.id => FIXME: recommended # server.port => # FIXME: recommended } + ) - counter = meter.create_counter('messaging.client.sent.messages') - counter.add(1, attributes: counter_attributes) - end + # FIXME: avoid create_counter repetition? + binding.pry + counter = instrumentation.get_counter('messaging.client.sent.messages') + counter.add(1, attributes: counter_attributes) end end end private + def instrumentation + Sidekiq::Instrumentation.instance + end + def instrumentation_config - Sidekiq::Instrumentation.instance.config + instrumentation.config end def tracer - Sidekiq::Instrumentation.instance.tracer + instrumentation.tracer end - def meter - Sidekiq::Instrumentation.instance.meter + def with_meter(&block) + instrumentation.with_meter(&block) + end + + def metrics_attributes(job) + { + 'messaging.system' => 'sidekiq', # FIXME: metrics semconv + 'messaging.destination.name' => job['queue'] # FIXME: metrics semconv + # server.address => # FIXME: required if available + # messaging.destination.partition.id => FIXME: recommended + # server.port => # FIXME: recommended + } end end end