getappmap / appmap-node

AppMap client agent for Node.js
Other
9 stars 4 forks source link

feat: Async context tracking #155

Closed zermelo-wisen closed 1 month ago

zermelo-wisen commented 1 month ago

Implement Async Tracking in AppMap-Node Agent Using AsyncLocalStorage

Problem

The current implementation of the AppMap-Node agent struggles with proper asynchronous tracking, leading to misleading call hierarchies. Specifically, the agent does not maintain the correct context of asynchronous computations, resulting in inaccuracies when representing operations that involve Promise or other asynchronous constructs.

For example:

function bar(x) { return x * 2; }

async function foo() {
  await setTimeout(() => 1, 100);
  console.log(bar(1));
}

foo();
console.log(bar(2));

In this case, the call to bar(1) is incorrectly represented as coming after bar(2) because the foo()'s context is lost early.

Analysis

The main issues with the current implementation are:

  1. Fixup After Promise Settlements: Return events are only fixed up when Promise settlements are observed.
  2. Early Context Closure: The context of an async function closes as soon as it returns, leading to incorrect parent-child relationships in the reported events.
  3. Immediate Event Streaming: Events are streamed as they come, which can disrupt the correct chronological context, especially with async functions.

We need to update the tracking mechanism to keep the context alive throughout the async function's execution and ensure that events are accurately reported.

Proposed Changes

  1. Context Preservation:

    • Use AsyncLocalStorage to preserve the execution context across asynchronous operations.
    • Ensure any asynchronous operations, such as await or callback procedures, maintain the correct parent context in the generated AppMaps.
  2. Event Streaming:

    • Amend the event streaming logic to respect asynchronous boundaries instead of streaming events immediately.
    • Queue events until the corresponding async context finalizes, ensuring the correct call hierarchy is maintained.

Detailed Changes

  1. Update Recording.ts:

    • Modify the function that handles function calls to use AsyncLocalStorage to store context information.
    • Adjust the logic that generates function return and exception events to account for async contexts.
  2. Adjust Event Generation Logic:

    • Ensure that the fixupPromise function correctly associates the context of async operations.
    • Keep async contexts open until the async function fully completes.
  3. Modify Event Emission:

    • Adjust how and when events are emitted to ensure they respect the actual call hierarchy within asynchronous flows.
  4. Tests and Validation:

    • Update the relevant tests in test/jest/asyncLib.test.js and other relevant test files to validate the accuracy of the reported call hierarchies.
    • Ensure extensive tests cover various async scenarios to validate the new tracking mechanism's effectiveness.

Making these adjustments will significantly improve the accuracy of asynchronous tracking in the AppMap-Node agent, providing clearer and more accurate call hierarchies and execution flows in AppMaps.

Resolves #103

zermelo-wisen commented 1 month ago

Fixes #103.

The main objective of this PR is to enhance the AppMap-Node agent's ability to track asynchronous operations accurately. The previous implementation faced challenges in correctly maintaining the context of asynchronous computations, leading to inaccurate call hierarchies.

Here are some changes made to achieve this:

Changes in AppMapStream.ts

Changes in Recording.ts

Changes in AppMapStream.test.ts and Recording.test.ts

Changes in prisma.ts

An example AppMap produced from the added test.

Before the async tracking feature

image

After the async tracking feature:

image
zermelo-wisen commented 1 month ago

Added the ability to control async tracking via the async_tracking_timout filed in configuration file and the APPMAP_ASYNC_TRACKING_TIMEOUT environment variable. The environment variable takes precedence over the config file value. If neither is set, a default timeout of 3000 milliseconds is applied. Setting the value to 0 disables async tracking entirely.

Effects of the timeout value can easily be seen with this test:

https://github.com/getappmap/appmap-node/blob/16a861fb0c4c288c1b321f2cba7d55faaf46c95c/test/simple.test.ts#L124-L149

https://github.com/getappmap/appmap-node/blob/11e8a081ae36478de2288d28401a992ad61517df/test/simple/asyncTimeout.mjs#L1-L16

appland-release commented 1 month ago

:tada: This PR is included in version 2.24.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: