feat: local workflow replay #42

cfraz89 · 2022-11-23T10:15:27Z

eventual replay <entry> <workflow> <execution>.

TODO

encode workflow entry file in the infra, so it doesn't need to be specified.
Ensure compiler tests still run

packages/@eventual/cli/src/local-runner-entry/orchestrator.ts

packages/@eventual/compiler/bin/eventual-bundle.js

packages/@eventual/compiler/src/eventual-bundle.ts

- Build workflow directly

…al-replay

packages/@eventual/aws-cdk/src/service.ts

sam-goodwin · 2022-11-25T12:50:48Z

packages/@eventual/aws-cdk/src/service.ts

+      "/executions/{executionId}/history": [
+        {
+          methods: [HttpMethod.GET],
+          entry: "executions/history.js",
+          config: (fn) => {
+            this.table.grantReadData(fn);
+          },
+        },
+      ],
+      "/executions/{executionId}/workflow-history": [
+        {
+          methods: [HttpMethod.GET],
+          entry: "executions/workflow-history.js",
+          config: (fn) => {
+            this.history.grantRead(fn);
+          },
+        },
+      ],


I've been thinking that we should bundle all the handlers into a single lambda function:

we should not over-create resources in the customer's account

minimize number of cold starts

minimize impact of account-wide concurrent executions

This is just for the API? Do we have a monolith lambda router we like to use?

How would a monolith lambda help with concurrent executions?

sam-goodwin · 2022-11-25T12:51:45Z

packages/@eventual/aws-runtime/src/handlers/api/executions/workflow-history.ts

+async function workflowHistory(
+  event: APIGatewayProxyEventV2
+): Promise<APIGatewayProxyResultV2<HistoryStateEvent[]>> {
+  const executionId = event.pathParameters?.executionId;
+  if (!executionId) {
+    return { statusCode: 400, body: `Missing executionId` };
+  }
+
+  const workflowClient = createWorkflowRuntimeClient(getService());
+  return workflowClient.getHistory(decodeExecutionId(executionId));
+}
+
+export const handler = middy(workflowHistory).use(errorMiddleware);


These are starting to look redundant. See my earlier comment - should we move all of these into a single handler?

What about a wrapper?

export const handler = withMiddy(workflowHandler);

packages/@eventual/compiler/src/eventual-bundle.ts

thantos · 2022-11-28T14:54:49Z

packages/@eventual/aws-cdk/src/service.ts

-        runtime: Runtime.NODEJS_16_X,
-        architecture: Architecture.ARM_64,
-        bundling: {
-          // https://github.com/aws/aws-cdk/issues/21329#issuecomment-1212336356
-          // cannot output as .mjs file as ulid does not support it.
-          mainFields: ["module", "main"],
-          esbuildArgs: {
-            "--conditions": "module,import,require",
-          },
-          metafile: true,
-        },
+        ...baseNodeFnProps,


Could we make a BaseNodeFn construct instead of merging in props?

thantos · 2022-11-28T15:00:48Z

packages/@eventual/aws-cdk/src/service.ts

+      "/executions/{executionId}/history": [
+        {
+          methods: [HttpMethod.GET],
+          entry: "executions/history.js",
+          config: (fn) => {
+            this.table.grantReadData(fn);
+          },
+        },
+      ],
+      "/executions/{executionId}/workflow-history": [
+        {
+          methods: [HttpMethod.GET],
+          entry: "executions/workflow-history.js",
+          config: (fn) => {
+            this.history.grantRead(fn);
+          },
+        },
+      ],


This is just for the API? Do we have a monolith lambda router we like to use?

How would a monolith lambda help with concurrent executions?

packages/@eventual/aws-cdk/src/service.ts

packages/@eventual/aws-runtime/src/handlers/api/executions/list.ts

thantos · 2022-11-28T15:08:06Z

packages/@eventual/aws-runtime/src/handlers/api/executions/workflow-history.ts

+async function workflowHistory(
+  event: APIGatewayProxyEventV2
+): Promise<APIGatewayProxyResultV2<HistoryStateEvent[]>> {
+  const executionId = event.pathParameters?.executionId;
+  if (!executionId) {
+    return { statusCode: 400, body: `Missing executionId` };
+  }
+
+  const workflowClient = createWorkflowRuntimeClient(getService());
+  return workflowClient.getHistory(decodeExecutionId(executionId));
+}
+
+export const handler = middy(workflowHistory).use(errorMiddleware);


What about a wrapper?

export const handler = withMiddy(workflowHandler);

packages/@eventual/cli/src/commands/replay.ts

packages/@eventual/cli/src/service-action.ts

packages/@eventual/cli/src/commands/replay.ts

Co-authored-by: Chris Fraser <chris@functionless.com>

packages/@eventual/compiler/src/eventual-bundle.ts

.vscode/settings.json

packages/@eventual/aws-env/package.json

packages/@eventual/aws-runtime/package.json

packages/@eventual/aws-runtime/src/execution-id.ts

packages/@eventual/cli/package.json

packages/@eventual/cli/src/api-ky.ts

sam-goodwin · 2022-11-29T11:14:05Z

packages/@eventual/cli/src/api-ky.ts

@@ -42,7 +45,7 @@ export async function apiKy(region?: string): Promise<KyInstance> {
            path: url.pathname,
            protocol: url.protocol,
            method: req.method.toUpperCase(),
-            body: (await req.body?.getReader().read())?.value,
+            body: req.body && (await streamToString(req.body)),


Why not req.text() ?

It consumes the body, leaving it unreadable for the actual request

Is that a problem? Is it used twice?

Yes, once for signing, and one to actually send the request

Looks like you overwrite the body in the new one with the signed body?

Also I think getReader.read() also consumes the stream.

Ok figured it out. When doing req.text() on node 14, polyfilled with node-fetch: it runs this in the consumeBody method:
data[INTERNALS].disturbed = true;
Which should be fine, since we're constructing a new request anyway. The new request uses a copy constructor, which unfortunately copied the disturbed flag, and hence would cause the new request to fail too. This was a quirk specific to node-fetch, on node 18, without the polyfill, text() was working fine.

I've changed the new request construction to just copy the minimum parameters it needs to work, url, method, headers, and body, so it doesnt copy the consumed flag. Now it is working fine on node 14.

packages/@eventual/compiler/src/build.ts

thantos · 2022-11-30T05:20:00Z

packages/@eventual/aws-runtime/src/clients/create.ts

+  //We box our cache in case our fn returns undefined
+  let resMap = new Map<any, { value: R }>();
+  return (...args) => {
+    let key = options?.cacheKey ? options.cacheKey(...args) : args;


Are we making use of the cache key concept?

Some of the function's we're memoing are parameterized. Although they may only ever currently be invoked with a single set of arguments currently, I think we're kind of breaking the function's contract by ignoring the parameters when memoing, and might lead to a nasty surprise down the road, so figured better to account for them.

packages/@eventual/aws-cdk/package.json

packages/@eventual/aws-cdk/src/service.ts

thantos · 2022-11-30T06:09:29Z

packages/@eventual/cli/src/api-ky.ts

@@ -42,7 +45,7 @@ export async function apiKy(region?: string): Promise<KyInstance> {
            path: url.pathname,
            protocol: url.protocol,
            method: req.method.toUpperCase(),
-            body: (await req.body?.getReader().read())?.value,
+            body: req.body && (await streamToString(req.body)),


Looks like you overwrite the body in the new one with the signed body?

thantos · 2022-11-30T06:16:11Z

packages/@eventual/cli/src/api-ky.ts

@@ -42,7 +45,7 @@ export async function apiKy(region?: string): Promise<KyInstance> {
            path: url.pathname,
            protocol: url.protocol,
            method: req.method.toUpperCase(),
-            body: (await req.body?.getReader().read())?.value,
+            body: req.body && (await streamToString(req.body)),


Also I think getReader.read() also consumes the stream.

packages/@eventual/cli/src/cli.ts

packages/@eventual/cli/src/commands/replay.ts

thantos · 2022-11-30T06:43:28Z

packages/@eventual/compiler/src/workflow-visitor.ts

+  "race",
+];
+
+export class OuterVisitor extends Visitor {


this is new? merge issue?

just moved from esbuild-plugin.ts isn't it?

thantos · 2022-12-01T08:33:09Z

packages/@eventual/aws-runtime/tsconfig.json

-    "module": "esnext",
-    "target": "esnext",
-    "moduleResolution": "node",


This needs to stay, may be why the bundle size grew too.

Moved these properites tsconfig-base.json a while ago, so they'll be consistent across all projects.
Also tweaked them a bit.
module - esnext - so esm is default.
target: es2019. so it''ll compile down to node 14 es support.
moduleResolution: nodenext. needed for esm
This file was identical between both commits in the bundle size comparison fwiw

packages/@eventual/aws-runtime/tsconfig.cjs.json

Chris Fraser added 2 commits November 23, 2022 13:44

Work on replay command

0d81928

Make replay command run to success

913e55b

sam-goodwin requested changes Nov 23, 2022

View reviewed changes

Chris Fraser added 8 commits November 24, 2022 13:41

Cleanup

3662531

Working debugging

f0d186b

- Restore bundler code now that its no longer used

aab814a

- Build workflow directly

Merge branch 'main' of github.com:functionless/eventual into feat/loc…

b37deae

…al-replay

Update cli to work again

6509826

Fix tail

a140497

Exclude @eventual/core from user workflow bundle

c045963

Move globals to globals file

2d0016d

sam-goodwin changed the title ~~feat: Local workflow replay~~ feat: local workflow replay Nov 25, 2022

sam-goodwin marked this pull request as ready for review November 25, 2022 12:47

sam-goodwin requested changes Nov 25, 2022

View reviewed changes

thantos suggested changes Nov 28, 2022

View reviewed changes

sam and others added 12 commits November 28, 2022 17:25

chore: merge replay branch with main (#54)

754500d

Co-authored-by: Chris Fraser <chris@functionless.com>

Merge branch 'main' into feat/local-replay

cf222f4

chore: refactor aws-env

65c3fa2

fix: tsconfig

f5ec1a5

chore: cjs

faf5547

Remove bundling logs

613cca6

Simplify applying api middleware

6adad19

Remove unused onError in serviceAction

0e3711b

Fix replay bundling,

8c2ce95

Add node-fetch back to pnpm-lock

4489c0c

Fix signing request body

6d18025

Fix workflow name

0da4766

sam-goodwin requested changes Nov 29, 2022

View reviewed changes

Chris Fraser added 2 commits November 30, 2022 09:33

Drop some dependencies

591b7e4

Fix replay debugger

26d1c4a

Chris Fraser added 4 commits November 30, 2022 13:05

Trim env package.json

da81a58

remove spew

054b8b2

Remove unnecessary rmrf function

f8b82f3

Re-implement memoizer and drop mem dependency

565713c

thantos suggested changes Nov 30, 2022

View reviewed changes

Chris Fraser added 7 commits December 1, 2022 09:49

remove random cli statement

ff097b5

eliminate aws-env package

fd71d96

Fix api-ky and import

62bcb74

Split out package.json to module/non-module for aws-runtime

6215a67

Fix aws-runtime build

6d26bec

swap to req.text, dont copy req (node-fetch copies consumed flag)

bd1e8ef

Copy request before reading text

827e554

thantos suggested changes Dec 1, 2022

View reviewed changes

Chris Fraser added 2 commits December 1, 2022 19:35

no longer set workflow.name

3b7aed1

Ensure src/package.json excluded from cjs

01c3136

thantos approved these changes Dec 1, 2022

View reviewed changes

thantos mentioned this pull request Dec 1, 2022

CLI: execution id commands do not work #58

Closed

Encode start parameters in json

20a7a5f

thantos merged commit f632589 into main Dec 2, 2022

thantos deleted the feat/local-replay branch June 16, 2023 15:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: local workflow replay #42

feat: local workflow replay #42

cfraz89 commented Nov 23, 2022 •

edited

Loading

sam-goodwin Nov 25, 2022

thantos Nov 28, 2022

sam-goodwin Nov 25, 2022

thantos Nov 28, 2022

thantos Nov 28, 2022

thantos Nov 28, 2022

thantos Nov 28, 2022

sam-goodwin Nov 29, 2022

cfraz89 Nov 29, 2022

sam-goodwin Nov 29, 2022

cfraz89 Nov 30, 2022

thantos Nov 30, 2022

thantos Nov 30, 2022

cfraz89 Dec 1, 2022

thantos Nov 30, 2022

cfraz89 Nov 30, 2022

thantos Nov 30, 2022

thantos Nov 30, 2022

thantos Nov 30, 2022

cfraz89 Nov 30, 2022

thantos Dec 1, 2022

cfraz89 Dec 1, 2022 •

edited

Loading

feat: local workflow replay #42

feat: local workflow replay #42

Conversation

cfraz89 commented Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfraz89 Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

cfraz89 commented Nov 23, 2022 •

edited

Loading

cfraz89 Dec 1, 2022 •

edited

Loading