Add Roda support for newrelic_ignore* #2267
Conversation
hannahramadan
requested review from
fallwith,
kaylareopelle and
tannalynn
as code owners
|
|
||
| def test_regex_ignores_intended_route | ||
| get('/ignore_me') | ||
| get('/ignore_me_not') |
There was a problem hiding this comment.
Including both desired and undesired is a great approach!
| return false if !app.opts.include?(:newrelic_ignores) | ||
|
|
||
| app.opts[:newrelic_ignores][type].any? do |pattern| | ||
| pattern === app.request.path_info |
There was a problem hiding this comment.
Out of curiosity, why did you choose to use ===?
There was a problem hiding this comment.
Sometimes pattern is a string when a user specifies a route. But if they don't specify a route, then pattern becomes a regexp /.*/.
So sometimes we get string === string. And in string land == and === are alias of one another.
But in regexp land, == is for comparing regexps and === is for comparing regexps to strings, which app.request.path_info should always be.
I think there may have been a more clever way to write this by always turning strings into regexps, but this is where I landed. Open to alt ideas ◡̈
There was a problem hiding this comment.
I think this is a very clever solution! Thanks for breaking it down! 🙂
| def assert_enduser_ignored(response) | ||
| refute_match(/#{JS_AGENT_LOADER}/o, response.body) | ||
| end | ||
|
|
||
| def refute_enduser_ignored(response) | ||
| assert_match(/#{JS_AGENT_LOADER}/o, response.body) | ||
| end | ||
|
|
||
| def fake_html_for_browser_timing_header | ||
| '<html><head><title></title></head><body></body></html>' | ||
| end |
There was a problem hiding this comment.
Would it work for you to move these methods into the class that calls them? Optionally, you could make them private.
There was a problem hiding this comment.
Made them private since more than one class uses them 0a4c140
There was a problem hiding this comment.
This looks better 😎 3b755c1
|
|
||
| Rails 7.1 introduced the public API [`ActiveSupport::BroadcastLogger`](https://api.rubyonrails.org/classes/ActiveSupport/BroadcastLogger.html). This logger replaces a private API, `ActiveSupport::Logger.broadcast`. In Rails versions below 7.1, the agent uses the `broadcast` method to stop duplicate logs from being recoded by broadcasted loggers. Now, we've updated the code to provide a similar duplication fix for the `ActiveSupport::BroadcastLogger` class. [PR#2252](https://github.com/newrelic/newrelic-ruby-agent/pull/2252) | ||
|
|
||
| - **Fix: Resolve Sidekiq 8.0 error handler deprecation warning** | ||
| - **Bugfix: Resolve Sidekiq 8.0 error handler deprecation warning** |
| - `newrelic_ignore_apdex`: exclude a given route from consideration in overall Apdex calculations. | ||
| - `newrelic_ignore_enduser`: prevent automatic injection of the page load timing JavaScript when a route is rendered. | ||
|
|
||
| For more information, see [Roda Instrumentation](https://docs.newrelic.com/docs/apm/agents/ruby-agent/instrumented-gems/roda-instrumentation/). [PR#2267](https://github.com/newrelic/newrelic-ruby-agent/pull/2267) |
There was a problem hiding this comment.
Do you have a separate issue to track updating these docs to include the ignore methods?
There was a problem hiding this comment.
I don't yet, but it's next on the list! Happy to hold this merge until we can get a docs PR going.
SimpleCov Report
|
Add the ability for New Relic to ignore particular requests within Roda applications using:
newrelic_ignorenewrelic_ignore_apdexnewrelic_ignore_enduserCloses #2165