Skip to content
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

NewRelic::Agent::Instrumentation::ControllerInstrumentation .add_transaction_tracer method is missing **kwargs in its signature #671

Closed
H3JFC opened this issue May 11, 2021 · 2 comments · Fixed by #685
Labels
community To tag external issues and PRs submitted by the community

Comments

@H3JFC
Copy link

H3JFC commented May 11, 2021

Description

Considering the class Bar that looks like this:

class Bar
    def self.foo(logger: Logger.new(STDOUT))
      logger.info stuff: "logged stuff"
    end

    class << self
      include ::NewRelic::Agent::Instrumentation::ControllerInstrumentation
      add_transaction_tracer :foo, category: :task
    end
end

I get ArgumentError: wrong number of arguments (given 1, expected 0) when using the .add_transaction_tracer method on internal method foo if foo has a key word argument on ruby version 3.0.1 AND foo is invoked with key word arguments.

Expected Behavior

I expect not to get the ArgumentError.

Troubleshooting or [NR Diag]

NA

Steps to Reproduce

Run ruby bar.rb in my example project.

Your Environment

Ruby Version 3.0.1
newrelic_rpm Version 7.0.0

Additional context

To fix, consider adding **kwargs to the dynamic method signature here

It would look like so:

  class_eval <<-EOC
    def #{with_method_name}(*args, **kwargs, &block)
      perform_action_with_newrelic_trace(#{argument_list.join(',')}) do
        #{without_method_name}(*args, **kwargs, &block)
       end
    end
  EOC
@angelatan2 angelatan2 added the community To tag external issues and PRs submitted by the community label May 11, 2021
@tannalynn tannalynn moved this from Triage to Backlog in Ruby Engineering Board May 18, 2021
@tannalynn
Copy link
Contributor

@H3JFC Thank you for bringing this to our attention. We haven't received a report for this issue on this method yet. I haven't had a chance to really look into a potential fix yet, but unfortunately we won't be able to add **kwargs directly due to the range of ruby versions the agent supports (2.2-3.0) and on ruby <2.7 that results in a syntax error.
I think this is pretty similar to part of what we are needing to do in this issue #502 for add_method_tracer. I suspect we should be able to do something similar to the workaround mentioned there with ruby2_keywords. I'll put this in our backlog and talk to my team about when we can fit this in.

@beauraF
Copy link
Contributor

beauraF commented May 19, 2021

Hello @tannalynn, same issue here at Doctolib. I think you are right, ruby2_keywords should do the trick.

edit: I opened a PR on my fork, currently testing it on our code base, before submitting it to you.

@tannalynn tannalynn linked a pull request May 19, 2021 that will close this issue
5 tasks
Ruby Engineering Board automation moved this from Backlog to Done/Pending Release May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community To tag external issues and PRs submitted by the community
Projects
Archived in project
Ruby Engineering Board
  
Code Complete/Done
Development

Successfully merging a pull request may close this issue.

4 participants