From e7a63440bd6114fef33bc6bd331469fe5599ac65 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Thu, 21 Nov 2024 10:54:10 -0800 Subject: [PATCH 01/10] Base Firehose Instrumentation --- .../agent/configuration/default_source.rb | 9 +++ .../agent/instrumentation/firehose.rb | 22 +++++++ .../agent/instrumentation/firehose/chain.rb | 21 ++++++ .../firehose/instrumentation.rb | 64 +++++++++++++++++++ .../agent/instrumentation/firehose/prepend.rb | 15 +++++ test/multiverse/suites/firehose/Envfile | 9 +++ .../suites/firehose/config/newrelic.yml | 19 ++++++ .../firehose/firehose_instrumentation_test.rb | 15 +++++ 8 files changed, 174 insertions(+) create mode 100644 lib/new_relic/agent/instrumentation/firehose.rb create mode 100644 lib/new_relic/agent/instrumentation/firehose/chain.rb create mode 100644 lib/new_relic/agent/instrumentation/firehose/instrumentation.rb create mode 100644 lib/new_relic/agent/instrumentation/firehose/prepend.rb create mode 100644 test/multiverse/suites/firehose/Envfile create mode 100644 test/multiverse/suites/firehose/config/newrelic.yml create mode 100644 test/multiverse/suites/firehose/firehose_instrumentation_test.rb diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 4bc5e3eee7..a8a49235a4 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1552,6 +1552,15 @@ def self.notify :allowed_from_server => false, :description => 'Controls auto-instrumentation of bunny at start-up. May be one of: `auto`, `prepend`, `chain`, `disabled`.' }, + :'instrumentation.firehose' => { + :default => 'auto', + :documentation_default => 'auto', + :public => true, + :type => String, + :dynamic_name => true, + :allowed_from_server => false, + :description => 'Controls auto-instrumentation of the firehose library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' + }, :'instrumentation.aws_sdk_lambda' => { :default => 'auto', :documentation_default => 'auto', diff --git a/lib/new_relic/agent/instrumentation/firehose.rb b/lib/new_relic/agent/instrumentation/firehose.rb new file mode 100644 index 0000000000..a3172396de --- /dev/null +++ b/lib/new_relic/agent/instrumentation/firehose.rb @@ -0,0 +1,22 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +require_relative 'firehose/instrumentation' +require_relative 'firehose/chain' +require_relative 'firehose/prepend' + +DependencyDetection.defer do + named :firehose + + depends_on do + defined?(Aws::Firehose::Client) + end + executes do + if use_prepend? + prepend_instrument Aws::Firehose::Client, NewRelic::Agent::Instrumentation::Firehose::Prepend + else + chain_instrument NewRelic::Agent::Instrumentation::Firehose::Chain + end + end +end diff --git a/lib/new_relic/agent/instrumentation/firehose/chain.rb b/lib/new_relic/agent/instrumentation/firehose/chain.rb new file mode 100644 index 0000000000..468a2143a2 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/firehose/chain.rb @@ -0,0 +1,21 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +module NewRelic::Agent::Instrumentation + module Firehose::Chain + def self.instrument! + ::Aws::Firehose::Client.class_eval do + include NewRelic::Agent::Instrumentation::Firehose + + NewRelic::Agent::Instrumentation::Firehose::INSTRUMENTED_METHODS.each do |method_name| + alias_method("#{method_name}_without_new_relic".to_sym, method_name.to_sym) + + define_method(method_name) do |*args| + instrument_method_with_new_relic(method_name, *args) { send("#{method_name}_without_new_relic".to_sym, *args) } + end + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb b/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb new file mode 100644 index 0000000000..bfbd4915e4 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb @@ -0,0 +1,64 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +module NewRelic::Agent::Instrumentation + module Firehose + INSTRUMENTED_METHODS = %w[ + create_delivery_stream + delete_delivery_stream + describe_delivery_stream + list_delivery_streams + list_tags_for_delivery_stream + put_record + put_record_batch + start_delivery_stream_encryption + stop_delivery_stream_encryption + tag_delivery_stream + untag_delivery_stream + update_destination + ].freeze + + FIREHOSE = 'Firehose' + AWS_KINESIS_DELIVERY_STREAMS = 'aws_kinesis_delivery_streams' + + def instrument_method_with_new_relic(method_name, *args) + return yield unless NewRelic::Agent::Tracer.tracing_enabled? + + NewRelic::Agent.record_instrumentation_invocation(FIREHOSE) + + params = args[0] + segment = NewRelic::Agent::Tracer.start_segment(name: segment_name(method_name, params)) + arn = get_arn(params) if params + segment&.add_agent_attribute('cloud.resource_id', arn) if arn + + begin + NewRelic::Agent::Tracer.capture_segment_error(segment) { yield } + ensure + segment&.add_agent_attribute('cloud.platform', AWS_KINESIS_DELIVERY_STREAMS) + segment&.add_agent_attribute('name', segment_name(method_name, params)) + segment&.finish + end + end + + def segment_name(method_name, params) + return "#{FIREHOSE}/#{method_name}/#{params[:delivery_stream_name]}" if params&.dig(:delivery_stream_name) + + "#{FIREHOSE}/#{method_name}" + rescue => e + NewRelic::Agent.logger.warn("Failed to create segment name: #{e}") + end + + def nr_account_id + return @nr_account_id if defined?(@nr_account_id) + + @nr_account_id = NewRelic::Agent::Aws.get_account_id(config) + end + + def get_arn(params) + return params[:stream_arn] if params[:stream_arn] + + NewRelic::Agent::Aws.create_arn(FIREHOSE.downcase, "stream/#{params[:delivery_stream_name]}", config&.region, nr_account_id) if params[:delivery_stream_name] + end + end +end diff --git a/lib/new_relic/agent/instrumentation/firehose/prepend.rb b/lib/new_relic/agent/instrumentation/firehose/prepend.rb new file mode 100644 index 0000000000..12a5dc192c --- /dev/null +++ b/lib/new_relic/agent/instrumentation/firehose/prepend.rb @@ -0,0 +1,15 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +module NewRelic::Agent::Instrumentation + module Firehose::Prepend + include NewRelic::Agent::Instrumentation::Firehose + + INSTRUMENTED_METHODS.each do |method_name| + define_method(method_name) do |*args| + instrument_method_with_new_relic(method_name, *args) { super(*args) } + end + end + end +end diff --git a/test/multiverse/suites/firehose/Envfile b/test/multiverse/suites/firehose/Envfile new file mode 100644 index 0000000000..8885958942 --- /dev/null +++ b/test/multiverse/suites/firehose/Envfile @@ -0,0 +1,9 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +instrumentation_methods :chain, :prepend + +gemfile <<~RB + gem 'aws-sdk-firehose' +RB diff --git a/test/multiverse/suites/firehose/config/newrelic.yml b/test/multiverse/suites/firehose/config/newrelic.yml new file mode 100644 index 0000000000..3b93087cb9 --- /dev/null +++ b/test/multiverse/suites/firehose/config/newrelic.yml @@ -0,0 +1,19 @@ +--- +development: + error_collector: + enabled: true + apdex_t: 0.5 + monitor_mode: true + license_key: bootstrap_newrelic_admin_license_key_000 + instrumentation: + firehose: <%= $instrumentation_method %> + app_name: test + log_level: debug + host: 127.0.0.1 + api_host: 127.0.0.1 + transaction_trace: + record_sql: obfuscated + enabled: true + stack_trace_threshold: 0.5 + transaction_threshold: 1.0 + capture_params: false diff --git a/test/multiverse/suites/firehose/firehose_instrumentation_test.rb b/test/multiverse/suites/firehose/firehose_instrumentation_test.rb new file mode 100644 index 0000000000..05c13a0019 --- /dev/null +++ b/test/multiverse/suites/firehose/firehose_instrumentation_test.rb @@ -0,0 +1,15 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +class FirehoseInstrumentationTest < Minitest::Test + def setup + @stats_engine = NewRelic::Agent.instance.stats_engine + end + + def teardown + NewRelic::Agent.instance.stats_engine.clear_stats + end + + # Add tests here +end From 13924cd1e5a23dca76c7095bd2cef24b144d098f Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 26 Nov 2024 11:42:50 -0800 Subject: [PATCH 02/10] Add tests --- .../firehose/instrumentation.rb | 2 +- .../firehose/firehose_instrumentation_test.rb | 316 +++++++++++++++++- 2 files changed, 314 insertions(+), 4 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb b/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb index bfbd4915e4..6539f84014 100644 --- a/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb @@ -56,7 +56,7 @@ def nr_account_id end def get_arn(params) - return params[:stream_arn] if params[:stream_arn] + return params[:delivery_stream_arn] if params[:delivery_stream_arn] NewRelic::Agent::Aws.create_arn(FIREHOSE.downcase, "stream/#{params[:delivery_stream_name]}", config&.region, nr_account_id) if params[:delivery_stream_name] end diff --git a/test/multiverse/suites/firehose/firehose_instrumentation_test.rb b/test/multiverse/suites/firehose/firehose_instrumentation_test.rb index 05c13a0019..7f4a1b0379 100644 --- a/test/multiverse/suites/firehose/firehose_instrumentation_test.rb +++ b/test/multiverse/suites/firehose/firehose_instrumentation_test.rb @@ -4,12 +4,322 @@ class FirehoseInstrumentationTest < Minitest::Test def setup - @stats_engine = NewRelic::Agent.instance.stats_engine + Aws.config.update(stub_responses: true) end def teardown NewRelic::Agent.instance.stats_engine.clear_stats end - # Add tests here -end + def create_client + Aws::Firehose::Client.new(region: 'us-east-2') + end + + def test_all_attributes_added_to_segment_in_create_delivery_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + end + + spans = harvest_span_events! + span = spans[1][0] + + assert_equal 'Firehose/create_delivery_stream/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_delete_delivery_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.delete_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/delete_delivery_stream/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_describe_delivery_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.describe_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/describe_delivery_stream/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_list_delivery_streams + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.list_delivery_streams + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/list_delivery_streams', span[2]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + refute span[2]['cloud.resource_id'] + end + end + + def test_tag_delivery_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.tag_delivery_stream({ + delivery_stream_name: 'the_shire', + tags: [{key: 'TagKey', value: 'salmon'}] + }) + end + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/tag_delivery_stream/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_list_tags_for_delivery_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.tag_delivery_stream({ + delivery_stream_name: 'the_shire', + tags: [{key: 'hobbit', value: 'frodo_baggins'}] + }) + + client.list_tags_for_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + end + + spans = harvest_span_events! + span = spans[1][2] + + assert_equal 'Firehose/list_tags_for_delivery_stream/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_untag_delivery_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.tag_delivery_stream({ + delivery_stream_name: 'the_shire', + tags: [{key: 'hobbit', value: 'frodo_baggins'}] + }) + + client.untag_delivery_stream({ + delivery_stream_name: 'the_shire', + tag_keys: ['hobbit'] + }) + end + + spans = harvest_span_events! + span = spans[1][2] + + assert_equal 'Firehose/untag_delivery_stream/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_put_record + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.put_record({ + delivery_stream_name: 'the_shire', + record: {data: 'samwise gamgee'} + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/put_record/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_put_record_batch + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.put_record_batch({ + delivery_stream_name: 'the_shire', + records: [ + { + data: 'legolas' + }, + { + data: 'gimli' + } + ] + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/put_record_batch/the_shire', span[2]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_start_delivery_stream_encryption + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.start_delivery_stream_encryption({ + delivery_stream_name: 'the_shire' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/start_delivery_stream_encryption/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_stop_delivery_stream_encryption + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.stop_delivery_stream_encryption({ + delivery_stream_name: 'the_shire' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/stop_delivery_stream_encryption/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_update_destination + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_delivery_stream({ + delivery_stream_name: 'the_shire' + }) + + client.update_destination({ + delivery_stream_name: 'the_shire', + current_delivery_stream_version_id: '1', + destination_id: '2', + s3_destination_update: { + bucket_arn: 'arn:aws:s3:::my-bucket', + role_arn: 'arn:aws:iam::123456789012:role/firehose_delivery_role' + } + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Firehose/update_destination/the_shire', span[0]['name'] + assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end +end \ No newline at end of file From 2a90bab2453fa6f21d509c96ef9a95ec953b1483 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 26 Nov 2024 12:51:09 -0800 Subject: [PATCH 03/10] Rubocop weewooo --- .../multiverse/suites/firehose/firehose_instrumentation_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/multiverse/suites/firehose/firehose_instrumentation_test.rb b/test/multiverse/suites/firehose/firehose_instrumentation_test.rb index 7f4a1b0379..314fb170ba 100644 --- a/test/multiverse/suites/firehose/firehose_instrumentation_test.rb +++ b/test/multiverse/suites/firehose/firehose_instrumentation_test.rb @@ -322,4 +322,4 @@ def test_update_destination assert_equal 'test-arn', span[2]['cloud.resource_id'] end end -end \ No newline at end of file +end From ce9a012f6391ba79ca757f8da1d5e157b0c63606 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Mon, 16 Dec 2024 15:38:09 -0800 Subject: [PATCH 04/10] Update Firehose Remove @name attribute, rename instrumentation --- lib/new_relic/agent/configuration/default_source.rb | 4 ++-- .../{firehose => aws_sdk_firehose}/chain.rb | 0 .../{firehose => aws_sdk_firehose}/instrumentation.rb | 1 - .../{firehose => aws_sdk_firehose}/prepend.rb | 0 lib/new_relic/agent/instrumentation/firehose.rb | 8 ++++---- .../suites/{firehose => aws_sdk_firehose}/Envfile | 0 .../{firehose => aws_sdk_firehose}/config/newrelic.yml | 2 +- .../firehose_instrumentation_test.rb | 4 ++-- 8 files changed, 9 insertions(+), 10 deletions(-) rename lib/new_relic/agent/instrumentation/{firehose => aws_sdk_firehose}/chain.rb (100%) rename lib/new_relic/agent/instrumentation/{firehose => aws_sdk_firehose}/instrumentation.rb (96%) rename lib/new_relic/agent/instrumentation/{firehose => aws_sdk_firehose}/prepend.rb (100%) rename test/multiverse/suites/{firehose => aws_sdk_firehose}/Envfile (100%) rename test/multiverse/suites/{firehose => aws_sdk_firehose}/config/newrelic.yml (88%) rename test/multiverse/suites/{firehose => aws_sdk_firehose}/firehose_instrumentation_test.rb (98%) diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index a8a49235a4..0b6b139997 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1552,14 +1552,14 @@ def self.notify :allowed_from_server => false, :description => 'Controls auto-instrumentation of bunny at start-up. May be one of: `auto`, `prepend`, `chain`, `disabled`.' }, - :'instrumentation.firehose' => { + :'instrumentation.aws_sdk_firehose' => { :default => 'auto', :documentation_default => 'auto', :public => true, :type => String, :dynamic_name => true, :allowed_from_server => false, - :description => 'Controls auto-instrumentation of the firehose library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' + :description => 'Controls auto-instrumentation of the aws_sdk_firehose library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' }, :'instrumentation.aws_sdk_lambda' => { :default => 'auto', diff --git a/lib/new_relic/agent/instrumentation/firehose/chain.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/chain.rb similarity index 100% rename from lib/new_relic/agent/instrumentation/firehose/chain.rb rename to lib/new_relic/agent/instrumentation/aws_sdk_firehose/chain.rb diff --git a/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb similarity index 96% rename from lib/new_relic/agent/instrumentation/firehose/instrumentation.rb rename to lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb index 6539f84014..65c22e9e5f 100644 --- a/lib/new_relic/agent/instrumentation/firehose/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb @@ -36,7 +36,6 @@ def instrument_method_with_new_relic(method_name, *args) NewRelic::Agent::Tracer.capture_segment_error(segment) { yield } ensure segment&.add_agent_attribute('cloud.platform', AWS_KINESIS_DELIVERY_STREAMS) - segment&.add_agent_attribute('name', segment_name(method_name, params)) segment&.finish end end diff --git a/lib/new_relic/agent/instrumentation/firehose/prepend.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/prepend.rb similarity index 100% rename from lib/new_relic/agent/instrumentation/firehose/prepend.rb rename to lib/new_relic/agent/instrumentation/aws_sdk_firehose/prepend.rb diff --git a/lib/new_relic/agent/instrumentation/firehose.rb b/lib/new_relic/agent/instrumentation/firehose.rb index a3172396de..82dc42f300 100644 --- a/lib/new_relic/agent/instrumentation/firehose.rb +++ b/lib/new_relic/agent/instrumentation/firehose.rb @@ -2,12 +2,12 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true -require_relative 'firehose/instrumentation' -require_relative 'firehose/chain' -require_relative 'firehose/prepend' +require_relative 'aws_sdk_firehose/instrumentation' +require_relative 'aws_sdk_firehose/chain' +require_relative 'aws_sdk_firehose/prepend' DependencyDetection.defer do - named :firehose + named :aws_sdk_firehose depends_on do defined?(Aws::Firehose::Client) diff --git a/test/multiverse/suites/firehose/Envfile b/test/multiverse/suites/aws_sdk_firehose/Envfile similarity index 100% rename from test/multiverse/suites/firehose/Envfile rename to test/multiverse/suites/aws_sdk_firehose/Envfile diff --git a/test/multiverse/suites/firehose/config/newrelic.yml b/test/multiverse/suites/aws_sdk_firehose/config/newrelic.yml similarity index 88% rename from test/multiverse/suites/firehose/config/newrelic.yml rename to test/multiverse/suites/aws_sdk_firehose/config/newrelic.yml index 3b93087cb9..fcaf6598ea 100644 --- a/test/multiverse/suites/firehose/config/newrelic.yml +++ b/test/multiverse/suites/aws_sdk_firehose/config/newrelic.yml @@ -6,7 +6,7 @@ development: monitor_mode: true license_key: bootstrap_newrelic_admin_license_key_000 instrumentation: - firehose: <%= $instrumentation_method %> + aws_sdk_firehose: <%= $instrumentation_method %> app_name: test log_level: debug host: 127.0.0.1 diff --git a/test/multiverse/suites/firehose/firehose_instrumentation_test.rb b/test/multiverse/suites/aws_sdk_firehose/firehose_instrumentation_test.rb similarity index 98% rename from test/multiverse/suites/firehose/firehose_instrumentation_test.rb rename to test/multiverse/suites/aws_sdk_firehose/firehose_instrumentation_test.rb index 314fb170ba..8fca1f0ebe 100644 --- a/test/multiverse/suites/firehose/firehose_instrumentation_test.rb +++ b/test/multiverse/suites/aws_sdk_firehose/firehose_instrumentation_test.rb @@ -99,7 +99,7 @@ def test_list_delivery_streams spans = harvest_span_events! span = spans[1][1] - assert_equal 'Firehose/list_delivery_streams', span[2]['name'] + assert_equal 'Firehose/list_delivery_streams', span[0]['name'] assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] refute span[2]['cloud.resource_id'] end @@ -239,7 +239,7 @@ def test_put_record_batch spans = harvest_span_events! span = spans[1][1] - assert_equal 'Firehose/put_record_batch/the_shire', span[2]['name'] + assert_equal 'Firehose/put_record_batch/the_shire', span[0]['name'] assert_equal 'aws_kinesis_delivery_streams', span[2]['cloud.platform'] assert_equal 'test-arn', span[2]['cloud.resource_id'] end From 663f60f76b97dc07fba76cb5705ed238453d3625 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Dec 2024 12:22:45 -0800 Subject: [PATCH 05/10] Update filename --- .../agent/instrumentation/{firehose.rb => aws_sdk_firehose.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename lib/new_relic/agent/instrumentation/{firehose.rb => aws_sdk_firehose.rb} (100%) diff --git a/lib/new_relic/agent/instrumentation/firehose.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose.rb similarity index 100% rename from lib/new_relic/agent/instrumentation/firehose.rb rename to lib/new_relic/agent/instrumentation/aws_sdk_firehose.rb From e1f7fd35bbf825c8d7f712430d4f7b689ea35325 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Dec 2024 14:31:18 -0800 Subject: [PATCH 06/10] Add CHANGELOG --- CHANGELOG.md | 4 ++++ lib/new_relic/agent/configuration/default_source.rb | 2 +- .../agent/instrumentation/aws_sdk_firehose/instrumentation.rb | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 310ffab922..f1afbb3709 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## dev +- **Feature: Add instrumentation for aws-sdk-firehose** + + The agent now has instrumentation for the [aws-sdk-firehose](https://rubygems.org/gems/aws-sdk-firehose) gem. [PR#2973](https://github.com/newrelic/newrelic-ruby-agent/pull/2973) + - **Bugfix: Do not attempt to decorate logs with `nil` messages** The agent no longer attempts to add New Relic linking metadata to logs with `nil` messages. Thank you, [@arlando](https://github.com/arlando) for bringing this to our attention! [Issue#2985](https://github.com/newrelic/newrelic-ruby-agent/issues/2985) [PR#2986](https://github.com/newrelic/newrelic-ruby-agent/pull/2986) diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 2ce8e93275..30fc0fe957 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1559,7 +1559,7 @@ def self.notify :type => String, :dynamic_name => true, :allowed_from_server => false, - :description => 'Controls auto-instrumentation of the aws_sdk_firehose library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' + :description => 'Controls auto-instrumentation of the aws-sdk-firehose library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' }, :'instrumentation.aws_sdk_lambda' => { :default => 'auto', diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb index 65c22e9e5f..ca63bd0da9 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb @@ -28,7 +28,7 @@ def instrument_method_with_new_relic(method_name, *args) NewRelic::Agent.record_instrumentation_invocation(FIREHOSE) params = args[0] - segment = NewRelic::Agent::Tracer.start_segment(name: segment_name(method_name, params)) + segment = NewRelic::Agent::Tracer.start_segment(name: get_segment_name(method_name, params)) arn = get_arn(params) if params segment&.add_agent_attribute('cloud.resource_id', arn) if arn @@ -40,7 +40,7 @@ def instrument_method_with_new_relic(method_name, *args) end end - def segment_name(method_name, params) + def get_segment_name(method_name, params) return "#{FIREHOSE}/#{method_name}/#{params[:delivery_stream_name]}" if params&.dig(:delivery_stream_name) "#{FIREHOSE}/#{method_name}" From 6332f8a1ba0702292c537eac53f4a6501fb43066 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Dec 2024 14:56:26 -0800 Subject: [PATCH 07/10] Update arn name --- .../agent/instrumentation/aws_sdk_firehose/instrumentation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb index ca63bd0da9..c3daf11a32 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb @@ -57,7 +57,7 @@ def nr_account_id def get_arn(params) return params[:delivery_stream_arn] if params[:delivery_stream_arn] - NewRelic::Agent::Aws.create_arn(FIREHOSE.downcase, "stream/#{params[:delivery_stream_name]}", config&.region, nr_account_id) if params[:delivery_stream_name] + NewRelic::Agent::Aws.create_arn(FIREHOSE.downcase, "deliverystream/#{params[:delivery_stream_name]}", config&.region, nr_account_id) if params[:delivery_stream_name] end end end From 15896b98d7c9db46a2f29768df635d42530bc4fa Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Fri, 20 Dec 2024 10:38:07 -0800 Subject: [PATCH 08/10] Use `dig` to prevent [] calls on nil --- .../agent/instrumentation/aws_sdk_firehose/instrumentation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb index c3daf11a32..3ee7863ec5 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb @@ -55,7 +55,7 @@ def nr_account_id end def get_arn(params) - return params[:delivery_stream_arn] if params[:delivery_stream_arn] + return params[:delivery_stream_arn] if params&.dig(:delivery_stream_arn) NewRelic::Agent::Aws.create_arn(FIREHOSE.downcase, "deliverystream/#{params[:delivery_stream_name]}", config&.region, nr_account_id) if params[:delivery_stream_name] end From e077f460d918ac65d34829151705ba0b76e0d2b7 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Fri, 20 Dec 2024 14:02:41 -0800 Subject: [PATCH 09/10] refactor dig --- .../instrumentation/aws_sdk_firehose/instrumentation.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb index 3ee7863ec5..1737555f0c 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb @@ -41,7 +41,8 @@ def instrument_method_with_new_relic(method_name, *args) end def get_segment_name(method_name, params) - return "#{FIREHOSE}/#{method_name}/#{params[:delivery_stream_name]}" if params&.dig(:delivery_stream_name) + stream_name = params&.dig(:delivery_stream_name) + return "#{FIREHOSE}/#{method_name}/#{stream_name}" if stream_name "#{FIREHOSE}/#{method_name}" rescue => e @@ -55,9 +56,10 @@ def nr_account_id end def get_arn(params) - return params[:delivery_stream_arn] if params&.dig(:delivery_stream_arn) + return params.dig(:delivery_stream_arn) if params&.dig(:delivery_stream_arn) - NewRelic::Agent::Aws.create_arn(FIREHOSE.downcase, "deliverystream/#{params[:delivery_stream_name]}", config&.region, nr_account_id) if params[:delivery_stream_name] + stream_name = params&.dig(:delivery_stream_name) + NewRelic::Agent::Aws.create_arn(FIREHOSE.downcase, "deliverystream/#{stream_name}", config&.region, nr_account_id) if stream_name end end end From 3605baa3c4f5e43acd0aa636728b3663b916a436 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Fri, 20 Dec 2024 14:06:35 -0800 Subject: [PATCH 10/10] refactor --- .../agent/instrumentation/aws_sdk_firehose/instrumentation.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb index 1737555f0c..d5ea186799 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_firehose/instrumentation.rb @@ -56,7 +56,8 @@ def nr_account_id end def get_arn(params) - return params.dig(:delivery_stream_arn) if params&.dig(:delivery_stream_arn) + stream_arn = params&.dig(:delivery_stream_arn) + return stream_arn if stream_arn stream_name = params&.dig(:delivery_stream_name) NewRelic::Agent::Aws.create_arn(FIREHOSE.downcase, "deliverystream/#{stream_name}", config&.region, nr_account_id) if stream_name