From 28c8b5fb72d839ec7f7dd9134cfc922e47329c4d Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Fri, 1 Nov 2024 11:11:50 -0700 Subject: [PATCH 01/11] WIP: base instrumentation and tests --- .../agent/configuration/default_source.rb | 9 + .../agent/instrumentation/kinesis.rb | 22 + .../agent/instrumentation/kinesis/chain.rb | 21 + .../kinesis/instrumentation.rb | 72 +++ .../agent/instrumentation/kinesis/prepend.rb | 19 + test/multiverse/suites/kinesis/Envfile | 9 + .../suites/kinesis/config/newrelic.yml | 19 + .../kinesis/kinesis_instrumentation_test.rb | 476 ++++++++++++++++++ 8 files changed, 647 insertions(+) create mode 100644 lib/new_relic/agent/instrumentation/kinesis.rb create mode 100644 lib/new_relic/agent/instrumentation/kinesis/chain.rb create mode 100644 lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb create mode 100644 lib/new_relic/agent/instrumentation/kinesis/prepend.rb create mode 100644 test/multiverse/suites/kinesis/Envfile create mode 100644 test/multiverse/suites/kinesis/config/newrelic.yml create mode 100644 test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index d8e7389116..da469640e6 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1537,6 +1537,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.kinesis' => { + :default => 'auto', + :documentation_default => 'auto', + :public => true, + :type => String, + :dynamic_name => true, + :allowed_from_server => false, + :description => 'Controls auto-instrumentation of the kinesis library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' + }, :'instrumentation.ruby_kafka' => { :default => 'auto', :public => true, diff --git a/lib/new_relic/agent/instrumentation/kinesis.rb b/lib/new_relic/agent/instrumentation/kinesis.rb new file mode 100644 index 0000000000..650125fa66 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/kinesis.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 'kinesis/instrumentation' +require_relative 'kinesis/chain' +require_relative 'kinesis/prepend' + +DependencyDetection.defer do + named :kinesis + + depends_on do + defined?(Aws::Kinesis::Client) + end + executes do + if use_prepend? + prepend_instrument Aws::Kinesis::Client, NewRelic::Agent::Instrumentation::Kinesis::Prepend + else + chain_instrument NewRelic::Agent::Instrumentation::Kinesis::Chain + end + end +end diff --git a/lib/new_relic/agent/instrumentation/kinesis/chain.rb b/lib/new_relic/agent/instrumentation/kinesis/chain.rb new file mode 100644 index 0000000000..ddcb72edaf --- /dev/null +++ b/lib/new_relic/agent/instrumentation/kinesis/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 Kinesis::Chain + def self.instrument! + ::Kinesis.class_eval do + include NewRelic::Agent::Instrumentation::Kinesis + + alias_method(:build_request_without_new_relic, :build_request) + + def build_request(*args) + build_request_with_new_relic(*args) do + build_request_without_new_relic(*args) + end + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb b/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb new file mode 100644 index 0000000000..85b189f6de --- /dev/null +++ b/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb @@ -0,0 +1,72 @@ +# 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 + +# external_request segment? + +module NewRelic::Agent::Instrumentation + module Kinesis + INSTRUMENTED_METHODS = %w[ + add_tags_to_stream + create_stream + decrease_stream_retention_period + delete_stream + describe_limits + describe_stream + disable_enhanced_monitoring + enable_enhanced_monitoring + get_records + get_shard_iterator + increase_stream_retention_period + list_streams + list_tags_for_stream + merge_shards + put_record + put_records + remove_tags_from_stream + split_shard + update_shard_count + ].freeze + + KINESIS = 'Kinesis' + AWS_KINESIS_DATA_STREAMS = 'aws_kinesis_data_streams' + + def instrument_method_with_new_relic(method_name, *args) + return yield unless NewRelic::Agent::Tracer.tracing_enabled? + + NewRelic::Agent.record_instrumentation_invocation(KINESIS) + + 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 + + @nr_captured_request = nil # clear request just in case + begin + NewRelic::Agent::Tracer.capture_segment_error(segment) { yield } + ensure + segment&.add_agent_attribute('cloud.platform', AWS_KINESIS_DATA_STREAMS) + segment&.add_agent_attribute('name', segment_name(method_name, params)) + segment&.finish + end + end + + def build_request_with_new_relic(*args) + @nr_captured_request = yield + end + + def segment_name(method_name, params) + return "#{KINESIS}/#{method_name}/#{params[:stream_name]}" if params&.dig(:stream_name) + + "#{KINESIS}/#{method_name}" + rescue => e + NewRelic::Agent.logger.warn("Failed to create segment name: #{e}") + end + + def get_arn(params) + return params[:stream_arn] if params[:stream_arn] + + NewRelic::Agent::Aws.create_arn(KINESIS.downcase, "stream/#{params[:stream_name]}", config&.region) if params[:stream_name] + end + end +end diff --git a/lib/new_relic/agent/instrumentation/kinesis/prepend.rb b/lib/new_relic/agent/instrumentation/kinesis/prepend.rb new file mode 100644 index 0000000000..ccdf281bfc --- /dev/null +++ b/lib/new_relic/agent/instrumentation/kinesis/prepend.rb @@ -0,0 +1,19 @@ +# 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 Kinesis::Prepend + include NewRelic::Agent::Instrumentation::Kinesis + + INSTRUMENTED_METHODS.each do |method_name| + define_method(method_name) do |*args| + instrument_method_with_new_relic(method_name, *args) { super(*args) } + end + end + + def build_request(*args) + build_request_with_new_relic(*args) { super } + end + end +end diff --git a/test/multiverse/suites/kinesis/Envfile b/test/multiverse/suites/kinesis/Envfile new file mode 100644 index 0000000000..1c637ab775 --- /dev/null +++ b/test/multiverse/suites/kinesis/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-kinesis' +RB diff --git a/test/multiverse/suites/kinesis/config/newrelic.yml b/test/multiverse/suites/kinesis/config/newrelic.yml new file mode 100644 index 0000000000..14e9d8bb16 --- /dev/null +++ b/test/multiverse/suites/kinesis/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: + kinesis: <%= $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/kinesis/kinesis_instrumentation_test.rb b/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb new file mode 100644 index 0000000000..4c8a90b8f4 --- /dev/null +++ b/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb @@ -0,0 +1,476 @@ +# 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 'aws-sdk-kinesis' + +class KinesisInstrumentationTest < Minitest::Test + def setup + @client = create_client + Aws.config.update(stub_responses: true) + # NewRelic::Agent::Aws.stubs(:create_arn).returns('test-arn') + @stats_engine = NewRelic::Agent.instance.stats_engine + end + + def teardown + @client = nil + NewRelic::Agent.instance.stats_engine.clear_stats + end + + def create_client + Aws::Kinesis::Client.new(region: 'us-east-2') + end + + # def test_all_attributes_added_to_segment + # client = create_client + + # in_transaction do |txn| + # client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + # end + + # spans = harvest_span_events! + # span = spans[1][0] + + # assert_equal 'Kinesis/create_stream/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + # def test_delete_stream + # client = create_client + + # in_transaction do |txn| + # client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # client.delete_stream({ + # stream_name: 'deschutes_river' + # }) + # end + + # spans = harvest_span_events! + # span = spans[1][1] + + # assert_equal 'Kinesis/delete_stream/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + # def test_describe_stream + # client = create_client + + # in_transaction do |txn| + # client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # client.describe_stream({ + # stream_name: 'deschutes_river' + # }) + # end + + # spans = harvest_span_events! + # span = spans[1][1] + + # assert_equal 'Kinesis/describe_stream/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + # def test_list_streams + # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + # in_transaction do |txn| + # @client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # @client.list_streams + # end + + # spans = harvest_span_events! + # span = spans[1][1] + # end + + # assert_equal 'Kinesis/list_streams', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # refute span[2]['cloud.resource_id'] + # end + + # def test_add_tags_to_stream + # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + # in_transaction do |txn| + # @client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # @client.add_tags_to_stream({ + # stream_name: 'deschutes_river', + # tags: {'TagKey' => 'salmon'} + # }) + # end + # spans = harvest_span_events! + # span = spans[1][1] + # end + + # assert_equal 'Kinesis/add_tags_to_stream/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + # def test_list_tags_for_stream + # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + # in_transaction do |txn| + # @client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # @client.add_tags_to_stream({ + # stream_name: 'deschutes_river', + # tags: {'TagKey' => 'salmon'} + # }) + + # @client.list_tags_for_stream({ + # stream_name: 'deschutes_river' + # }) + # end + + # spans = harvest_span_events! + # span = spans[1][2] + # end + + # assert_equal 'Kinesis/list_tags_for_stream/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + # def test_remove_tags_from_stream + # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + # in_transaction do |txn| + # @client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # @client.add_tags_to_stream({ + # stream_name: 'deschutes_river', + # tags: {'TagKey' => 'salmon'} + # }) + + # @client.remove_tags_from_stream({ + # stream_name: 'deschutes_river', + # tag_keys: ['TagKey'] + # }) + # end + + # spans = harvest_span_events! + # span = spans[1][2] + # end + + # assert_equal 'Kinesis/remove_tags_from_stream/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + # def test_increase_stream_retention_period + # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + # in_transaction do |txn| + # @client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # @client.increase_stream_retention_period({ + # stream_name: 'deschutes_river', + # retention_period_hours: 1 + # }) + # end + + # spans = harvest_span_events! + # span = spans[1][1] + # end + + # assert_equal 'Kinesis/increase_stream_retention_period/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + # def test_decrease_stream_retention_period + # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + # in_transaction do |txn| + # @client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # @client.decrease_stream_retention_period({ + # stream_name: 'deschutes_river', + # retention_period_hours: 1 + # }) + # end + + # spans = harvest_span_events! + # span = spans[1][1] + # end + + # assert_equal 'Kinesis/decrease_stream_retention_period/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + # def test_put_record + # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + # in_transaction do |txn| + # @client.create_stream({ + # stream_name: 'deschutes_river', + # shard_count: 2 + # }) + + # @client.put_record({ + # stream_name: 'deschutes_river', + # data: 'little lava lake', + # partition_key: 'wickiup' + # }) + # end + + # spans = harvest_span_events! + # span = spans[1][1] + # end + + # assert_equal 'Kinesis/put_record/deschutes_river', span[0]['name'] + # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + # assert_equal 'test-arn', span[2]['cloud.resource_id'] + # end + + def test_put_records + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.put_records({ + stream_name: 'deschutes_river', + records: [ + { + data: 'spring chinook', + explicit_hash_key: 'HashKey', + partition_key: 'wickiup' + }, + { + data: 'summer steelhead', + explicit_hash_key: 'HashKey', + partition_key: 'wickiup' + } + ] + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/put_records/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_get_record + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.put_record({ + stream_name: 'deschutes_river', + data: 'little lava lake', + partition_key: 'wickiup' + }) + + @client.get_records({ + shard_iterator: 'shard_iterator', + stream_arn: 'arn:aws:kinesis:us-east-1:123456789012:stream/deschutes_river' + }) + end + + spans = harvest_span_events! + span = spans[1][2] + + assert_equal 'Kinesis/get_records', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'arn:aws:kinesis:us-east-1:123456789012:stream/deschutes_river', span[2]['cloud.resource_id'] + end + end + + def test_update_shard_count + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.update_shard_count({ + stream_name: 'deschutes_river', + target_shard_count: 4, + scaling_type: 'UNIFORM_SCALING' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/update_shard_count/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_split_shard + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.split_shard({ + stream_name: 'deschutes_river', + shard_to_split: 'shardId-000', + new_starting_hash_key: 'HashKey' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/split_shard/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_merge_shards + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.merge_shards({ + stream_name: 'deschutes_river', + shard_to_merge: 'shardId-000', + adjacent_shard_to_merge: 'shardId-001' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/merge_shards/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_get_shard_iterator + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.get_shard_iterator({ + stream_name: 'deschutes_river', + shard_id: 'shardId-000', + shard_iterator_type: 'TRIM_HORIZON' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/get_shard_iterator/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_enable_enhanced_monitoring + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.enable_enhanced_monitoring({ + stream_name: 'deschutes_river', + shard_level_metrics: ['ALL'] + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/enable_enhanced_monitoring/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_disable_enhanced_monitoring + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.disable_enhanced_monitoring({ + stream_name: 'deschutes_river', + shard_level_metrics: ['ALL'] + }) + end + + spans = harvest_span_events! + span = spans[0][1] + + assert_equal 'Kinesis/disable_enhanced_monitoring/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_describe_limits + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + @client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + @client.describe_limits + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/describe_limits', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + end + end +end From 308f7304da3f8bf333ce917506c27aee060f8ad2 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Wed, 6 Nov 2024 14:53:18 -0800 Subject: [PATCH 02/11] Tests and things --- .../agent/instrumentation/kinesis/chain.rb | 12 +- .../kinesis/instrumentation.rb | 7 +- .../agent/instrumentation/kinesis/prepend.rb | 4 - .../kinesis/kinesis_instrumentation_test.rb | 560 ++++++++++-------- 4 files changed, 313 insertions(+), 270 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/kinesis/chain.rb b/lib/new_relic/agent/instrumentation/kinesis/chain.rb index ddcb72edaf..f5d11c8062 100644 --- a/lib/new_relic/agent/instrumentation/kinesis/chain.rb +++ b/lib/new_relic/agent/instrumentation/kinesis/chain.rb @@ -5,14 +5,14 @@ module NewRelic::Agent::Instrumentation module Kinesis::Chain def self.instrument! - ::Kinesis.class_eval do + ::Aws::Kinesis::Client.class_eval do include NewRelic::Agent::Instrumentation::Kinesis + + NewRelic::Agent::Instrumentation::Kinesis::INSTRUMENTED_METHODS.each do |method_name| + alias_method("#{method_name}_without_new_relic".to_sym, method_name.to_sym) - alias_method(:build_request_without_new_relic, :build_request) - - def build_request(*args) - build_request_with_new_relic(*args) do - build_request_without_new_relic(*args) + 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 diff --git a/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb b/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb index 85b189f6de..d681c3515a 100644 --- a/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb @@ -35,13 +35,12 @@ def instrument_method_with_new_relic(method_name, *args) return yield unless NewRelic::Agent::Tracer.tracing_enabled? NewRelic::Agent.record_instrumentation_invocation(KINESIS) - + 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 - @nr_captured_request = nil # clear request just in case begin NewRelic::Agent::Tracer.capture_segment_error(segment) { yield } ensure @@ -51,10 +50,6 @@ def instrument_method_with_new_relic(method_name, *args) end end - def build_request_with_new_relic(*args) - @nr_captured_request = yield - end - def segment_name(method_name, params) return "#{KINESIS}/#{method_name}/#{params[:stream_name]}" if params&.dig(:stream_name) diff --git a/lib/new_relic/agent/instrumentation/kinesis/prepend.rb b/lib/new_relic/agent/instrumentation/kinesis/prepend.rb index ccdf281bfc..79b2e0bf2e 100644 --- a/lib/new_relic/agent/instrumentation/kinesis/prepend.rb +++ b/lib/new_relic/agent/instrumentation/kinesis/prepend.rb @@ -11,9 +11,5 @@ module Kinesis::Prepend instrument_method_with_new_relic(method_name, *args) { super(*args) } end end - - def build_request(*args) - build_request_with_new_relic(*args) { super } - end end end diff --git a/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb b/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb index 4c8a90b8f4..ff5a063b75 100644 --- a/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb +++ b/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb @@ -6,14 +6,10 @@ class KinesisInstrumentationTest < Minitest::Test def setup - @client = create_client Aws.config.update(stub_responses: true) - # NewRelic::Agent::Aws.stubs(:create_arn).returns('test-arn') - @stats_engine = NewRelic::Agent.instance.stats_engine end def teardown - @client = nil NewRelic::Agent.instance.stats_engine.clear_stats end @@ -21,244 +17,277 @@ def create_client Aws::Kinesis::Client.new(region: 'us-east-2') end - # def test_all_attributes_added_to_segment - # client = create_client - - # in_transaction do |txn| - # client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - # end - - # spans = harvest_span_events! - # span = spans[1][0] - - # assert_equal 'Kinesis/create_stream/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end - - # def test_delete_stream - # client = create_client - - # in_transaction do |txn| - # client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # client.delete_stream({ - # stream_name: 'deschutes_river' - # }) - # end - - # spans = harvest_span_events! - # span = spans[1][1] - - # assert_equal 'Kinesis/delete_stream/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end - - # def test_describe_stream - # client = create_client - - # in_transaction do |txn| - # client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # client.describe_stream({ - # stream_name: 'deschutes_river' - # }) - # end - - # spans = harvest_span_events! - # span = spans[1][1] - - # assert_equal 'Kinesis/describe_stream/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end - - # def test_list_streams - # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do - # in_transaction do |txn| - # @client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # @client.list_streams - # end - - # spans = harvest_span_events! - # span = spans[1][1] - # end - - # assert_equal 'Kinesis/list_streams', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # refute span[2]['cloud.resource_id'] - # end - - # def test_add_tags_to_stream - # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do - # in_transaction do |txn| - # @client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # @client.add_tags_to_stream({ - # stream_name: 'deschutes_river', - # tags: {'TagKey' => 'salmon'} - # }) - # end - # spans = harvest_span_events! - # span = spans[1][1] - # end - - # assert_equal 'Kinesis/add_tags_to_stream/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end - - # def test_list_tags_for_stream - # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do - # in_transaction do |txn| - # @client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # @client.add_tags_to_stream({ - # stream_name: 'deschutes_river', - # tags: {'TagKey' => 'salmon'} - # }) - - # @client.list_tags_for_stream({ - # stream_name: 'deschutes_river' - # }) - # end - - # spans = harvest_span_events! - # span = spans[1][2] - # end - - # assert_equal 'Kinesis/list_tags_for_stream/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end - - # def test_remove_tags_from_stream - # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do - # in_transaction do |txn| - # @client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # @client.add_tags_to_stream({ - # stream_name: 'deschutes_river', - # tags: {'TagKey' => 'salmon'} - # }) - - # @client.remove_tags_from_stream({ - # stream_name: 'deschutes_river', - # tag_keys: ['TagKey'] - # }) - # end - - # spans = harvest_span_events! - # span = spans[1][2] - # end - - # assert_equal 'Kinesis/remove_tags_from_stream/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end - - # def test_increase_stream_retention_period - # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do - # in_transaction do |txn| - # @client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # @client.increase_stream_retention_period({ - # stream_name: 'deschutes_river', - # retention_period_hours: 1 - # }) - # end - - # spans = harvest_span_events! - # span = spans[1][1] - # end - - # assert_equal 'Kinesis/increase_stream_retention_period/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end - - # def test_decrease_stream_retention_period - # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do - # in_transaction do |txn| - # @client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # @client.decrease_stream_retention_period({ - # stream_name: 'deschutes_river', - # retention_period_hours: 1 - # }) - # end - - # spans = harvest_span_events! - # span = spans[1][1] - # end - - # assert_equal 'Kinesis/decrease_stream_retention_period/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end - - # def test_put_record - # NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do - # in_transaction do |txn| - # @client.create_stream({ - # stream_name: 'deschutes_river', - # shard_count: 2 - # }) - - # @client.put_record({ - # stream_name: 'deschutes_river', - # data: 'little lava lake', - # partition_key: 'wickiup' - # }) - # end - - # spans = harvest_span_events! - # span = spans[1][1] - # end - - # assert_equal 'Kinesis/put_record/deschutes_river', span[0]['name'] - # assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] - # assert_equal 'test-arn', span[2]['cloud.resource_id'] - # end + def test_all_attributes_added_to_segment + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + end + + spans = harvest_span_events! + span = spans[1][0] + + assert_equal 'Kinesis/create_stream/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_delete_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.delete_stream({ + stream_name: 'deschutes_river' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/delete_stream/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_describe_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.describe_stream({ + stream_name: 'deschutes_river' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/describe_stream/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_list_streams + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.list_streams + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/list_streams', span[2]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + refute span[2]['cloud.resource_id'] + end + end + + def test_add_tags_to_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.add_tags_to_stream({ + stream_name: 'deschutes_river', + tags: {'TagKey' => 'salmon'} + }) + end + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/add_tags_to_stream/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_list_tags_for_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.add_tags_to_stream({ + stream_name: 'deschutes_river', + tags: {'TagKey' => 'salmon'} + }) + + client.list_tags_for_stream({ + stream_name: 'deschutes_river' + }) + end + + spans = harvest_span_events! + span = spans[1][2] + + assert_equal 'Kinesis/list_tags_for_stream/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_remove_tags_from_stream + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.add_tags_to_stream({ + stream_name: 'deschutes_river', + tags: {'TagKey' => 'salmon'} + }) + + client.remove_tags_from_stream({ + stream_name: 'deschutes_river', + tag_keys: ['TagKey'] + }) + end + + spans = harvest_span_events! + span = spans[1][2] + + assert_equal 'Kinesis/remove_tags_from_stream/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_increase_stream_retention_period + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.increase_stream_retention_period({ + stream_name: 'deschutes_river', + retention_period_hours: 1 + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/increase_stream_retention_period/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end + + def test_decrease_stream_retention_period + client = create_client + + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do + in_transaction do |txn| + txn.stubs(:sampled?).returns(true) + client.create_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.decrease_stream_retention_period({ + stream_name: 'deschutes_river', + retention_period_hours: 1 + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/decrease_stream_retention_period/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_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_stream({ + stream_name: 'deschutes_river', + shard_count: 2 + }) + + client.put_record({ + stream_name: 'deschutes_river', + data: 'little lava lake', + partition_key: 'wickiup' + }) + end + + spans = harvest_span_events! + span = spans[1][1] + + assert_equal 'Kinesis/put_record/deschutes_river', span[0]['name'] + assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] + end + end def test_put_records + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + txn.stubs(:sampled?).returns(true) + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - @client.put_records({ + client.put_records({ stream_name: 'deschutes_river', records: [ { @@ -278,27 +307,30 @@ def test_put_records spans = harvest_span_events! span = spans[1][1] - assert_equal 'Kinesis/put_records/deschutes_river', span[0]['name'] + assert_equal 'Kinesis/put_records/deschutes_river', span[2]['name'] assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] assert_equal 'test-arn', span[2]['cloud.resource_id'] end end def test_get_record + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + txn.stubs(:sampled?).returns(true) + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - @client.put_record({ + client.put_record({ stream_name: 'deschutes_river', data: 'little lava lake', partition_key: 'wickiup' }) - @client.get_records({ + client.get_records({ shard_iterator: 'shard_iterator', stream_arn: 'arn:aws:kinesis:us-east-1:123456789012:stream/deschutes_river' }) @@ -314,14 +346,17 @@ def test_get_record end def test_update_shard_count + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + txn.stubs(:sampled?).returns(true) + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - @client.update_shard_count({ + client.update_shard_count({ stream_name: 'deschutes_river', target_shard_count: 4, scaling_type: 'UNIFORM_SCALING' @@ -338,14 +373,17 @@ def test_update_shard_count end def test_split_shard + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + txn.stubs(:sampled?).returns(true) + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - @client.split_shard({ + client.split_shard({ stream_name: 'deschutes_river', shard_to_split: 'shardId-000', new_starting_hash_key: 'HashKey' @@ -362,14 +400,17 @@ def test_split_shard end def test_merge_shards + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + txn.stubs(:sampled?).returns(true) + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - @client.merge_shards({ + client.merge_shards({ stream_name: 'deschutes_river', shard_to_merge: 'shardId-000', adjacent_shard_to_merge: 'shardId-001' @@ -386,14 +427,17 @@ def test_merge_shards end def test_get_shard_iterator + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + txn.stubs(:sampled?).returns(true) + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - @client.get_shard_iterator({ + client.get_shard_iterator({ stream_name: 'deschutes_river', shard_id: 'shardId-000', shard_iterator_type: 'TRIM_HORIZON' @@ -410,14 +454,17 @@ def test_get_shard_iterator end def test_enable_enhanced_monitoring + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + txn.stubs(:sampled?).returns(true) + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - - @client.enable_enhanced_monitoring({ + + client.enable_enhanced_monitoring({ stream_name: 'deschutes_river', shard_level_metrics: ['ALL'] }) @@ -433,21 +480,24 @@ def test_enable_enhanced_monitoring end def test_disable_enhanced_monitoring + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + txn.stubs(:sampled?).returns(true) + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - @client.disable_enhanced_monitoring({ + client.disable_enhanced_monitoring({ stream_name: 'deschutes_river', shard_level_metrics: ['ALL'] }) end spans = harvest_span_events! - span = spans[0][1] + span = spans[1][1] assert_equal 'Kinesis/disable_enhanced_monitoring/deschutes_river', span[0]['name'] assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] @@ -456,14 +506,16 @@ def test_disable_enhanced_monitoring end def test_describe_limits + client = create_client + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| - @client.create_stream({ + client.create_stream({ stream_name: 'deschutes_river', shard_count: 2 }) - @client.describe_limits + client.describe_limits end spans = harvest_span_events! From cb65b75367318078582a3fca41f12f1655582c3f Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Thu, 21 Nov 2024 10:59:03 -0800 Subject: [PATCH 03/11] Wire up account_id for ARN --- .../agent/instrumentation/kinesis/instrumentation.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb b/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb index d681c3515a..d92e5e8384 100644 --- a/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb @@ -2,8 +2,6 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true -# external_request segment? - module NewRelic::Agent::Instrumentation module Kinesis INSTRUMENTED_METHODS = %w[ @@ -35,7 +33,7 @@ def instrument_method_with_new_relic(method_name, *args) return yield unless NewRelic::Agent::Tracer.tracing_enabled? NewRelic::Agent.record_instrumentation_invocation(KINESIS) - + params = args[0] segment = NewRelic::Agent::Tracer.start_segment(name: segment_name(method_name, params)) arn = get_arn(params) if params @@ -58,10 +56,16 @@ def segment_name(method_name, params) 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(KINESIS.downcase, "stream/#{params[:stream_name]}", config&.region) if params[:stream_name] + NewRelic::Agent::Aws.create_arn(KINESIS.downcase, "stream/#{params[:stream_name]}", config&.region, nr_account_id) if params[:stream_name] end end end From 202538aae9450d1b9442002be94be212afa402ba Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 26 Nov 2024 12:52:53 -0800 Subject: [PATCH 04/11] Rubocop - trailing whitespace --- lib/new_relic/agent/instrumentation/kinesis/chain.rb | 2 +- .../multiverse/suites/kinesis/kinesis_instrumentation_test.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/kinesis/chain.rb b/lib/new_relic/agent/instrumentation/kinesis/chain.rb index f5d11c8062..78527b1c3a 100644 --- a/lib/new_relic/agent/instrumentation/kinesis/chain.rb +++ b/lib/new_relic/agent/instrumentation/kinesis/chain.rb @@ -7,7 +7,7 @@ module Kinesis::Chain def self.instrument! ::Aws::Kinesis::Client.class_eval do include NewRelic::Agent::Instrumentation::Kinesis - + NewRelic::Agent::Instrumentation::Kinesis::INSTRUMENTED_METHODS.each do |method_name| alias_method("#{method_name}_without_new_relic".to_sym, method_name.to_sym) diff --git a/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb b/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb index ff5a063b75..c10d4b4f1c 100644 --- a/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb +++ b/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb @@ -19,7 +19,7 @@ def create_client def test_all_attributes_added_to_segment client = create_client - + NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do in_transaction do |txn| txn.stubs(:sampled?).returns(true) @@ -463,7 +463,7 @@ def test_enable_enhanced_monitoring stream_name: 'deschutes_river', shard_count: 2 }) - + client.enable_enhanced_monitoring({ stream_name: 'deschutes_river', shard_level_metrics: ['ALL'] From f725d793911c2f3c718ff34e7d44316598d5e6c3 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Dec 2024 11:02:34 -0800 Subject: [PATCH 05/11] Rename files and add message broker spans --- .../agent/configuration/default_source.rb | 4 +-- .../{kinesis.rb => aws_sdk_kinesis.rb} | 8 ++--- .../{kinesis => aws_sdk_kinesis}/chain.rb | 0 .../instrumentation.rb | 31 ++++++++++++++++--- .../{kinesis => aws_sdk_kinesis}/prepend.rb | 0 .../transaction/message_broker_segment.rb | 3 ++ .../{kinesis => aws_sdk_kinesis}/Envfile | 0 .../config/newrelic.yml | 2 +- .../kinesis_instrumentation_test.rb | 14 ++++++--- 9 files changed, 46 insertions(+), 16 deletions(-) rename lib/new_relic/agent/instrumentation/{kinesis.rb => aws_sdk_kinesis.rb} (75%) rename lib/new_relic/agent/instrumentation/{kinesis => aws_sdk_kinesis}/chain.rb (100%) rename lib/new_relic/agent/instrumentation/{kinesis => aws_sdk_kinesis}/instrumentation.rb (70%) rename lib/new_relic/agent/instrumentation/{kinesis => aws_sdk_kinesis}/prepend.rb (100%) rename test/multiverse/suites/{kinesis => aws_sdk_kinesis}/Envfile (100%) rename test/multiverse/suites/{kinesis => aws_sdk_kinesis}/config/newrelic.yml (88%) rename test/multiverse/suites/{kinesis => aws_sdk_kinesis}/kinesis_instrumentation_test.rb (95%) diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 92889ce67c..1d30666ae7 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1561,14 +1561,14 @@ def self.notify :allowed_from_server => false, :description => 'Controls auto-instrumentation of the aws_sdk_lambda library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' }, - :'instrumentation.kinesis' => { + :'instrumentation.aws_sdk_kinesis' => { :default => 'auto', :documentation_default => 'auto', :public => true, :type => String, :dynamic_name => true, :allowed_from_server => false, - :description => 'Controls auto-instrumentation of the kinesis library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' + :description => 'Controls auto-instrumentation of the aws_sdk_kinesis library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' }, :'instrumentation.ruby_kafka' => { :default => 'auto', diff --git a/lib/new_relic/agent/instrumentation/kinesis.rb b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis.rb similarity index 75% rename from lib/new_relic/agent/instrumentation/kinesis.rb rename to lib/new_relic/agent/instrumentation/aws_sdk_kinesis.rb index 650125fa66..59d82dd053 100644 --- a/lib/new_relic/agent/instrumentation/kinesis.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis.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 'kinesis/instrumentation' -require_relative 'kinesis/chain' -require_relative 'kinesis/prepend' +require_relative 'aws_sdk_kinesis/instrumentation' +require_relative 'aws_sdk_kinesis/chain' +require_relative 'aws_sdk_kinesis/prepend' DependencyDetection.defer do - named :kinesis + named :aws_sdk_kinesis depends_on do defined?(Aws::Kinesis::Client) diff --git a/lib/new_relic/agent/instrumentation/kinesis/chain.rb b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/chain.rb similarity index 100% rename from lib/new_relic/agent/instrumentation/kinesis/chain.rb rename to lib/new_relic/agent/instrumentation/aws_sdk_kinesis/chain.rb diff --git a/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb similarity index 70% rename from lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb rename to lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb index d92e5e8384..52412ded2f 100644 --- a/lib/new_relic/agent/instrumentation/kinesis/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb @@ -28,14 +28,26 @@ module Kinesis KINESIS = 'Kinesis' AWS_KINESIS_DATA_STREAMS = 'aws_kinesis_data_streams' + BROKER_METHODS = %w[put_record put_records get_records].freeze def instrument_method_with_new_relic(method_name, *args) return yield unless NewRelic::Agent::Tracer.tracing_enabled? NewRelic::Agent.record_instrumentation_invocation(KINESIS) - params = args[0] - segment = NewRelic::Agent::Tracer.start_segment(name: segment_name(method_name, params)) + + if BROKER_METHODS.include?(method_name) + stream_name = get_stream_name(params) + segment = NewRelic::Agent::Tracer.start_message_broker_segment( + action: method_name == 'get_records' ? :consume : :produce, + library: KINESIS, + destination_type: :stream, + destination_name: stream_name + ) + else + segment = NewRelic::Agent::Tracer.start_segment(name: get_segment_name(method_name, params)) + end + arn = get_arn(params) if params segment&.add_agent_attribute('cloud.resource_id', arn) if arn @@ -43,12 +55,11 @@ 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_DATA_STREAMS) - segment&.add_agent_attribute('name', segment_name(method_name, params)) segment&.finish end end - def segment_name(method_name, params) + def get_segment_name(method_name, params) return "#{KINESIS}/#{method_name}/#{params[:stream_name]}" if params&.dig(:stream_name) "#{KINESIS}/#{method_name}" @@ -56,6 +67,18 @@ def segment_name(method_name, params) NewRelic::Agent.logger.warn("Failed to create segment name: #{e}") end + def get_stream_name(params) + return params[:stream_name] if params&.dig(:stream_name) + + arn = get_arn(params) + + return arn.split('/').last if arn + rescue => e + NewRelic::Agent.logger.warn("Failed to get stream name: #{e}") + + 'unknown' + end + def nr_account_id return @nr_account_id if defined?(@nr_account_id) diff --git a/lib/new_relic/agent/instrumentation/kinesis/prepend.rb b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/prepend.rb similarity index 100% rename from lib/new_relic/agent/instrumentation/kinesis/prepend.rb rename to lib/new_relic/agent/instrumentation/aws_sdk_kinesis/prepend.rb diff --git a/lib/new_relic/agent/transaction/message_broker_segment.rb b/lib/new_relic/agent/transaction/message_broker_segment.rb index c511fed283..c11e244f52 100644 --- a/lib/new_relic/agent/transaction/message_broker_segment.rb +++ b/lib/new_relic/agent/transaction/message_broker_segment.rb @@ -15,6 +15,7 @@ class MessageBrokerSegment < Segment PRODUCE = 'Produce'.freeze QUEUE = 'Queue'.freeze PURGE = 'Purge'.freeze + STREAM = 'Stream'.freeze TEMP = 'Temp'.freeze TOPIC = 'Topic'.freeze UNKNOWN = 'Unknown'.freeze @@ -22,6 +23,7 @@ class MessageBrokerSegment < Segment DESTINATION_TYPES = [ :exchange, :queue, + :stream, :topic, :temporary_queue, :temporary_topic, @@ -37,6 +39,7 @@ class MessageBrokerSegment < Segment TYPES = { exchange: EXCHANGE, temporary_queue: QUEUE, + stream: STREAM, queue: QUEUE, temporary_topic: TOPIC, topic: TOPIC, diff --git a/test/multiverse/suites/kinesis/Envfile b/test/multiverse/suites/aws_sdk_kinesis/Envfile similarity index 100% rename from test/multiverse/suites/kinesis/Envfile rename to test/multiverse/suites/aws_sdk_kinesis/Envfile diff --git a/test/multiverse/suites/kinesis/config/newrelic.yml b/test/multiverse/suites/aws_sdk_kinesis/config/newrelic.yml similarity index 88% rename from test/multiverse/suites/kinesis/config/newrelic.yml rename to test/multiverse/suites/aws_sdk_kinesis/config/newrelic.yml index 14e9d8bb16..dfb4f28ac7 100644 --- a/test/multiverse/suites/kinesis/config/newrelic.yml +++ b/test/multiverse/suites/aws_sdk_kinesis/config/newrelic.yml @@ -6,7 +6,7 @@ development: monitor_mode: true license_key: bootstrap_newrelic_admin_license_key_000 instrumentation: - kinesis: <%= $instrumentation_method %> + aws_sdk_kinesis: <%= $instrumentation_method %> app_name: test log_level: debug host: 127.0.0.1 diff --git a/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb b/test/multiverse/suites/aws_sdk_kinesis/kinesis_instrumentation_test.rb similarity index 95% rename from test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb rename to test/multiverse/suites/aws_sdk_kinesis/kinesis_instrumentation_test.rb index c10d4b4f1c..f019c813c8 100644 --- a/test/multiverse/suites/kinesis/kinesis_instrumentation_test.rb +++ b/test/multiverse/suites/aws_sdk_kinesis/kinesis_instrumentation_test.rb @@ -10,6 +10,7 @@ def setup end def teardown + harvest_span_events! NewRelic::Agent.instance.stats_engine.clear_stats end @@ -105,7 +106,7 @@ def test_list_streams spans = harvest_span_events! span = spans[1][1] - assert_equal 'Kinesis/list_streams', span[2]['name'] + assert_equal 'Kinesis/list_streams', span[0]['name'] assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] refute span[2]['cloud.resource_id'] end @@ -270,7 +271,8 @@ def test_put_record spans = harvest_span_events! span = spans[1][1] - assert_equal 'Kinesis/put_record/deschutes_river', span[0]['name'] + assert_metrics_recorded(['MessageBroker/Kinesis/Stream/Produce/Named/deschutes_river']) + assert_equal 'MessageBroker/Kinesis/Stream/Produce/Named/deschutes_river', span[0]['name'] assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] assert_equal 'test-arn', span[2]['cloud.resource_id'] end @@ -307,13 +309,14 @@ def test_put_records spans = harvest_span_events! span = spans[1][1] - assert_equal 'Kinesis/put_records/deschutes_river', span[2]['name'] + assert_metrics_recorded(['MessageBroker/Kinesis/Stream/Produce/Named/deschutes_river']) + assert_equal 'MessageBroker/Kinesis/Stream/Produce/Named/deschutes_river', span[0]['name'] assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] assert_equal 'test-arn', span[2]['cloud.resource_id'] end end - def test_get_record + def test_get_records client = create_client NewRelic::Agent::Aws.stub(:create_arn, 'test-arn') do @@ -339,7 +342,8 @@ def test_get_record spans = harvest_span_events! span = spans[1][2] - assert_equal 'Kinesis/get_records', span[0]['name'] + assert_metrics_recorded(['MessageBroker/Kinesis/Stream/Consume/Named/deschutes_river']) + assert_equal 'MessageBroker/Kinesis/Stream/Consume/Named/deschutes_river', span[0]['name'] assert_equal 'aws_kinesis_data_streams', span[2]['cloud.platform'] assert_equal 'arn:aws:kinesis:us-east-1:123456789012:stream/deschutes_river', span[2]['cloud.resource_id'] end From e5914c25516d97744c009186ffaa446218d88b8b Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Dec 2024 12:28:19 -0800 Subject: [PATCH 06/11] Add CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 310ffab922..f52816c428 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## dev +- **Feature: Add instrumentation for aws-sdk-kinesis** + + The agent now has instrumentation for the [aws-sdk-kinesis](https://rubygems.org/gems/aws-sdk-kinesis) gem. [PR#2974](https://github.com/newrelic/newrelic-ruby-agent/pull/2974) + - **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) From 38d8148602c096cf8f19c612bcaa1e8c9c33ed46 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Dec 2024 12:36:29 -0800 Subject: [PATCH 07/11] Refactor --- .../agent/configuration/default_source.rb | 2 +- .../aws_sdk_kinesis/instrumentation.rb | 14 ++++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 399de4020c..d5163432f4 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1568,7 +1568,7 @@ def self.notify :type => String, :dynamic_name => true, :allowed_from_server => false, - :description => 'Controls auto-instrumentation of the aws_sdk_kinesis library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' + :description => 'Controls auto-instrumentation of the aws-sdk-kinesis library at start-up. May be one of `auto`, `prepend`, `chain`, `disabled`.' }, :'instrumentation.ruby_kafka' => { :default => 'auto', diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb index 52412ded2f..0cc26e9737 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb @@ -35,9 +35,10 @@ def instrument_method_with_new_relic(method_name, *args) NewRelic::Agent.record_instrumentation_invocation(KINESIS) params = args[0] + arn = get_arn(params) if params if BROKER_METHODS.include?(method_name) - stream_name = get_stream_name(params) + stream_name = get_stream_name(params, arn) segment = NewRelic::Agent::Tracer.start_message_broker_segment( action: method_name == 'get_records' ? :consume : :produce, library: KINESIS, @@ -48,7 +49,6 @@ def instrument_method_with_new_relic(method_name, *args) segment = NewRelic::Agent::Tracer.start_segment(name: get_segment_name(method_name, params)) end - arn = get_arn(params) if params segment&.add_agent_attribute('cloud.resource_id', arn) if arn begin @@ -67,16 +67,10 @@ def get_segment_name(method_name, params) NewRelic::Agent.logger.warn("Failed to create segment name: #{e}") end - def get_stream_name(params) - return params[:stream_name] if params&.dig(:stream_name) - - arn = get_arn(params) - - return arn.split('/').last if arn + def get_stream_name(params, arn) + params[:stream_name] || arn.split('/').last || 'unknown' rescue => e NewRelic::Agent.logger.warn("Failed to get stream name: #{e}") - - 'unknown' end def nr_account_id From 22ef84658c68f88071aa6973ee88d5d64979929d Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Tue, 17 Dec 2024 12:37:58 -0800 Subject: [PATCH 08/11] Update constant name --- .../agent/instrumentation/aws_sdk_kinesis/instrumentation.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb index 0cc26e9737..bef69a8524 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb @@ -28,7 +28,7 @@ module Kinesis KINESIS = 'Kinesis' AWS_KINESIS_DATA_STREAMS = 'aws_kinesis_data_streams' - BROKER_METHODS = %w[put_record put_records get_records].freeze + MESSAGE_BROKER_SEGMENT_METHODS = %w[put_record put_records get_records].freeze def instrument_method_with_new_relic(method_name, *args) return yield unless NewRelic::Agent::Tracer.tracing_enabled? @@ -37,7 +37,7 @@ def instrument_method_with_new_relic(method_name, *args) params = args[0] arn = get_arn(params) if params - if BROKER_METHODS.include?(method_name) + if MESSAGE_BROKER_SEGMENT_METHODS.include?(method_name) stream_name = get_stream_name(params, arn) segment = NewRelic::Agent::Tracer.start_message_broker_segment( action: method_name == 'get_records' ? :consume : :produce, From 1df32aa8a07a161cdbe02739ec7d9491d5d72735 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:13:49 -0800 Subject: [PATCH 09/11] Update CHANGELOG.md Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f52816c428..928ff41c5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ - **Feature: Add instrumentation for aws-sdk-kinesis** - The agent now has instrumentation for the [aws-sdk-kinesis](https://rubygems.org/gems/aws-sdk-kinesis) gem. [PR#2974](https://github.com/newrelic/newrelic-ruby-agent/pull/2974) + The agent now has instrumentation for the [aws-sdk-kinesis](https://rubygems.org/gems/aws-sdk-kinesis) gem. It will record message broker segments for `get_records`, `put_record`, and `put_records` operations. All other operations will record standard segments. [PR#2974](https://github.com/newrelic/newrelic-ruby-agent/pull/2974) - **Bugfix: Do not attempt to decorate logs with `nil` messages** From 39dfac2ec4005a4538d9c23aae81244d292e6742 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Wed, 18 Dec 2024 11:27:23 -0800 Subject: [PATCH 10/11] Use dig --- .../instrumentation/aws_sdk_kinesis/instrumentation.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb index bef69a8524..6164e45cbd 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb @@ -68,7 +68,7 @@ def get_segment_name(method_name, params) end def get_stream_name(params, arn) - params[:stream_name] || arn.split('/').last || 'unknown' + params&.dig(:stream_name) || arn.split('/').last || 'unknown' rescue => e NewRelic::Agent.logger.warn("Failed to get stream name: #{e}") end @@ -80,9 +80,9 @@ def nr_account_id end def get_arn(params) - return params[:stream_arn] if params[:stream_arn] + return params[:stream_arn] if params&.dig(:stream_arn) - NewRelic::Agent::Aws.create_arn(KINESIS.downcase, "stream/#{params[:stream_name]}", config&.region, nr_account_id) if params[:stream_name] + NewRelic::Agent::Aws.create_arn(KINESIS.downcase, "stream/#{params[:stream_name]}", config&.region, nr_account_id) if params&.dig(:stream_name) end end end From 78241a47f5226a7e31eae1e0a5def5df6962b93a Mon Sep 17 00:00:00 2001 From: Hannah Ramadan Date: Fri, 20 Dec 2024 14:05:32 -0800 Subject: [PATCH 11/11] update dig --- .../instrumentation/aws_sdk_kinesis/instrumentation.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb index 6164e45cbd..c4da1d3c04 100644 --- a/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb @@ -60,7 +60,8 @@ def instrument_method_with_new_relic(method_name, *args) end def get_segment_name(method_name, params) - return "#{KINESIS}/#{method_name}/#{params[:stream_name]}" if params&.dig(:stream_name) + stream_name = params&.dig(:stream_name) + return "#{KINESIS}/#{method_name}/#{stream_name}" if stream_name "#{KINESIS}/#{method_name}" rescue => e @@ -80,9 +81,11 @@ def nr_account_id end def get_arn(params) - return params[:stream_arn] if params&.dig(:stream_arn) + stream_arn = params&.dig(:stream_arn) + return stream_arn if stream_arn - NewRelic::Agent::Aws.create_arn(KINESIS.downcase, "stream/#{params[:stream_name]}", config&.region, nr_account_id) if params&.dig(:stream_name) + stream_name = params&.dig(:stream_name) + NewRelic::Agent::Aws.create_arn(KINESIS.downcase, "stream/#{stream_name}", config&.region, nr_account_id) if stream_name end end end