Skip to content

Commit e6597b9

Browse files
authored
pkg(profiling-node): port profiling-node repo to monorepo (#10151)
Ports profiling-node to monorepo This should only require CI step changes and no changes to the source code (except inheriting from base configs in monorepo, but those were already ported to profiling-node so they shouldn't result in any actual changes to the codebase). TODO: - [x] verify pkg cmds work and we have no lint/ts issues. - [x] verify tests pass - [x] verify ci commands work - [x] prebuild binaries - [x] port build to rollup - [x] create e2e verdaccio config - [x] ensure e2e tests pass and the app can build. I'm opening this as ready to review. Currently e2e test on profiling fails as the package is missing. I'm not exactly sure why that is so hoping I can get a helping hand on that. The condition for a successful e2e test is to just execute a node script which triggers a profile and attempt to bundle profiling-node. The bundler test is there to ensure that we do in fact provide all of the statically required prebuild binaries and ensure bundlers can resolve them - else we risk breaking build tooling for folks bundling their code, which can be a common optimization in serverless environments. The good: - Node profiling can resolve some issues around types which kept falling out fo sync with the SDK - We can follow the changes in JS SDK and core packages along as well as their versioning - The integration between the rest of the packages is tighter and safer, as it should be impossible for the sentry-javascript packages we rely on now to break in profiling-node. The unfortunate: - CI timings will increase mostly because of the performance of windows runners. Installing repository dependencies on there takes roughly 10 minutes. I have tried leveraging the cache as much as I could, but since our dep paths are marked as `~/path/to/cached_dep` and `${{github.workspace}}/path/to/dep` it means they cannot be cached cross os as paths differ. I did not change those paths in this PR, but it can be an optimization as I suspect most of the packages are pure js and nothing would break. - When we pack artifacts in yarn build:tarball, we need skip profiling-node and assemble it separately. This is because we need to pull in the prebuilt binaries, else we'll only have the packed binary for the os we are running the action on. This same step needs to be replicated in e2e tests which prepare the tarball as well. Couple of things I had to fix: - Our build tooling was not windows compatible (util polyfill was using awrong os path separator which wound up creating an incompatible build output on windows) - CI is finicky. I learned the hard way that node-gyp configure and build need to be sequential, else all hell breaks loose and for reasons which I didn't bother to investigate, python path and tooling is never correctly resolved (even when specified via gyp arg)
1 parent 4bd82c9 commit e6597b9

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+6107
-11
lines changed

Diff for: .craft.yml

+3
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ targets:
4444
- name: npm
4545
id: '@sentry/node'
4646
includeNames: /^sentry-node-\d.*\.tgz$/
47+
- name: npm
48+
id: '@sentry/profiling-node'
49+
includeNames: /^sentry-profiling-node-\d.*\.tgz$/
4750

4851
## 3 Browser-based Packages
4952
- name: npm

0 commit comments

Comments
 (0)