elastic / elastic/apm-agent-nodejs
add warning and troubleshooting docs for accidental extremely deep span tree from lingering ended transaction
- Dominant language
- JavaScript
- Stars
- 594
- Forks
- 244
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 16
Description
PR #2929 describes a user case where, through reasonable usage of the Agent API to manually create transactions and spans, a lingering ended transaction accumulated a span tree so deep that it was problematic (it crashed the app with "maximum stack depth exceeded"). Comment https://github.com/elastic/apm-agent-nodejs/pull/2929#issuecomment-1255501285 summarizes, shows a possibly matching repro, and discusses some "fixes". Part of the fix was #2939 that changed the impl of the internal `maybeTime` function to no longer be recursive -- this avoids a "maximum stack depth exceeded" crash. However, the root issue remains: an accidental lingering ended transaction that accumulates spans.
The "fixing this" section of the linked comment is repeated here:
> # Fixing this
>
> Fixing that `maybeTime` recursion is good. However, it doesn't solve your issue (assuming this repro matches your issue). This case is leaking Spans -- which _could_ be a long term memory leak issue -- and a deep current span stack leads to poor CPU performance in the agent (inside its RunContext.js).
>
> Some thoughts on possible fixes/workarounds:
>
> First, I think the best thing would be to add the ".withTransaction()" and ".withSpan()" APIs to our agent API as suggested in #2611
>
> Second, we could consider emitting a warning for the user if it looks like we are hitting a pathological situation like this. E.g. in RunContext.enterSpan() there is a huge perf cost if frequently entering/leaving a very deep stack (copying the `_spans` array). A heuristic for this warning could be (a) `_spans` is deep and (b) the transaction ended long ago.
>
> Note that this isn't perfect. We could still be leaking Spans via the `span._timer._parent` linked list, even if we don't have a single deep RunContext stack. An alternate heuristic warning could be on Transaction.createSpan: if _buildSpans is large and transaction ended long ago, then likely in this case.
>
> Third, we could/should have a troubleshooting doc landing anchor for this situation. It discuss the issue somewhat and perhaps defers to a longer discussion on an issue (perhaps on #2611?).
This issue is about considering doing those "Second, ..." and "Third, ..." items. This situation can still occur. We should at the least warn about it, if we can detect it reasonably.
Contributor guide
Assessment
This issue has not been assessed yet.