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

Improve segment timing during transactions with concurrency #1884

Open
1 task
kaylareopelle opened this issue Mar 13, 2023 · 3 comments
Open
1 task

Improve segment timing during transactions with concurrency #1884

kaylareopelle opened this issue Mar 13, 2023 · 3 comments

Comments

@kaylareopelle
Copy link
Contributor

Description

Concurrent tasks are unique from non-concurrent tasks because their wall clock time and CPU time differ. Right now, both of these timings are being cut short when the transaction ends before the segment.

When a transaction finishes before its asynchronous segment, a warning message is logged:

# lib/new_relic/agent/transaction/abstract_segment.rb 

        def force_finish
          finish
          NewRelic::Agent.logger.warn("Segment: #{name} was unfinished at " \
            "the end of transaction. Timing information for this segment's " \
            "parent #{parent.name} in #{transaction.best_name} may be inaccurate.")
        end

Acceptance Criteria

  • Allow concurrent segments to complete after their transaction ends

Design Consideration/Limitations

  • All spans for a transaction aren’t sent until a transaction finishes
  • We would need to change the relationship in the harvest so that all spans were sent for the transaction when the spans were finished
  • We don't know how intertwined the end of a segment and the end of a transaction are
  • We're uncertain about what's beneficial for customers

How much of a problem is this for people? Mostly people are concerned there’s a warning they don’t understand.

Dependencies

  • Other agents may already work like this, we could understand how they behave

Estimates

L

@workato-integration
Copy link

@boomer196
Copy link

@kaylareopelle, hopping over to this issue from #1859.

@boomer196 - These warnings are most likely logged because an asynchronous process was running when the transaction completed. In this case the segment didn't finish through the standard path, but through a forced finish. The forced finish code path logs this warning.

This is a bit of technical debt around our asynchronous segment timing we've discussed, but have not addressed because we were uncertain about what solution would be beneficial for our customers.

I turned my notes on the team's prior discussions into this issue: #1884

What would you like to happen in this scenario?

I'm not sure what the best behavior would be, but for us, it appears to happen on every page load (controller/action), so in addition to it being "a warning we don't understand", it also becomes very "noisy" in our logs. Whichever direction this ends up going that would be something to keep in mind as well.

@kaylareopelle
Copy link
Contributor Author

A short-term solution we've considered is moving the log level for that message to debug if the log is related to the thread instrumentation. Would this work for your use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants