newrelic / newrelic-ruby-agent

New Relic RPM Ruby Agent
https://docs.newrelic.com/docs/apm/agents/ruby-agent/getting-started/introduction-new-relic-ruby/
Apache License 2.0
1.2k stars 598 forks source link

Investigate: Potential duplication of reported Sinatra errors #2413

Open hannahramadan opened 7 months ago

hannahramadan commented 7 months ago

It's possible the agent is reporting Sinatra errors twice if show_exceptions is enabled, once when notice_error is called via the perform_action_with_newrelic_trace inside the instrumentation method dispatch_with_tracing, and again in the ensure block under the same method:

# lib/new_relic/agent/instrumentation/sinatra/instrumentation.rb

def dispatch_with_tracing
  NewRelic::Agent.record_instrumentation_invocation(INSTRUMENTATION_NAME)

  request_params = get_request_params
  filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params || {})

  name = TransactionNamer.initial_transaction_name(request)
  perform_action_with_newrelic_trace(:category => :sinatra,
    :name => name,
    :params => filtered_params) do
    begin
      yield
    ensure
      # Will only see an error raised if :show_exceptions is true, but
      # will always see them in the env hash if they occur
      had_error = env.has_key?('sinatra.error')
      ::NewRelic::Agent.notice_error(env['sinatra.error']) if had_error
    end
  end
end

We should investigate whether this is the case and prevent duplicate reporting if so. Either way, :show_exceptions is only tested as a disabled value and we should add a test under the enabled condition.

workato-integration[bot] commented 7 months ago

https://new-relic.atlassian.net/browse/NR-218159