From 7168a34c26ae4ae0db9806acfa482d100ffc6edf Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 15 Nov 2021 16:22:23 -0800 Subject: [PATCH 1/6] skip recording a span event when the transaction is ignored --- .../infinite_tracing/agent_integrations/datastore_segment.rb | 3 +++ .../agent_integrations/external_request_segment.rb | 3 +++ .../lib/infinite_tracing/agent_integrations/segment.rb | 3 +++ lib/new_relic/agent/transaction/datastore_segment.rb | 3 +++ lib/new_relic/agent/transaction/external_request_segment.rb | 3 +++ lib/new_relic/agent/transaction/segment.rb | 3 +++ 6 files changed, 18 insertions(+) diff --git a/infinite_tracing/lib/infinite_tracing/agent_integrations/datastore_segment.rb b/infinite_tracing/lib/infinite_tracing/agent_integrations/datastore_segment.rb index 56328e79fa..566324ff23 100644 --- a/infinite_tracing/lib/infinite_tracing/agent_integrations/datastore_segment.rb +++ b/infinite_tracing/lib/infinite_tracing/agent_integrations/datastore_segment.rb @@ -8,6 +8,9 @@ module Agent class Transaction class DatastoreSegment def record_span_event + # don't record a span event if the transaction is ignored + return if transaction.ignore? + tracer = ::NewRelic::Agent.agent.infinite_tracer tracer << Proc.new { SpanEventPrimitive.for_datastore_segment self } end diff --git a/infinite_tracing/lib/infinite_tracing/agent_integrations/external_request_segment.rb b/infinite_tracing/lib/infinite_tracing/agent_integrations/external_request_segment.rb index bd0a9ddd05..acd658519f 100644 --- a/infinite_tracing/lib/infinite_tracing/agent_integrations/external_request_segment.rb +++ b/infinite_tracing/lib/infinite_tracing/agent_integrations/external_request_segment.rb @@ -8,6 +8,9 @@ module Agent class Transaction class ExternalRequestSegment def record_span_event + # don't record a span event if the transaction is ignored + return if transaction.ignore? + tracer = ::NewRelic::Agent.agent.infinite_tracer tracer << Proc.new { SpanEventPrimitive.for_external_request_segment self } end diff --git a/infinite_tracing/lib/infinite_tracing/agent_integrations/segment.rb b/infinite_tracing/lib/infinite_tracing/agent_integrations/segment.rb index 99d7b2e164..4a2488e528 100644 --- a/infinite_tracing/lib/infinite_tracing/agent_integrations/segment.rb +++ b/infinite_tracing/lib/infinite_tracing/agent_integrations/segment.rb @@ -13,6 +13,9 @@ def segment_complete end def record_span_event + # don't record a span event if the transaction is ignored + return if transaction.ignore? + tracer = ::NewRelic::Agent.agent.infinite_tracer tracer << Proc.new { SpanEventPrimitive.for_segment self } end diff --git a/lib/new_relic/agent/transaction/datastore_segment.rb b/lib/new_relic/agent/transaction/datastore_segment.rb index 063c1b1692..73aa784960 100644 --- a/lib/new_relic/agent/transaction/datastore_segment.rb +++ b/lib/new_relic/agent/transaction/datastore_segment.rb @@ -138,6 +138,9 @@ def record_sql? end def record_span_event + # don't record a span event if the transaction is ignored + return if transaction.ignore? + aggregator = ::NewRelic::Agent.agent.span_event_aggregator priority = transaction.priority diff --git a/lib/new_relic/agent/transaction/external_request_segment.rb b/lib/new_relic/agent/transaction/external_request_segment.rb index a870e1d0ff..7ae6662623 100644 --- a/lib/new_relic/agent/transaction/external_request_segment.rb +++ b/lib/new_relic/agent/transaction/external_request_segment.rb @@ -250,6 +250,9 @@ def obfuscator end def record_span_event + # don't record a span event if the transaction is ignored + return if transaction.ignore? + aggregator = ::NewRelic::Agent.agent.span_event_aggregator priority = transaction.priority aggregator.record(priority: priority) do diff --git a/lib/new_relic/agent/transaction/segment.rb b/lib/new_relic/agent/transaction/segment.rb index 66c0cdf6a2..7f8eace5fa 100644 --- a/lib/new_relic/agent/transaction/segment.rb +++ b/lib/new_relic/agent/transaction/segment.rb @@ -79,6 +79,9 @@ def segment_complete end def record_span_event + # don't record a span event if the transaction is ignored + return if transaction.ignore? + aggregator = ::NewRelic::Agent.agent.span_event_aggregator priority = transaction.priority From 713bdbc560ed7edc66a3f884b36bfd02964e30bf Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 16 Nov 2021 14:36:54 -0800 Subject: [PATCH 2/6] add test for no span events recorded when using newrelic_ignore --- test/multiverse/suites/rails/ignore_test.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/multiverse/suites/rails/ignore_test.rb b/test/multiverse/suites/rails/ignore_test.rb index ff8c863667..0839599394 100644 --- a/test/multiverse/suites/rails/ignore_test.rb +++ b/test/multiverse/suites/rails/ignore_test.rb @@ -77,4 +77,11 @@ def test_apdex_ignored_if_ignored_in_parent_class assert_metrics_not_recorded("Apdex") end + + def test_ignored_transaction_does_not_record_span_events + get '/ignored/action_to_ignore' + + last_span_events = NewRelic::Agent.agent.span_event_aggregator.harvest![1] + assert_empty last_span_events + end end From a462b220a9714dd080c5dd667c8199bac3bb6036 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 16 Nov 2021 14:54:22 -0800 Subject: [PATCH 3/6] add test for not recording span events when transaction is ignored using ignore_url_regexes --- test/multiverse/suites/rails/ignore_test.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/multiverse/suites/rails/ignore_test.rb b/test/multiverse/suites/rails/ignore_test.rb index 0839599394..6abc6ffc4c 100644 --- a/test/multiverse/suites/rails/ignore_test.rb +++ b/test/multiverse/suites/rails/ignore_test.rb @@ -17,6 +17,10 @@ def action_to_ignore def action_to_ignore_apdex render body: "This too" end + + def action_not_ignored + render body: "Not this!" + end end class ParentController < ApplicationController @@ -84,4 +88,13 @@ def test_ignored_transaction_does_not_record_span_events last_span_events = NewRelic::Agent.agent.span_event_aggregator.harvest![1] assert_empty last_span_events end + + def test_ignored_by_ignore_url_regexes_does_not_record_span_events + with_config(:rules => { :ignore_url_regexes => ['/ignored/action_not_ignored'] }) do + get '/ignored/action_not_ignored' + + last_span_events = NewRelic::Agent.agent.span_event_aggregator.harvest![1] + assert_empty last_span_events + end + end end From 40763077bfdb7eeda0d5801951634bb7b2efda5e Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 16 Nov 2021 15:15:47 -0800 Subject: [PATCH 4/6] added test for ignored transaction not creating span events for all segment types and infinite tracing segments --- .../datastore_segment_test.rb | 19 +++++++++++++++++++ .../external_request_segment_test.rb | 19 +++++++++++++++++++ .../agent_integrations/segment_test.rb | 16 ++++++++++++++++ .../transaction/datastore_segment_test.rb | 19 +++++++++++++++++++ .../external_request_segment_test.rb | 17 +++++++++++++++++ .../agent/transaction/segment_test.rb | 15 +++++++++++++++ 6 files changed, 105 insertions(+) diff --git a/infinite_tracing/test/infinite_tracing/agent_integrations/datastore_segment_test.rb b/infinite_tracing/test/infinite_tracing/agent_integrations/datastore_segment_test.rb index be36cbca6f..2b5ff13e58 100644 --- a/infinite_tracing/test/infinite_tracing/agent_integrations/datastore_segment_test.rb +++ b/infinite_tracing/test/infinite_tracing/agent_integrations/datastore_segment_test.rb @@ -96,6 +96,25 @@ def test_non_sampled_segment_does_record_span_event assert_equal 2, span_events.size end + def test_ignored_transaction_does_not_record_span_event + span_events = generate_and_stream_segments do + in_web_transaction('wat') do |txn| + txn.stubs(:ignore?).returns(true) + + segment = Tracer.start_datastore_segment( + product: "SQLite", + operation: "select", + port_path_or_id: 1337807 + ) + + segment.start + advance_process_time(1.0) + segment.finish + end + end + + assert_equal 0, span_events.size + end end end end diff --git a/infinite_tracing/test/infinite_tracing/agent_integrations/external_request_segment_test.rb b/infinite_tracing/test/infinite_tracing/agent_integrations/external_request_segment_test.rb index 28ba96cf0b..95925f68ef 100644 --- a/infinite_tracing/test/infinite_tracing/agent_integrations/external_request_segment_test.rb +++ b/infinite_tracing/test/infinite_tracing/agent_integrations/external_request_segment_test.rb @@ -86,6 +86,25 @@ def test_non_sampled_segment_does_record_span_event assert_equal 2, span_events.size end + def test_ignored_transaction_does_not_record_span_event + span_events = generate_and_stream_segments do + in_transaction('wat') do |txn| + txn.stubs(:ignore?).returns(true) + + segment = Transaction::ExternalRequestSegment.new \ + "Typhoeus", + "http://remotehost.com/blogs/index", + "GET" + + txn.add_segment segment + segment.start + advance_process_time(1.0) + segment.finish + end + end + + assert_equal 0, span_events.size + end end end end diff --git a/infinite_tracing/test/infinite_tracing/agent_integrations/segment_test.rb b/infinite_tracing/test/infinite_tracing/agent_integrations/segment_test.rb index 5402873fd1..dcfe1f44fd 100644 --- a/infinite_tracing/test/infinite_tracing/agent_integrations/segment_test.rb +++ b/infinite_tracing/test/infinite_tracing/agent_integrations/segment_test.rb @@ -72,6 +72,22 @@ def test_non_sampled_segment_does_record_span_event assert_equal 2, span_events.size end + def test_ignored_transaction_does_not_record_span_event + span_events = generate_and_stream_segments do + in_transaction('wat') do |txn| + txn.stubs(:ignore?).returns(true) + + segment = Transaction::Segment.new 'Ummm' + txn.add_segment segment + segment.start + advance_process_time(1.0) + segment.finish + end + end + + assert_equal 0, span_events.size + end + def test_streams_multiple_segments total_spans = 5 segments = [] diff --git a/test/new_relic/agent/transaction/datastore_segment_test.rb b/test/new_relic/agent/transaction/datastore_segment_test.rb index 04cdc6ffcb..64eee53167 100644 --- a/test/new_relic/agent/transaction/datastore_segment_test.rb +++ b/test/new_relic/agent/transaction/datastore_segment_test.rb @@ -210,6 +210,25 @@ def test_non_sampled_segment_does_not_record_span_event assert_empty last_span_events end + def test_ignored_transaction_does_not_record_span_event + in_web_transaction('wat') do |txn| + txn.stubs(:ignore?).returns(true) + + segment = Tracer.start_datastore_segment( + product: "SQLite", + operation: "select", + port_path_or_id: 1337807 + ) + + segment.start + advance_process_time 1.0 + segment.finish + end + + last_span_events = NewRelic::Agent.agent.span_event_aggregator.harvest![1] + assert_empty last_span_events + end + def test_sampled_segment_records_span_event trace_id = nil txn_guid = nil diff --git a/test/new_relic/agent/transaction/external_request_segment_test.rb b/test/new_relic/agent/transaction/external_request_segment_test.rb index ec3d103e47..8076538f36 100644 --- a/test/new_relic/agent/transaction/external_request_segment_test.rb +++ b/test/new_relic/agent/transaction/external_request_segment_test.rb @@ -877,6 +877,23 @@ def test_non_sampled_segment_does_not_record_span_event assert_empty last_span_events end + def test_non_sampled_segment_does_not_record_span_event + in_transaction('wat') do |txn| + txn.stubs(:ignore?).returns(true) + + segment = ExternalRequestSegment.new "Typhoeus", + "http://remotehost.com/blogs/index", + "GET" + txn.add_segment segment + segment.start + advance_process_time 1.0 + segment.finish + end + + last_span_events = NewRelic::Agent.agent.span_event_aggregator.harvest![1] + assert_empty last_span_events + end + def test_span_event_truncates_long_value with_config(distributed_tracing_config) do in_transaction('wat') do |txn| diff --git a/test/new_relic/agent/transaction/segment_test.rb b/test/new_relic/agent/transaction/segment_test.rb index 3a4229d8ee..ba636c67cb 100644 --- a/test/new_relic/agent/transaction/segment_test.rb +++ b/test/new_relic/agent/transaction/segment_test.rb @@ -213,6 +213,21 @@ def test_non_sampled_segment_does_not_record_span_event assert_empty last_span_events end + def test_ignored_transaction_does_not_record_span_events + in_transaction('wat') do |txn| + txn.stubs(:ignore?).returns(true) + + segment = Segment.new 'Ummm' + txn.add_segment segment + segment.start + advance_process_time 1.0 + segment.finish + end + + last_span_events = NewRelic::Agent.agent.span_event_aggregator.harvest![1] + assert_empty last_span_events + end + def test_sampled_segment_records_span_event trace_id = nil txn_guid = nil From 59fa34da037190f3f96b34edf9f8326b402501da Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 16 Nov 2021 16:17:32 -0800 Subject: [PATCH 5/6] removed note on ignore_url_regexes config as that behavior has been fixed --- lib/new_relic/agent/configuration/default_source.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 03b4bd4718..b256b1321a 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1814,10 +1814,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :type => Array, :allowed_from_server => true, :transform => DefaultSource.method(:convert_to_regexp_list), - :description => 'Define transactions you want the agent to ignore, by specifying a list of patterns matching the URI you want to ignore.' \ - '*Note:* This will only ignore transaction events, not spans or traces from the same transation. See documentation on ' \ - '(Ignoring Specific Transactions)[https://docs.newrelic.com/docs/agents/ruby-agent/api-guides/ignoring-specific-transactions/#config-ignoring] ' \ - 'for more details.' + :description => 'Define transactions you want the agent to ignore, by specifying a list of patterns matching the URI you want to ignore.' }, :'synthetics.traces_limit' => { :default => 20, From 987369a92b9b4fa2600303399d45abde639ef973 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 16 Nov 2021 16:22:42 -0800 Subject: [PATCH 6/6] added changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 204dd94634..77aff85e6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ Template rendering using [Tilt](https://github.com/rtomayko/tilt) is now instrumented. See [PR #847](https://github.com/newrelic/newrelic-ruby-agent/pull/847) for details. + * **Bugfix: Span Events recorded when using newrelic_ignore** + + Previously, the agent was incorrectly recording span events only on transactions that should be ignored. This fix will prevent any span events from being created for transactions using newrelic_ignore, or ignored through the `rules.ignore_url_regexes` configuration option. + ## v8.1.0