-
Notifications
You must be signed in to change notification settings - Fork 600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kinesis instrumentation #2974
base: dev
Are you sure you want to change the base?
Kinesis instrumentation #2974
Conversation
SimpleCov Report
|
lib/new_relic/agent/instrumentation/aws_sdk_kinesis/instrumentation.rb
Outdated
Show resolved
Hide resolved
Co-authored-by: Kayla Reopelle <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks for all your hard work on this!
def get_arn(params) | ||
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&.dig(:stream_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how come the if is checking for params&.dig(:stream_name)
, but method is using just params[:stream_name]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I was using the params&.dig in part for safe navigation, but to your point this doesnt really make sense to dig and then assume the stream_name is top level. An oversight. Updating!
end | ||
|
||
def get_segment_name(method_name, params) | ||
return "#{KINESIS}/#{method_name}/#{params[:stream_name]}" if params&.dig(:stream_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question on the dig VS direct hash on stream_name
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated!
Add support for aws-sdk-kinesis.