From 33bfd125a3e9b8faa1b0e5171e2398d3dd5f6c8b Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Fri, 25 Aug 2023 11:00:17 -0400 Subject: [PATCH 1/4] NH-44743: use txn_manager to store root span --- lib/solarwinds_apm/api/transaction_name.rb | 4 +--- lib/solarwinds_apm/constants.rb | 4 ---- .../opentelemetry/solarwinds_processor.rb | 6 ++---- lib/solarwinds_apm/support/txn_name_manager.rb | 9 +++++++++ test/opentelemetry/solarwinds_processor_test.rb | 3 +-- test/support/txn_name_manager_test.rb | 12 ++++++++---- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/lib/solarwinds_apm/api/transaction_name.rb b/lib/solarwinds_apm/api/transaction_name.rb index a8d218bc..27552e47 100644 --- a/lib/solarwinds_apm/api/transaction_name.rb +++ b/lib/solarwinds_apm/api/transaction_name.rb @@ -42,9 +42,7 @@ def set_transaction_name(custom_name=nil) SolarWindsAPM.logger.warn {"[#{name}/#{__method__}] Solarwinds processor is missing. Set transaction name failed."} status = false else - entry_trace_id = ::OpenTelemetry::Baggage.value(::SolarWindsAPM::Constants::INTL_SWO_CURRENT_TRACE_ID) - entry_span_id = ::OpenTelemetry::Baggage.value(::SolarWindsAPM::Constants::INTL_SWO_CURRENT_SPAN_ID) - trace_flags = ::OpenTelemetry::Baggage.value(::SolarWindsAPM::Constants::INTL_SWO_CURRENT_TRACE_FLAG) + entry_trace_id, entry_span_id, trace_flags = solarwinds_processor.txn_manager.get_root_context&.split('-') status = false if entry_trace_id.nil? || entry_span_id.nil? || trace_flags.nil? status = false if entry_trace_id == '0'*32 || entry_span_id == '0'*16 || trace_flags == '00' # not sampled diff --git a/lib/solarwinds_apm/constants.rb b/lib/solarwinds_apm/constants.rb index ec014bfa..0598c93b 100644 --- a/lib/solarwinds_apm/constants.rb +++ b/lib/solarwinds_apm/constants.rb @@ -20,10 +20,6 @@ module Constants INTL_SWO_PROPAGATOR = "solarwinds_propagator".freeze INTL_SWO_DEFAULT_PROPAGATORS = [INTL_SWO_TRACECONTEXT_PROPAGATOR, "baggage",INTL_SWO_PROPAGATOR].freeze INTL_SWO_SUPPORT_EMAIL = "SWO-support@solarwinds.com".freeze - INTL_SWO_CURRENT_SPAN_ID = "sw-current-entry-span-id".freeze - INTL_SWO_CURRENT_TRACE_ID = "sw-current-trace-id".freeze - INTL_SWO_CURRENT_TRACE_FLAG = "sw-current-trace-flag".freeze - INTL_SWO_OTEL_SCOPE_NAME = "otel.scope.name".freeze INTL_SWO_OTEL_SCOPE_VERSION = "otel.scope.version".freeze diff --git a/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb b/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb index 79dcca06..ed353302 100644 --- a/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb +++ b/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb @@ -32,10 +32,8 @@ def on_start(span, parent_context) return if parent_span && parent_span.context != ::OpenTelemetry::Trace::SpanContext::INVALID && parent_span.context.remote? == false trace_flags = span.context.trace_flags.sampled? ? '01' : '00' - ::OpenTelemetry::Context.attach(::OpenTelemetry::Baggage.set_value(::SolarWindsAPM::Constants::INTL_SWO_CURRENT_TRACE_ID, span.context.hex_trace_id)) - ::OpenTelemetry::Context.attach(::OpenTelemetry::Baggage.set_value(::SolarWindsAPM::Constants::INTL_SWO_CURRENT_SPAN_ID, span.context.hex_span_id)) - ::OpenTelemetry::Context.attach(::OpenTelemetry::Baggage.set_value(::SolarWindsAPM::Constants::INTL_SWO_CURRENT_TRACE_FLAG, trace_flags)) - + @txn_manager.set_root_context("#{span.context.hex_trace_id}-#{span.context.hex_span_id}-#{trace_flags}") + SolarWindsAPM.logger.debug {"[#{self.class}/#{__method__}] current baggage values: #{::OpenTelemetry::Baggage.values}"} end diff --git a/lib/solarwinds_apm/support/txn_name_manager.rb b/lib/solarwinds_apm/support/txn_name_manager.rb index 760a29a1..7efd8c0b 100644 --- a/lib/solarwinds_apm/support/txn_name_manager.rb +++ b/lib/solarwinds_apm/support/txn_name_manager.rb @@ -4,6 +4,7 @@ module OpenTelemetry class TxnNameManager def initialize @cache = {} + @root_context = nil end def get(key) @@ -20,6 +21,14 @@ def set(key, value) end alias []= set + + def set_root_context(value) + @root_context = value + end + + def get_root_context + @root_context + end end end end diff --git a/test/opentelemetry/solarwinds_processor_test.rb b/test/opentelemetry/solarwinds_processor_test.rb index f730cc07..66dce929 100644 --- a/test/opentelemetry/solarwinds_processor_test.rb +++ b/test/opentelemetry/solarwinds_processor_test.rb @@ -60,7 +60,6 @@ span = create_span processor = SolarWindsAPM::OpenTelemetry::SolarWindsProcessor.new(@exporter, @txn_name_manager) processor.on_start(span, ::OpenTelemetry::Context.current) - _(::OpenTelemetry::Baggage.value(::SolarWindsAPM::Constants::INTL_SWO_CURRENT_TRACE_ID)).must_equal '77cb6ccc522d3106114dd6ecbb70036a' - _(::OpenTelemetry::Baggage.value(::SolarWindsAPM::Constants::INTL_SWO_CURRENT_SPAN_ID)).must_equal '31e175128efc4018' + _(processor.instance_variable_get(:@txn_manager).get_root_context).must_equal "77cb6ccc522d3106114dd6ecbb70036a-31e175128efc4018-00" end end diff --git a/test/support/txn_name_manager_test.rb b/test/support/txn_name_manager_test.rb index 6b25aba1..9ecc3185 100644 --- a/test/support/txn_name_manager_test.rb +++ b/test/support/txn_name_manager_test.rb @@ -11,23 +11,27 @@ @txn_manager.set("c","d") end - it 'test set' do + it 'test_set' do @txn_manager.set("a","b") _(@txn_manager.get("a")).must_equal "b" end - it 'test []' do + it 'test_[]' do @txn_manager["e"] = "f" _(@txn_manager.get("e")).must_equal "f" end - it 'test del' do + it 'test_del' do @txn_manager.del("c") _(@txn_manager.get("c")).must_equal nil end - it 'test get' do + it 'test_get' do @txn_manager.get("c").must_equal "d" end + it 'test_set_get_root_context' do + @txn_manager.set_root_context('abcd') + _(@txn_manager.get_root_context).must_equal 'abcd' + end end From 8042ffe29c9b0fbbbb7f4b444f705c1d402c57d8 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Fri, 25 Aug 2023 11:14:27 -0400 Subject: [PATCH 2/4] NH-44743: use attr --- lib/solarwinds_apm/api/transaction_name.rb | 2 +- .../opentelemetry/solarwinds_processor.rb | 2 +- lib/solarwinds_apm/support/txn_name_manager.rb | 10 ++-------- test/opentelemetry/solarwinds_processor_test.rb | 2 +- test/support/txn_name_manager_test.rb | 4 ++-- 5 files changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/solarwinds_apm/api/transaction_name.rb b/lib/solarwinds_apm/api/transaction_name.rb index 27552e47..4330a9cf 100644 --- a/lib/solarwinds_apm/api/transaction_name.rb +++ b/lib/solarwinds_apm/api/transaction_name.rb @@ -42,7 +42,7 @@ def set_transaction_name(custom_name=nil) SolarWindsAPM.logger.warn {"[#{name}/#{__method__}] Solarwinds processor is missing. Set transaction name failed."} status = false else - entry_trace_id, entry_span_id, trace_flags = solarwinds_processor.txn_manager.get_root_context&.split('-') + entry_trace_id, entry_span_id, trace_flags = solarwinds_processor.txn_manager.root_context&.split('-') status = false if entry_trace_id.nil? || entry_span_id.nil? || trace_flags.nil? status = false if entry_trace_id == '0'*32 || entry_span_id == '0'*16 || trace_flags == '00' # not sampled diff --git a/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb b/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb index ed353302..0029bf7a 100644 --- a/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb +++ b/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb @@ -32,7 +32,7 @@ def on_start(span, parent_context) return if parent_span && parent_span.context != ::OpenTelemetry::Trace::SpanContext::INVALID && parent_span.context.remote? == false trace_flags = span.context.trace_flags.sampled? ? '01' : '00' - @txn_manager.set_root_context("#{span.context.hex_trace_id}-#{span.context.hex_span_id}-#{trace_flags}") + @txn_manager.root_context = "#{span.context.hex_trace_id}-#{span.context.hex_span_id}-#{trace_flags}" SolarWindsAPM.logger.debug {"[#{self.class}/#{__method__}] current baggage values: #{::OpenTelemetry::Baggage.values}"} end diff --git a/lib/solarwinds_apm/support/txn_name_manager.rb b/lib/solarwinds_apm/support/txn_name_manager.rb index 7efd8c0b..a6740415 100644 --- a/lib/solarwinds_apm/support/txn_name_manager.rb +++ b/lib/solarwinds_apm/support/txn_name_manager.rb @@ -2,6 +2,8 @@ module SolarWindsAPM module OpenTelemetry # SolarWindsTxnNameManager class TxnNameManager + attr_accessor :root_context + def initialize @cache = {} @root_context = nil @@ -21,14 +23,6 @@ def set(key, value) end alias []= set - - def set_root_context(value) - @root_context = value - end - - def get_root_context - @root_context - end end end end diff --git a/test/opentelemetry/solarwinds_processor_test.rb b/test/opentelemetry/solarwinds_processor_test.rb index 66dce929..b9e0e08d 100644 --- a/test/opentelemetry/solarwinds_processor_test.rb +++ b/test/opentelemetry/solarwinds_processor_test.rb @@ -60,6 +60,6 @@ span = create_span processor = SolarWindsAPM::OpenTelemetry::SolarWindsProcessor.new(@exporter, @txn_name_manager) processor.on_start(span, ::OpenTelemetry::Context.current) - _(processor.instance_variable_get(:@txn_manager).get_root_context).must_equal "77cb6ccc522d3106114dd6ecbb70036a-31e175128efc4018-00" + _(processor.txn_manager.root_context).must_equal "77cb6ccc522d3106114dd6ecbb70036a-31e175128efc4018-00" end end diff --git a/test/support/txn_name_manager_test.rb b/test/support/txn_name_manager_test.rb index 9ecc3185..50a87b3f 100644 --- a/test/support/txn_name_manager_test.rb +++ b/test/support/txn_name_manager_test.rb @@ -31,7 +31,7 @@ end it 'test_set_get_root_context' do - @txn_manager.set_root_context('abcd') - _(@txn_manager.get_root_context).must_equal 'abcd' + @txn_manager.root_context = 'abcd' + _(@txn_manager.root_context).must_equal 'abcd' end end From fe977cd8889394b42be09e8e8f0ff1794b776405 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Tue, 29 Aug 2023 10:24:43 -0400 Subject: [PATCH 3/4] NH-44743: use hash to store root span id --- lib/solarwinds_apm/api/transaction_name.rb | 22 +++++++++---------- .../opentelemetry/solarwinds_processor.rb | 4 ++-- .../support/txn_name_manager.rb | 21 +++++++++++++++--- test/minitest_helper.rb | 1 - 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/lib/solarwinds_apm/api/transaction_name.rb b/lib/solarwinds_apm/api/transaction_name.rb index 4330a9cf..ec0c2bb4 100644 --- a/lib/solarwinds_apm/api/transaction_name.rb +++ b/lib/solarwinds_apm/api/transaction_name.rb @@ -36,20 +36,20 @@ def set_transaction_name(custom_name=nil) status = false elsif SolarWindsAPM::Context.toString == '99-00000000000000000000000000000000-0000000000000000-00' # noop status = true + elsif SolarWindsAPM::OTelConfig.class_variable_get(:@@config)[:span_processor].nil? + SolarWindsAPM.logger.warn {"[#{name}/#{__method__}] Solarwinds processor is missing. Set transaction name failed."} + status = false else - solarwinds_processor = SolarWindsAPM::OTelConfig.class_variable_get(:@@config)[:span_processor] - if solarwinds_processor.nil? - SolarWindsAPM.logger.warn {"[#{name}/#{__method__}] Solarwinds processor is missing. Set transaction name failed."} - status = false - else - entry_trace_id, entry_span_id, trace_flags = solarwinds_processor.txn_manager.root_context&.split('-') + solarwinds_processor = SolarWindsAPM::OTelConfig.class_variable_get(:@@config)[:span_processor] + current_span = ::OpenTelemetry::Trace.current_span + entry_trace_id = current_span.context.hex_trace_id + entry_span_id, trace_flags = solarwinds_processor.txn_manager.get_root_context_h(entry_trace_id)&.split('-') - status = false if entry_trace_id.nil? || entry_span_id.nil? || trace_flags.nil? - status = false if entry_trace_id == '0'*32 || entry_span_id == '0'*16 || trace_flags == '00' # not sampled + status = false if entry_trace_id.nil? || entry_span_id.nil? || trace_flags.nil? + status = false if entry_trace_id == '0'*32 || entry_span_id == '0'*16 || trace_flags == '00' # not sampled - solarwinds_processor.txn_manager.set("#{entry_trace_id}-#{entry_span_id}",custom_name) - SolarWindsAPM.logger.debug {"[#{name}/#{__method__}] Cached custom transaction name for #{entry_trace_id}-#{entry_span_id} as #{custom_name}"} - end + solarwinds_processor.txn_manager.set("#{entry_trace_id}-#{entry_span_id}",custom_name) + SolarWindsAPM.logger.debug {"[#{name}/#{__method__}] Cached custom transaction name for #{entry_trace_id}-#{entry_span_id} as #{custom_name}"} end status end diff --git a/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb b/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb index 0029bf7a..38fc8b7c 100644 --- a/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb +++ b/lib/solarwinds_apm/opentelemetry/solarwinds_processor.rb @@ -32,7 +32,7 @@ def on_start(span, parent_context) return if parent_span && parent_span.context != ::OpenTelemetry::Trace::SpanContext::INVALID && parent_span.context.remote? == false trace_flags = span.context.trace_flags.sampled? ? '01' : '00' - @txn_manager.root_context = "#{span.context.hex_trace_id}-#{span.context.hex_span_id}-#{trace_flags}" + @txn_manager.set_root_context_h(span.context.hex_trace_id,"#{span.context.hex_span_id}-#{trace_flags}") SolarWindsAPM.logger.debug {"[#{self.class}/#{__method__}] current baggage values: #{::OpenTelemetry::Baggage.values}"} end @@ -90,7 +90,7 @@ def on_finish(span) SolarWindsAPM.logger.debug {"[#{self.class}/#{__method__}] liboboe_txn_name: #{liboboe_txn_name}"} @txn_manager["#{span.context.hex_trace_id}-#{span.context.hex_span_id}"] = liboboe_txn_name if span.context.trace_flags.sampled? - + @txn_manager.delete_root_context_h(span.context.hex_trace_id) @exporter&.export([span.to_span_data]) if span.context.trace_flags.sampled? end diff --git a/lib/solarwinds_apm/support/txn_name_manager.rb b/lib/solarwinds_apm/support/txn_name_manager.rb index a6740415..5051f055 100644 --- a/lib/solarwinds_apm/support/txn_name_manager.rb +++ b/lib/solarwinds_apm/support/txn_name_manager.rb @@ -2,11 +2,10 @@ module SolarWindsAPM module OpenTelemetry # SolarWindsTxnNameManager class TxnNameManager - attr_accessor :root_context - def initialize @cache = {} - @root_context = nil + @root_context_h = {} + @mutex = Mutex.new end def get(key) @@ -23,6 +22,22 @@ def set(key, value) end alias []= set + + def set_root_context_h(key, value) + @mutex.synchronize do + @root_context_h[key] = value + end + end + + def get_root_context_h(key) + @root_context_h[key] + end + + def delete_root_context_h(key) + @mutex.synchronize do + @root_context_h.delete(key) + end + end end end end diff --git a/test/minitest_helper.rb b/test/minitest_helper.rb index a10d0ba7..931997bb 100644 --- a/test/minitest_helper.rb +++ b/test/minitest_helper.rb @@ -57,7 +57,6 @@ def $stdout.write(string) # Bundler.require(:default, :test) # this load the solarwinds_apm library SolarWindsAPM.logger.level = 1 - # Dummy Propagator for testing module Dummy class TextMapPropagator From 245a3cf75e0606739cff91960cb1d17a766ae844 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Tue, 29 Aug 2023 11:17:57 -0400 Subject: [PATCH 4/4] NH-44743: update test case --- test/api/set_transaction_name_test.rb | 20 +++++++++++++------ .../solarwinds_processor_test.rb | 2 +- test/support/txn_name_manager_test.rb | 4 ++-- test/test_setup.sh | 2 ++ 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/test/api/set_transaction_name_test.rb b/test/api/set_transaction_name_test.rb index 992aab01..095b36de 100644 --- a/test/api/set_transaction_name_test.rb +++ b/test/api/set_transaction_name_test.rb @@ -11,6 +11,8 @@ describe 'SolarWinds Set Transaction Name Test' do before do @span = create_span + @dummy_span = create_span + @dummy_span.context.instance_variable_set(:@span_id, 'fake_span_id') # with fake span_id, should still find the right root span SolarWindsAPM::OTelConfig.initialize @processors = ::OpenTelemetry.tracer_provider.instance_variable_get(:@span_processors) @solarwinds_processor = @processors.last @@ -23,23 +25,29 @@ it 'calculate_transaction_names_with_unsampled_span' do @solarwinds_processor.on_start(@span, ::OpenTelemetry::Context.current) - result = SolarWindsAPM::API.set_transaction_name('abcdf') - _(result).must_equal false + OpenTelemetry::Trace.stub(:current_span, @dummy_span) do + result = SolarWindsAPM::API.set_transaction_name('abcdf') + _(result).must_equal false + end _(@solarwinds_processor.txn_manager.get("77cb6ccc522d3106114dd6ecbb70036a-31e175128efc4018")).must_equal "abcdf" end it 'calculate_transaction_names_with_empty_transaction_name' do @solarwinds_processor.on_start(@span, ::OpenTelemetry::Context.current) - result = SolarWindsAPM::API.set_transaction_name('') - _(result).must_equal false + OpenTelemetry::Trace.stub(:current_span, @dummy_span) do + result = SolarWindsAPM::API.set_transaction_name('') + _(result).must_equal false + end assert_nil(@solarwinds_processor.txn_manager.get("77cb6ccc522d3106114dd6ecbb70036a-31e175128efc4018")) end it 'calculate_transaction_names_with_sampled_span' do @span.context.trace_flags.instance_variable_set(:@flags, 1) @solarwinds_processor.on_start(@span, ::OpenTelemetry::Context.current) - result = SolarWindsAPM::API.set_transaction_name('abcdf') - _(result).must_equal true + OpenTelemetry::Trace.stub(:current_span, @dummy_span) do + result = SolarWindsAPM::API.set_transaction_name('abcdf') + _(result).must_equal true + end _(@solarwinds_processor.txn_manager.get("77cb6ccc522d3106114dd6ecbb70036a-31e175128efc4018")).must_equal "abcdf" end end diff --git a/test/opentelemetry/solarwinds_processor_test.rb b/test/opentelemetry/solarwinds_processor_test.rb index b9e0e08d..3d25a683 100644 --- a/test/opentelemetry/solarwinds_processor_test.rb +++ b/test/opentelemetry/solarwinds_processor_test.rb @@ -60,6 +60,6 @@ span = create_span processor = SolarWindsAPM::OpenTelemetry::SolarWindsProcessor.new(@exporter, @txn_name_manager) processor.on_start(span, ::OpenTelemetry::Context.current) - _(processor.txn_manager.root_context).must_equal "77cb6ccc522d3106114dd6ecbb70036a-31e175128efc4018-00" + _(processor.txn_manager.get_root_context_h('77cb6ccc522d3106114dd6ecbb70036a')).must_equal "31e175128efc4018-00" end end diff --git a/test/support/txn_name_manager_test.rb b/test/support/txn_name_manager_test.rb index 50a87b3f..6e24fc7a 100644 --- a/test/support/txn_name_manager_test.rb +++ b/test/support/txn_name_manager_test.rb @@ -31,7 +31,7 @@ end it 'test_set_get_root_context' do - @txn_manager.root_context = 'abcd' - _(@txn_manager.root_context).must_equal 'abcd' + @txn_manager.set_root_context_h('key1', 'abcd') + _(@txn_manager.get_root_context_h('key1')).must_equal 'abcd' end end diff --git a/test/test_setup.sh b/test/test_setup.sh index 79f0aefd..cb85e8b9 100755 --- a/test/test_setup.sh +++ b/test/test_setup.sh @@ -22,6 +22,8 @@ if [ -n "$RUN_TESTS" ]; then { export SW_APM_REPORTER=file mkdir -p log test/run_tests.sh + status=$? echo "Finished Unit Test" + exit $status } fi