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

feat: custom transform for bundling and running suites #687

Merged

Conversation

vigneshshanmugam
Copy link
Member

@vigneshshanmugam vigneshshanmugam commented Dec 20, 2022

Summary

There are lot of changes on this PR and needs to be done at the same time so we can get all issues addressed at with sourcemap generation and retrieving them with proper stack trace, Will cover one by one to make it easier for the reviewers

Running test suites

  • Previously, the agent relied on ts-node for transpilation of the journey files which could be written in .ts, .js or .mjs files and prior to running the tests, the transformation of these files happens on the fly and resolved internally to the trasnspiled version. This was a good solution for the time being, but turned out to be problematic given the sourcemap locations were wrong most of the times as ts-node automatically installs the source-map-support module and we will not have granular access to the generated source files.
  • Now we rely on the custom transformation using esbuild to achieve similar behaviour, which means we get the same benefits like automatic transpilation of the TS files, configs, etc. with also granular control of the sourcemap that got generated for those files. This becomes super handy as we can reuse those sourcemaps and maps to those files after bundling using esbuild.

Bundling

  • Previously, we used esbuild in Bundling mode with a custom Synthetics plugin that resolved all the journey and imported modules without any transpilation and bundled only the external node_modules. This was done for main two reasons
    • Reduce the bundle size of the generated journey file - No sourcemaps
    • Replicate the file structure so the errors are captured with proper stacktrace information.

This method turned our to be really complex and we would need to handle some weird edge cases which is on another PR here - #673

  • The current solutions relies on Esbuild to bundle the full journey in a single bundled file and use sourcemaps that are inlined to map it to the source when errors are thrown. One thing to note here is we set sourceContents: false as we dont need the full source content right now as we are not replicating the file structure on the Kibana side, plus removing it also reduces the size by 90%. The rest is just base64 content mappings.

Making sourcemaps work

  • To ensure correct sourcemap gets generated during the running suites phase and also post bundling phase, we have to have a single source of truth. Meaning
    • Single tool should be used for transpiling the source journey files and bundling.
    • source-map-support should be installed only once, otherwise mapping back to the original source position would become problematic.
  • To avoid these problems, we use the custom transformation phase and store the sourcemaps in memory for each of the generated files and use them for retrieving later to find which journey was executed.
  • We rely on the correct source map also for journey, step, monitor functions as we use them to identify all the duplicate monitors and report them as errors.

Bug fixes

Failed journey as discussed on the issue #652

Screen Shot 2022-12-20 at 11 36 42 AM

@apmmachine
Copy link

apmmachine commented Dec 20, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-01-17T15:11:44.929+0000

  • Duration: 15 min 0 sec

Test stats 🧪

Test Results
Failed 0
Passed 206
Skipped 2
Total 208

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks great, but there's complex implementation details here that could really use comments. The why and how are well fleshed out in the issue description, can you copy that content into the codebase as comments as well?

__tests__/push/bundler.test.ts Outdated Show resolved Hide resolved
src/core/transform.ts Show resolved Hide resolved
return;
}

// Ignore entry points as these refer to the journey files
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love the simplification here

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

journey did not finish executing, 0 steps ran when calling exported function.
3 participants