diff --git a/lib/instana/exporter/otlp/base_converter.rb b/lib/instana/exporter/otlp/base_converter.rb index 075a836c..1075d24d 100644 --- a/lib/instana/exporter/otlp/base_converter.rb +++ b/lib/instana/exporter/otlp/base_converter.rb @@ -4,6 +4,7 @@ require_relative 'resource' require 'opentelemetry/trace' +require 'forwardable' module Instana module Exporter @@ -31,6 +32,14 @@ class BaseConverter # Represents the status of a span (OK, ERROR, or UNSET) Status = Struct.new(:code, :description) + # Represents a span event (mirrors OpenTelemetry::SDK::Trace::Event) + # + # Fields: + # name [String] - event name + # attributes [Hash] - key/value attributes attached to the event + # timestamp [Integer] - Unix nanoseconds + Event = Struct.new(:name, :attributes, :timestamp) + # Adapter to make resource objects compatible with OTLP exporter expectations ResourceAdapter = Struct.new(:attributes) do # @return [Enumerator] Iterator over resource attributes @@ -68,12 +77,12 @@ def total_recorded_attributes attributes.size end - # @return [Array] Empty array (events not currently supported) + # @return [Array] Empty array — error-free spans carry no events def events EMPTY_ARRAY end - # @return [Integer] Zero (events not currently supported) + # @return [Integer] Zero — no events on error-free spans def total_recorded_events 0 end @@ -101,6 +110,29 @@ def trace_flags EMPTY_ARRAY = [].freeze # rubocop:disable Lint/ConstantDefinitionInBlock end + # SpanData extended with an optional list of span events + # + # We wrap the base struct to carry events without changing the positional + # keyword_init constructor used by all converters. + SpanDataWithEvents = Struct.new(:span_data, :span_events) do + extend Forwardable + def_delegators :span_data, + :name, :trace_id, :span_id, :parent_span_id, + :resource, :instrumentation_scope, :kind, + :start_timestamp, :end_timestamp, :attributes, :status, + :tracestate, :total_recorded_attributes, + :links, :total_recorded_links, + :parent_span_is_remote, :trace_flags + + def events + span_events + end + + def total_recorded_events + span_events.size + end + end + # Milliseconds to nanoseconds conversion factor MS_TO_NS = 1_000_000 private_constant :MS_TO_NS @@ -114,9 +146,14 @@ def initialize(span, resource = nil) # Convert the Instana span to OTLP-compatible span data # - # @return [SpanData] Converted span data object ready for export + # @return [SpanData, SpanDataWithEvents] Converted span data object ready for export def convert - SpanData.new( + # Resolve error info once — reused by both status and events + error_count = span[:ec].to_i + error_msg = error_count.positive? ? extract_error_message : nil + stacktrace = error_count.positive? ? convert_stack_trace : nil + + span_data = SpanData.new( name: span_name, trace_id: format_trace_id(span[:t]), span_id: format_span_id(span[:s]), @@ -127,8 +164,13 @@ def convert start_timestamp: convert_to_unix_nano(span[:ts]), end_timestamp: calculate_end_timestamp, attributes: convert_attributes, - status: convert_status + status: build_status(error_count, error_msg) ) + + events = build_error_events(error_count, error_msg, stacktrace) + return SpanDataWithEvents.new(span_data, events) unless events.empty? + + span_data end protected @@ -194,22 +236,54 @@ def convert_to_unix_nano(time) end end - # Convert span status to OpenTelemetry status object + # Build span status from pre-resolved error info # - # @return [Status] Status object with code and optional description - def convert_status - if span[:error] - error_message = extract_error_message - Status.new(OpenTelemetry::Trace::Status::ERROR, error_message.to_s) + # @param error_count [Integer] Span error count (span[:ec].to_i) + # @param error_msg [String, nil] Pre-extracted error message + # @return [Status] + def build_status(error_count, error_msg) + if error_count.positive? + Status.new(OpenTelemetry::Trace::Status::ERROR, error_msg.to_s) else Status.new(OpenTelemetry::Trace::Status::UNSET, '') end end - # Extract error message from span - # @return [String, nil] Error message + # Extract error message from span data + # + # Searches `span[:data][][:error]` for any span type that + # carries an error field (e.g. http.error, activerecord.error). + # Returns the first non-nil value found, truncated to 1024 chars + # per the OTel status.message recommendation. + # + # @return [String, nil] Error message or nil when not present def extract_error_message - # TODO: Implement error message extraction + data = span[:data] + return nil unless data.is_a?(Hash) + + data.each_value do |type_data| + next unless type_data.is_a?(Hash) + + msg = type_data[:error] + return msg.to_s[0, 1024] if msg + end + + nil + end + + # Convert Instana stack trace to OTel exception.stacktrace string + # + # Instana stores stack frames as an Array of Hashes with keys: + # c: file path, n: line number, m: method name + # + # OTel requires a newline-separated string of stack frames. + # + # @return [String, nil] Formatted stacktrace or nil if not present + def convert_stack_trace + stack = span[:stack] + return nil unless stack.is_a?(Array) && !stack.empty? + + stack.map { |frame| "#{frame[:c]}:#{frame[:n]} in #{frame[:m]}" }.join("\n") end # Convert span attributes to OTLP-compatible attributes @@ -222,6 +296,31 @@ def convert_attributes {} end + # Build OTel span events from pre-resolved error info + # + # - error_count > 0 AND stacktrace present → "exception" event + # - error_count > 0, no stack → "error" event + # - error_count == 0 → [] + # + # @param error_count [Integer] + # @param error_msg [String, nil] + # @param stacktrace [String, nil] + # @return [Array] + def build_error_events(error_count, error_msg, stacktrace) + return [] unless error_count.positive? + + timestamp = calculate_end_timestamp + + if stacktrace + attrs = { 'exception.type' => span_name } + attrs['exception.message'] = error_msg if error_msg + attrs['exception.stacktrace'] = stacktrace + [Event.new('exception', attrs, timestamp)] + else + [Event.new('error', { 'error.type' => span_name }, timestamp)] + end + end + # Add an attribute to the attributes hash if value is not nil # # @param attributes [Hash] The attributes hash to add to @@ -257,9 +356,18 @@ def normalize_attribute_value(value) # Get the span name as a string # + # For custom (SDK) spans the user-supplied name is stored in + # span[:data][:sdk][:name], not in span[:n] (which is always :sdk). + # We read that path directly to avoid crashing when sdk data has been + # overwritten by tests or other code. Non-custom spans use span[:n]. + # # @return [String] The span name def span_name - span[:n].to_s + if span.respond_to?(:custom?) ? span.custom? : span[:n]&.to_sym == :sdk + span[:data]&.dig(:sdk, :name).to_s + else + span[:n].to_s + end end # Format parent span ID, returning INVALID_SPAN_ID if no parent diff --git a/test/exporter/otlp/base_converter_test.rb b/test/exporter/otlp/base_converter_test.rb index 127f8325..bdf4a31a 100644 --- a/test/exporter/otlp/base_converter_test.rb +++ b/test/exporter/otlp/base_converter_test.rb @@ -81,23 +81,178 @@ def test_convert_to_unix_nano end def test_convert_status - # Test UNSET status (no error) + # Test UNSET status (no error — ec is 0 or absent) span = create_test_span(name: :rack) converter = TestConverter.new(span) - status = converter.send(:convert_status) + status = converter.send(:build_status, span[:ec].to_i, nil) assert_equal OpenTelemetry::Trace::Status::UNSET, status.code assert_equal '', status.description - # Test ERROR status + # Test ERROR status driven by span.ec > 0 span = create_test_span(name: :rack) span.record_exception(StandardError.new('Test error')) converter = TestConverter.new(span) - status = converter.send(:convert_status) + status = converter.send(:build_status, span[:ec].to_i, converter.send(:extract_error_message)) assert_equal OpenTelemetry::Trace::Status::ERROR, status.code end + def test_convert_status_uses_ec_not_error_flag + # span.ec > 0 should always yield ERROR, independent of :error flag + span = create_test_span(name: :rack) + span[:ec] = 2 + converter = TestConverter.new(span) + status = converter.send(:build_status, span[:ec].to_i, nil) + assert_equal OpenTelemetry::Trace::Status::ERROR, status.code + + # ec == 0 should yield UNSET even when :error is true + span2 = create_test_span(name: :rack) + span2[:error] = true + span2[:ec] = 0 + converter2 = TestConverter.new(span2) + status2 = converter2.send(:build_status, span2[:ec].to_i, nil) + assert_equal OpenTelemetry::Trace::Status::UNSET, status2.code + end + + def test_extract_error_message_returns_nil_when_no_data + span = create_test_span(name: :rack) + converter = TestConverter.new(span) + assert_nil converter.send(:extract_error_message) + end + + def test_extract_error_message_finds_error_in_type_data + span = create_test_span(name: :rack, data: { http: { error: 'Connection refused' } }) + converter = TestConverter.new(span) + assert_equal 'Connection refused', converter.send(:extract_error_message) + end + + def test_extract_error_message_truncates_to_1024_chars + long_msg = 'x' * 2000 + span = create_test_span(name: :rack, data: { http: { error: long_msg } }) + converter = TestConverter.new(span) + result = converter.send(:extract_error_message) + assert_equal 1024, result.length + end + + def test_convert_stack_trace_returns_nil_when_no_stack + span = create_test_span(name: :rack) + converter = TestConverter.new(span) + assert_nil converter.send(:convert_stack_trace) + end + + def test_convert_stack_trace_formats_frames_as_string + span = create_test_span(name: :rack) + span[:stack] = [ + { c: '/app/models/user.rb', n: '42', m: 'in `save`' }, + { c: '/app/controllers/users_controller.rb', n: '10', m: 'in `create`' } + ] + converter = TestConverter.new(span) + result = converter.send(:convert_stack_trace) + assert_equal "/app/models/user.rb:42 in in `save`\n" \ + "/app/controllers/users_controller.rb:10 in in `create`", + result + end + + def test_convert_stack_trace_returns_nil_for_empty_stack + span = create_test_span(name: :rack) + span[:stack] = [] + converter = TestConverter.new(span) + assert_nil converter.send(:convert_stack_trace) + end + + # --- build_error_events / event-based error recording --- + + def test_no_error_events_when_no_error + span = create_test_span(name: :rack) + converter = TestConverter.new(span) + assert_equal [], converter.send(:build_error_events, span[:ec].to_i, nil, nil) + end + + def test_exception_event_emitted_when_ec_positive_and_stack_present + span = create_test_span(name: :rack, data: { http: { error: 'Timeout' } }) + span[:stack] = [{ c: '/app/lib/client.rb', n: '7', m: 'in `call`' }] + span[:ec] = 1 + converter = TestConverter.new(span) + events = converter.send(:build_error_events, span[:ec].to_i, converter.send(:extract_error_message), converter.send(:convert_stack_trace)) + + assert_equal 1, events.size + event = events.first + assert_equal 'exception', event.name + assert_equal 'rack', event.attributes['exception.type'] + assert_equal 'Timeout', event.attributes['exception.message'] + assert_equal '/app/lib/client.rb:7 in in `call`', event.attributes['exception.stacktrace'] + assert_kind_of Integer, event.timestamp + end + + def test_error_event_emitted_when_ec_positive_but_no_stack + span = create_test_span(name: :rack, data: { http: { error: 'Timeout' } }) + span[:ec] = 1 + converter = TestConverter.new(span) + events = converter.send(:build_error_events, span[:ec].to_i, converter.send(:extract_error_message), converter.send(:convert_stack_trace)) + + assert_equal 1, events.size + event = events.first + assert_equal 'error', event.name + assert_equal 'rack', event.attributes['error.type'] + end + + def test_convert_returns_span_data_with_events_on_error + span = create_test_span(name: :rack, data: { http: { error: 'Timeout' } }) + span[:stack] = [{ c: '/app/lib/client.rb', n: '7', m: 'in `call`' }] + span[:ec] = 1 + converter = TestConverter.new(span) + result = converter.convert + + assert_instance_of Instana::Exporter::Otlp::BaseConverter::SpanDataWithEvents, result + assert_equal 1, result.total_recorded_events + assert_equal 'exception', result.events.first.name + end + + def test_convert_returns_plain_span_data_when_no_error + span = create_test_span(name: :rack) + converter = TestConverter.new(span) + result = converter.convert + + assert_instance_of Instana::Exporter::Otlp::BaseConverter::SpanData, result + assert_equal 0, result.total_recorded_events + assert_equal [], result.events + end + + def test_exception_event_has_no_message_when_no_error_field + span = create_test_span(name: :rack) + span[:stack] = [{ c: '/app/lib/client.rb', n: '7', m: 'in `call`' }] + span[:ec] = 1 + converter = TestConverter.new(span) + events = converter.send(:build_error_events, span[:ec].to_i, converter.send(:extract_error_message), converter.send(:convert_stack_trace)) + + assert_equal 1, events.size + event = events.first + assert_equal 'exception', event.name + refute event.attributes.key?('exception.message') + assert event.attributes.key?('exception.stacktrace') + end + + def test_convert_status_description_contains_error_message + span = create_test_span(name: :rack, data: { http: { error: 'Server error' } }) + span[:ec] = 1 + converter = TestConverter.new(span) + status = converter.send(:build_status, span[:ec].to_i, converter.send(:extract_error_message)) + assert_equal 'Server error', status.description + end + + def test_span_attributes_do_not_contain_exception_keys + # exception.* belong on events, NOT on span attributes + span = create_test_span(name: :rack, data: { http: { error: 'Timeout' } }) + span[:stack] = [{ c: '/app/lib/client.rb', n: '7', m: 'in `call`' }] + span[:ec] = 1 + converter = TestConverter.new(span) + result = converter.convert + + refute result.attributes.key?('exception.stacktrace') + refute result.attributes.key?('exception.message') + end + def test_convert_attributes - # Test empty attributes for base converter + # Test empty attributes for base converter (no stack) span = create_test_span(data: { undefined: { method: 'GET', url: 'http://example.com' } }) converter = TestConverter.new(span) attributes = converter.send(:convert_attributes) @@ -174,10 +329,19 @@ def create_test_span(kind: 3, data: nil, name: :rack) span end + def create_error_span(name: :rack, error_msg: nil, stack: nil) + span = create_test_span(name: name) + span[:ec] = 1 + span[:data] = { http: { error: error_msg } } if error_msg + span[:stack] = stack if stack + span + end + # Test converter class that exposes protected methods for testing class TestConverter < Instana::Exporter::Otlp::BaseConverter # Make protected methods public for testing public :convert_span_kind, :convert_to_unix_nano, - :convert_status, :convert_attributes, :normalize_attribute_value, :span + :build_status, :convert_attributes, :normalize_attribute_value, :span, + :extract_error_message, :convert_stack_trace, :build_error_events end end