diff --git a/e2e/ruby/rails/demo/test/integration/observability_instrumentation_test.rb b/e2e/ruby/rails/demo/test/integration/observability_instrumentation_test.rb new file mode 100644 index 0000000000..9c79a41206 --- /dev/null +++ b/e2e/ruby/rails/demo/test/integration/observability_instrumentation_test.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'test_helper' + +# End-to-end coverage for the LaunchDarkly observability plugin's Rails +# auto-instrumentation. +# +# Background: the plugin configures OpenTelemetry from `Plugin#register`, which +# runs when the LaunchDarkly client is created. In this app that happens in +# `config/initializers/launchdarkly.rb` — i.e. DURING Rails boot — so the OTel +# Rails-family instrumentations (Rack, ActionPack, ActiveRecord, ...) install +# correctly. A customer who instead creates the client lazily AFTER boot sees +# "Instrumentation: OpenTelemetry::Instrumentation::ActionPack failed to install" +# because the ActiveSupport.on_load hooks those instrumentations rely on have +# already fired. These tests pin the working boot-time behavior so a regression +# (or a change that breaks instrumentation install) is caught in CI. +class ObservabilityInstrumentationTest < ActionDispatch::IntegrationTest + # The Rails-family instrumentations that must attach during a boot-time init. + # These are exactly the ones that report "failed to install" on the lazy path. + RAILS_INSTRUMENTATIONS = %w[Rack ActionPack ActiveRecord ActiveSupport Rails].freeze + + def instrumentation_instance(name) + Object.const_get("OpenTelemetry::Instrumentation::#{name}::Instrumentation").instance + end + + test 'rails auto-instrumentation installed during boot' do + RAILS_INSTRUMENTATIONS.each do |name| + assert instrumentation_instance(name).installed?, + "#{name} instrumentation should be installed after a boot-time plugin init " \ + '(it reports "failed to install" when the client is created lazily after boot)' + end + end + + test 'http request produces a server span via the rack instrumentation' do + exporter = OpenTelemetry::SDK::Trace::Export::InMemorySpanExporter.new + processor = OpenTelemetry::SDK::Trace::Export::SimpleSpanProcessor.new(exporter) + OpenTelemetry.tracer_provider.add_span_processor(processor) + + get pages_home_url + assert_response :success + + server_spans = exporter.finished_spans.select { |s| s.kind == :server } + refute_empty server_spans, 'expected an HTTP server span from the Rack/ActionPack instrumentation' + end +end diff --git a/e2e/ruby/rails/demo/test/test_helper.rb b/e2e/ruby/rails/demo/test/test_helper.rb index 0c92e8e881..967eed2a76 100644 --- a/e2e/ruby/rails/demo/test/test_helper.rb +++ b/e2e/ruby/rails/demo/test/test_helper.rb @@ -1,6 +1,14 @@ # frozen_string_literal: true ENV['RAILS_ENV'] ||= 'test' + +# The observability plugin only configures OpenTelemetry (and installs the Rails +# auto-instrumentation) when the LaunchDarkly client registers it, which requires +# a non-empty SDK key. Set a dummy key BEFORE the app boots so the instrumentation +# attaches during initialization. The key is invalid, so the client never connects +# (background connection attempts fail gracefully and do not affect tests). +ENV['LAUNCHDARKLY_SDK_KEY'] ||= 'sdk-test-0000000000000000000000' + require_relative '../config/environment' require 'rails/test_help' diff --git a/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability.rb b/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability.rb index dec63a7c40..f1f421d83a 100644 --- a/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability.rb +++ b/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability.rb @@ -13,7 +13,13 @@ require_relative 'launchdarkly_observability/middleware' require_relative 'launchdarkly_observability/otel_log_bridge' -require_relative 'launchdarkly_observability/rails' + +# NOTE: rails.rb is required at the *bottom* of this file, after the +# LaunchDarklyObservability module body has been fully defined. Its Railtie +# registers a `config.after_initialize` hook that runs synchronously when the +# gem is required lazily after Rails has booted. That hook references module +# constants and `class << self` methods, so they must already exist when the +# require runs. See the require at the end of this file. module LaunchDarklyObservability # Default OTLP endpoint for LaunchDarkly Observability @@ -179,3 +185,9 @@ def otel_logger_provider_available? end end end + +# Required last, on purpose: the Rails Railtie's `config.after_initialize` hook +# can run synchronously during this require (lazy require after Rails has +# booted), and it depends on the fully-defined LaunchDarklyObservability module +# above. See the note next to the other require_relative calls at the top. +require_relative 'launchdarkly_observability/rails' diff --git a/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/hook.rb b/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/hook.rb index 630336b501..f0709a5de2 100644 --- a/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/hook.rb +++ b/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/hook.rb @@ -91,12 +91,23 @@ def build_before_attributes(series_context) FEATURE_FLAG_PROVIDER_NAME => 'LaunchDarkly' } - context = series_context.context - if context - attrs[FEATURE_FLAG_CONTEXT_ID] = context.fully_qualified_key - end + attrs[FEATURE_FLAG_CONTEXT_ID] = context_id_for(series_context) - attrs + # OpenTelemetry rejects nil attribute values and logs an error for each + # one. Drop nils centrally so individual call sites cannot reintroduce the + # "invalid attribute value type NilClass" noise (e.g. an invalid context + # whose fully_qualified_key is nil). + attrs.compact + end + + # Returns the fully-qualified context key, or nil if unavailable. + # + # An invalid context (e.g. a non-string context kind) is still a non-nil + # object but yields a nil fully_qualified_key. OpenTelemetry rejects nil + # attribute values ("invalid attribute value type NilClass"), so callers + # must omit the attribute entirely when this returns nil. + def context_id_for(series_context) + series_context.context&.fully_qualified_key end def serialize_value(value) @@ -127,10 +138,7 @@ def add_feature_flag_event(span, series_context, detail) FEATURE_FLAG_PROVIDER_NAME => 'LaunchDarkly' } - context = series_context.context - if context - event_attributes[FEATURE_FLAG_CONTEXT_ID] = context.fully_qualified_key - end + event_attributes[FEATURE_FLAG_CONTEXT_ID] = context_id_for(series_context) if detail.variation_index event_attributes[FEATURE_FLAG_RESULT_VARIANT] = detail.variation_index.to_s @@ -146,7 +154,9 @@ def add_feature_flag_event(span, series_context, detail) add_reason_event_details(event_attributes, reason) end - span.add_event(FEATURE_FLAG_EVENT_NAME, attributes: event_attributes) + # See build_before_attributes: drop nil values so a missing context id (or + # any future optional attribute) cannot emit a nil-attribute OTel error. + span.add_event(FEATURE_FLAG_EVENT_NAME, attributes: event_attributes.compact) end def add_reason_event_details(event_attributes, reason) diff --git a/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/opentelemetry_config.rb b/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/opentelemetry_config.rb index 7c0603e2b3..4ad8215e5b 100644 --- a/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/opentelemetry_config.rb +++ b/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/opentelemetry_config.rb @@ -105,9 +105,16 @@ def configure_traces # User-provided instrumentation config is merged on top of defaults, # so users only need to specify the instrumentations they want to override. def configure_instrumentations(config) + # Only pass options that the instrumentations actually accept. Unknown + # options are not fatal but emit a warning on every boot ("ignored the + # following unknown configuration options [...]"): + # - `enable_recognize_route` is not an option on the Rails, Rack, or + # ActionPack instrumentations; route-based span naming (http.route) is + # handled automatically by the ActionPack instrumentation. + # - ActiveRecord has no `db_statement` option; SQL capture comes from the + # database adapter instrumentations (Mysql2, PG, ...) which default to + # obfuscating statements. defaults = { - 'OpenTelemetry::Instrumentation::Rails' => { enable_recognize_route: true }, - 'OpenTelemetry::Instrumentation::ActiveRecord' => { db_statement: :include }, 'OpenTelemetry::Instrumentation::Net::HTTP' => { untraced_hosts: [] }, 'OpenTelemetry::Instrumentation::Rack' => { untraced_endpoints: ['/health', '/healthz', '/ready'] } } diff --git a/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/rails.rb b/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/rails.rb index fa50f2d389..c6238dd549 100644 --- a/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/rails.rb +++ b/sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/rails.rb @@ -127,8 +127,18 @@ def attach_otel_log_bridge warn "[LaunchDarklyObservability] Could not attach log bridge to Rails.logger: #{e.message}" end + # The availability check is inlined here rather than delegating to + # LaunchDarklyObservability.otel_logger_provider_available? on purpose. + # rails.rb is required from launchdarkly_observability.rb *before* that + # file's `class << self` block (which defines the module method) has run. + # When the gem is lazily required after Rails has booted, the + # `config.after_initialize` hook above executes synchronously while this + # file is still loading, so the module method does not exist yet and the + # delegation raised "undefined method `otel_logger_provider_available?'". def otel_logger_provider_available? - LaunchDarklyObservability.send(:otel_logger_provider_available?) + defined?(OpenTelemetry::SDK::Logs::LoggerProvider) && + OpenTelemetry.respond_to?(:logger_provider) && + OpenTelemetry.logger_provider.is_a?(OpenTelemetry::SDK::Logs::LoggerProvider) end end diff --git a/sdk/@launchdarkly/observability-ruby/test/hook_test.rb b/sdk/@launchdarkly/observability-ruby/test/hook_test.rb index 700006cbd8..3f15f68665 100644 --- a/sdk/@launchdarkly/observability-ruby/test/hook_test.rb +++ b/sdk/@launchdarkly/observability-ruby/test/hook_test.rb @@ -192,6 +192,47 @@ def test_context_id_uses_fully_qualified_key_for_multi_kind assert_equal 'org:org-1:user:user-1', span.attributes['feature_flag.context.id'] end + # Regression: an invalid context (e.g. a non-string kind) is a non-nil + # object whose fully_qualified_key is nil. Setting a nil OTel attribute + # raised "OpenTelemetry error: invalid attribute value type NilClass for + # key 'feature_flag.context.id'" on every evaluation. The attribute must be + # omitted instead. + def test_invalid_context_omits_context_id_attribute + invalid_context = LaunchDarkly::LDContext.create({ key: 'k', kind: 123 }) + refute invalid_context.valid? + assert_nil invalid_context.fully_qualified_key + + series_context = LaunchDarkly::Interfaces::Hooks::EvaluationSeriesContext.new( + 'my-flag', invalid_context, false, :variation + ) + + # Capture OpenTelemetry's error channel. Setting an attribute to nil does not + # raise and does not set the attribute (OTel drops it) — the only observable + # symptom is an "invalid attribute value type NilClass" error logged here, + # which is exactly the noise the customer saw on every evaluation. + captured = StringIO.new + original_logger = OpenTelemetry.logger + OpenTelemetry.logger = ::Logger.new(captured) + begin + data = @hook.before_evaluation(series_context, {}) + @hook.after_evaluation(series_context, data, create_evaluation_detail) + ensure + OpenTelemetry.logger = original_logger + end + + refute_match(/invalid .*attribute value type NilClass/, captured.string, + 'must not emit an OTel nil-attribute error for an invalid context') + refute_match(/feature_flag\.context\.id/, captured.string) + + span = @exporter.finished_spans.first + refute_nil span + refute span.attributes.key?('feature_flag.context.id') + + flag_event = span.events.find { |e| e.name == 'feature_flag' } + refute_nil flag_event + refute flag_event.attributes.key?('feature_flag.context.id') + end + def test_nil_context_does_not_raise_and_still_records_event series_context = LaunchDarkly::Interfaces::Hooks::EvaluationSeriesContext.new( 'my-flag', nil, false, :variation diff --git a/sdk/@launchdarkly/observability-ruby/test/rails_railtie_test.rb b/sdk/@launchdarkly/observability-ruby/test/rails_railtie_test.rb index c8a163f766..da4c7072e0 100644 --- a/sdk/@launchdarkly/observability-ruby/test/rails_railtie_test.rb +++ b/sdk/@launchdarkly/observability-ruby/test/rails_railtie_test.rb @@ -114,5 +114,30 @@ def test_rails_file_loads_when_after_initialize_runs_immediately 'ViewHelpers should be defined after loading rails.rb' assert defined?(LaunchDarklyObservability::Railtie), 'Railtie should be defined after loading rails.rb' + + # Regression: the Railtie's `attach_otel_log_bridge` runs from the synchronous + # `config.after_initialize` block *during* the require of launchdarkly_observability.rb, + # before that file's `class << self` block (which defines the module-level + # `otel_logger_provider_available?`) has executed. The Railtie used to delegate to + # that not-yet-defined method, so the bridge attach failed with: + # + # Could not attach log bridge to Rails.logger: undefined method + # `otel_logger_provider_available?' for module LaunchDarklyObservability + # + # The Railtie's check must be self-contained. Simulate the load-order state by + # removing the module method, then assert the Railtie check still works. + sc = LaunchDarklyObservability.singleton_class + saved = sc.instance_method(:otel_logger_provider_available?) + sc.send(:remove_method, :otel_logger_provider_available?) + begin + result = nil + assert_silent do + result = LaunchDarklyObservability::Railtie.send(:otel_logger_provider_available?) + end + assert_includes [true, false], result + ensure + sc.send(:define_method, :otel_logger_provider_available?, saved) + sc.send(:private, :otel_logger_provider_available?) + end end end