Report heap usage status with sonar.javascript.node.debugMemory
#4294
Conversation
| @@ -151,6 +151,7 @@ | |||
| "bin/" | |||
| ], | |||
| "_moduleAliases": { | |||
| "@sonar/bridge": "lib/bridge/src", | |||
There was a problem hiding this comment.
I had to do this to be able to require memory in worker, although they are in the same dir. Not sure if there is easier / better way.
| @@ -30,6 +30,7 @@ export interface Context { | |||
| workDir: string; | |||
| shouldUseTypeScriptParserForJS: boolean; | |||
| sonarlint: boolean; | |||
| debugMemory?: boolean; | |||
There was a problem hiding this comment.
Did you mark the field as optional because some unit tests would fail otherwise?
There was a problem hiding this comment.
exactly, it's created in many places.
| @@ -109,6 +110,7 @@ if (parentPort) { | |||
|
|
|||
| case 'on-create-program': { | |||
| const { tsConfig } = data; | |||
| logHeapStatistics(); | |||
There was a problem hiding this comment.
Just to be sure that it's intentional, logs will only be printed when analyzing with programs, not with watch programs, right?
We might face a weird situation where SonarLint users see the log about out-of-memory suggesting to enable the property sonar.javascript.node.debugMemory. However, even if they try to enable it, they won't see heap status.
Maybe the following line should be executed conditionally, when getContext().sonarlint === false:
SonarJS/packages/bridge/src/memory.ts
Lines 70 to 72 in 0ef5749
What do you think?
There was a problem hiding this comment.
yes, you are right, however, it's quite a corner case, I would not overcomplicate this
| @@ -146,7 +146,7 @@ describe('server', () => { | |||
| await new Promise(r => setTimeout(r, 600)); | |||
| expect(server.listening).toBeFalsy(); | |||
|
|
|||
| expect(console.log).toHaveBeenCalledWith('DEBUG The bridge server shutted down'); | |||
| expect(console.log).toHaveBeenCalledWith('DEBUG The bridge server shut down'); | |||
There was a problem hiding this comment.
Thanks!
| @@ -20,11 +20,12 @@ const host = process.argv[3]; | |||
| const workDir = process.argv[4]; | |||
| const shouldUseTypeScriptParserForJS = process.argv[5] !== 'false'; | |||
| const sonarlint = process.argv[6] === 'true'; | |||
| const debugMemory = process.argv[7] === 'true'; | |||
|
|
|||
| let bundles = []; | |||
There was a problem hiding this comment.
Maybe we could get rid of the if-statement and replace the bundles-related lines with:
const bundles = process.argv[8]?.split(path.delimiter) ?? [];
Co-authored-by: Yassin Kammoun <52890329+yassin-kammoun-sonarsource@users.noreply.github.com>
|
SonarQube Quality Gate
|
Fixes #4256