Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
| spanStackTraceMinDuration: 0, // Always have span stacktraces. | ||
| transport () { return new CapturingTransport() } |
There was a problem hiding this comment.
nit: Can drop these two. I'm guessing they are copied from span.test.js. They aren't used in this test. The CapturingTransport here is replaced with a mockClient in resetAgent() below.
There was a problem hiding this comment.
It was a nit, so totally fine to skip it, but is there a reason to keep those two config vars on the agent in this test file? They are slightly misleading for a future maintainer of this test file.
Co-authored-by: Trent Mick <trent.mick@elastic.co>
Co-authored-by: Trent Mick <trent.mick@elastic.co>
| spanStackTraceMinDuration: 0, // Always have span stacktraces. | ||
| transport () { return new CapturingTransport() } |
There was a problem hiding this comment.
It was a nit, so totally fine to skip it, but is there a reason to keep those two config vars on the agent in this test file? They are slightly misleading for a future maintainer of this test file.
No real intent here @trentm -- those configs are just part of whatever boilerplate I copy/pasted to get started with the tests and I didn't really want to think through whether removing them about have any impact. I don't think that's all that confusing -- (or no more so than other things in this codebase) -- but I don't have any objections if you wanted to remove them. |
* feat: Implement dropping fast exit spans. elastic#2301
Closes: #2301
Refs: elastic/apm#496
Introduces the
exitSpanMinDurationconfiguration value.Checklist