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

fix an infinite loop which happens when autometrics is initialized in a node service without a git repository #149

Merged
merged 3 commits into from
Feb 26, 2024

Conversation

benjibuiltit
Copy link
Contributor

after instrumenting my node / ts server with autometrics, things seemed to be working just fine locally.

unfortunately when i deployed to my dev environment (kubernetes cluster) i saw my service spike in cpu utilization and the event loop would hang causing all requests to time out.

this made it difficult to debug since i was unable to reproduce locally initially. i found that i could reproduce this behavior if i built the docker image and ran the container locally. from there i was able to run node with profiling flags to get the v8 log output. this pointed me to the readFileSync call inside the readClosest function and the issue became clear. there is an unhandled case on the node platform that will result in an infinite while loop if no git repository is initialized (as is the case in a container).

this fixes that, but i'm uncertain if the other differences found between the node and deno platforms should be ported as well.

additionally, i had a troublesome time getting the just build-all command to work when trying to prepare this for a proper release/pr. please let me know how to proceed with getting this merged in. it is currently blocking my adoption of autometrics which otherwise looks to be of great benefit to me.

node profile ouput (sample)

 [Bottom up (heavy) profile]:
  Note: percentage shows a share of a particular caller in the total
  amount of its parent calls.
  Callers occupying less than 1.0% are not shown.

   ticks parent  name
   9335   64.6%  /usr/local/bin/node
   4380   46.9%    LazyCompile: *uvException node:internal/errors:511:57
   4380  100.0%      LazyCompile: *readFileSync node:fs:468:22
   4371   99.8%        LazyCompile: *readClosest /app/node_modules/@autometrics/autometrics/index.cjs:566:21
   4371  100.0%          LazyCompile: ~detectRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:560:29
   4371  100.0%            LazyCompile: ~getRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:517:30
   3742   40.1%    LazyCompile: *readFileSync node:fs:468:22
   3735   99.8%      LazyCompile: *readClosest /app/node_modules/@autometrics/autometrics/index.cjs:566:21
   3735  100.0%        LazyCompile: ~detectRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:560:29
   3735  100.0%          LazyCompile: ~getRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:517:30
   3735  100.0%            LazyCompile: ~getDefaultValue /app/node_modules/@autometrics/autometrics/index.cjs:715:25
    299    3.2%    /usr/local/bin/node
    270   90.3%      LazyCompile: *readFileSync node:fs:468:22
    270  100.0%        LazyCompile: *readClosest /app/node_modules/@autometrics/autometrics/index.cjs:566:21
    270  100.0%          LazyCompile: ~detectRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:560:29
    270  100.0%            LazyCompile: ~getRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:517:30
     10    3.3%      LazyCompile: *uvException node:internal/errors:511:57
     10  100.0%        LazyCompile: *readFileSync node:fs:468:22
     10  100.0%          LazyCompile: *readClosest /app/node_modules/@autometrics/autometrics/index.cjs:566:21
     10  100.0%            LazyCompile: ~detectRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:560:29
    280    3.0%    LazyCompile: *readClosest /app/node_modules/@autometrics/autometrics/index.cjs:566:21
    280  100.0%      LazyCompile: ~detectRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:560:29
    280  100.0%        LazyCompile: ~getRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:517:30
    280  100.0%          LazyCompile: ~getDefaultValue /app/node_modules/@autometrics/autometrics/index.cjs:715:25
    280  100.0%            LazyCompile: ~recordBuildInfo /app/node_modules/@autometrics/autometrics/index.cjs:695:29
    170    1.8%    LazyCompile: *isErrorStackTraceLimitWritable node:internal/errors:205:40
    170  100.0%      LazyCompile: *uvException node:internal/errors:511:57
    170  100.0%        LazyCompile: *readFileSync node:fs:468:22
    167   98.2%          LazyCompile: *readClosest /app/node_modules/@autometrics/autometrics/index.cjs:566:21
    167  100.0%            LazyCompile: ~detectRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:560:29
      3    1.8%          Function: ^readClosest /app/node_modules/@autometrics/autometrics/index.cjs:566:21
      3  100.0%            LazyCompile: ~detectRepositoryUrl /app/node_modules/@autometrics/autometrics/index.cjs:560:29
    105    1.1%    Function: ^internalCompileFunction node:internal/vm:31:33
    104   99.0%      Function: ^wrapSafe node:internal/modules/cjs/loader:1154:18
    104  100.0%        Function: ^Module._compile node:internal/modules/cjs/loader:1210:37
    103   99.0%          Function: ^Module._extensions..js node:internal/modules/cjs/loader:1265:37
    102   99.0%            Function: ^Module.load node:internal/modules/cjs/loader:1107:33

just build-all error

~/Development/autometrics-ts/examples/fastify ~/Development/autometrics-ts
+ just build
yarn && yarn build
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[Error: ENOENT: no such file or directory, readlink '/Users/benji/Development/autometrics-ts/dist/autometrics/node_modules/@types/node'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'readlink',
  path: '/Users/benji/Development/autometrics-ts/dist/autometrics/node_modules/@types/node'
}
error: Recipe `build` failed on line 2 with exit code 1
error: Recipe `build-examples` failed with exit code 1

@brettimus
Copy link
Collaborator

really good catch! this must've been a pain to debug. i think the solution needs a slight tweak, will comment on the file itself, then look into the build-all command error

@brettimus
Copy link
Collaborator

approved but if you could fix the type annotation in the new util function that'd be great!

@brettimus
Copy link
Collaborator

alright i did get the just script to work locally, kinda hackily. what i did was:

  • delete node_modules (because i had a crufty copy)
  • update deno locally
  • bust all cached deno files with a bash one-liner find . -type f \( -name "*.ts" -o -name "*.js" \) -exec deno cache --reload {} +

after that, running just worked for me

@brettimus
Copy link
Collaborator

the plan for now is that we'll merge your PRs, then i'm going to open a separate one to prep a release. we should have these changes released later this week!

@brettimus brettimus merged commit 2d5e6e8 into autometrics-dev:main Feb 26, 2024
@benjibuiltit benjibuiltit deleted the FIX-INFINITE-LOOP branch February 26, 2024 14:14
@brettimus brettimus mentioned this pull request Feb 26, 2024
4 tasks
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.

None yet

3 participants