Skip to content

Commit

Permalink
Merge pull request #160 from solarwinds/fix-response-metric-otlp
Browse files Browse the repository at this point in the history
NH-91517 only set response time HTTP attrs for HTTP server entry spans
  • Loading branch information
cheempz authored Sep 18, 2024
2 parents 9877b82 + 4c0749e commit e6a30c1
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 23 deletions.
1 change: 1 addition & 0 deletions gemfiles/test_gems.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ group :development, :test do
gem 'opentelemetry-instrumentation-all'
gem 'opentelemetry-propagator-b3'
gem 'opentelemetry-exporter-otlp'
gem 'opentelemetry-metrics-sdk'
end
9 changes: 5 additions & 4 deletions lib/solarwinds_apm/opentelemetry/otlp_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,11 @@ def meter_attributes(span)
'sw.transaction' => calculate_lambda_transaction_name(span)
}

http_status_code = get_http_status_code(span)
meter_attrs['http.status_code'] = http_status_code if http_status_code != 0
meter_attrs['http.method'] = span.attributes[HTTP_METHOD] if span.attributes[HTTP_METHOD]

if span_http?(span)
http_status_code = get_http_status_code(span)
meter_attrs['http.status_code'] = http_status_code if http_status_code != 0
meter_attrs['http.method'] = span.attributes[HTTP_METHOD] if span.attributes[HTTP_METHOD]
end
SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] meter_attrs: #{meter_attrs.inspect}" }
meter_attrs
end
Expand Down
31 changes: 12 additions & 19 deletions test/opentelemetry/otlp_processor_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,40 +9,25 @@
require './lib/solarwinds_apm/support/txn_name_manager'
require './lib/solarwinds_apm/otel_config'
require './lib/solarwinds_apm/api'
require 'opentelemetry-metrics-sdk'

describe 'otlp processor test' do
before do
skip unless defined?(OpenTelemetry::SDK::Metrics) # skip if no metrics_sdk loaded

@exporter = OpenTelemetry::Exporter::OTLP::Exporter.new
@txn_manager = SolarWindsAPM::TxnNameManager.new

@meters = { 'sw.apm.sampling.metrics' => OpenTelemetry.meter_provider.meter('sw.apm.sampling.metrics'),
'sw.apm.request.metrics' => OpenTelemetry.meter_provider.meter('sw.apm.request.metrics') }

@processor = SolarWindsAPM::OpenTelemetry::OTLPProcessor.new(@meters, @exporter, @txn_manager)
@processor = SolarWindsAPM::OpenTelemetry::OTLPProcessor.new
end

after do
@processor.instance_variable_get(:@meters)['sw.apm.request.metrics'].instance_variable_set(:@instrument_registry, {})
@processor.instance_variable_get(:@meters)['sw.apm.sampling.metrics'].instance_variable_set(:@instrument_registry, {})
end

# Yellow ERROR due to missing metrics_sdk so far for testing otlp processor
it 'processor_meters_should_be_nil_at_beginning' do
_(@processor.instance_variable_get(:@metrics).size).must_equal 0
end

# Yellow ERROR due to missing metrics_sdk so far for testing otlp processor
it 'test_on_start_verfy_component_initialized_correctly' do
@processor.on_start(create_span, OpenTelemetry::Context.current)

it 'initializes_meters_and_metrics' do
request_metrics = @processor.instance_variable_get(:@meters)['sw.apm.request.metrics']
sampling_metrics = @processor.instance_variable_get(:@meters)['sw.apm.sampling.metrics']
request_metrics_registry = request_metrics.instance_variable_get(:@instrument_registry)
sampling_metrics_registry = sampling_metrics.instance_variable_get(:@instrument_registry)

_(@processor.txn_manager.get_root_context_h('77cb6ccc522d3106114dd6ecbb70036a')).must_equal '31e175128efc4018-00'
_(@processor.instance_variable_get(:@meters).size).must_equal 2
_(@processor.instance_variable_get(:@metrics).size).must_equal 7

refute_nil(request_metrics_registry['trace.service.response_time'])
Expand All @@ -53,4 +38,12 @@
refute_nil(sampling_metrics_registry['trace.service.through_trace_count'])
refute_nil(sampling_metrics_registry['trace.service.triggered_trace_count'])
end

it 'does_not_have_transaction_manager' do
# currently otlp processor is only used in lambda which does not support transaction naming via SDK
# this assumption may change when we introduce otlp export for non-lambda environments
assert_nil(@processor.txn_manager)
end

# TODO: tests for on_start and on_end behaviour
end

0 comments on commit e6a30c1

Please sign in to comment.