DataDog / datadog-static-analyzer

Datadog Static Analyzer
https://docs.datadoghq.com/static_analysis/
Apache License 2.0
100 stars 13 forks source link

Add option to specify heap memory limit to underlying JavaScript runtime #367

Open pranavpudasaini opened 6 months ago

pranavpudasaini commented 6 months ago

Is your feature request related to a problem? Please describe. While scanning a repo with 4000+ TypeScript files with a total of 100+ rules, it always crashes with the error: "Fatal JavaScript out of memory: Reached heap limit". I'm using a standard GitHub-hosted runner for private repos with 2 cores and 7 GiB memory to perform the test.

Describe the solution you'd like It would be awesome if there were an option to specify the maximum heap size for the underlying JavaScript runtime. Could be something equivalent to NODE_OPTIONS=--max_old_space_size=2048.

Describe alternatives you've considered I've tried specifying heap size by adding the command prefix NODE_OPTIONS=--max_old_space_size=2048 to the datadog-static-analyzer binary but the JS runtime doesn't seem to use it.

image

jasonforal commented 6 months ago

Thanks for the report.

We have been aware of this issue, and we are currently working on some changes that will (among other things) prevent this particular crash.

While we do not currently expose configuration options for memory usage, we may in the future--similarly to how we allow max CPU core configuration.

--

In the meantime, we have adjusted our default rulesets to temporarily disable some static analysis rules that exacerbate this issue. Hopefully that bandaid solves your particular issue, but if not, I'll update this issue when the fixes I mentioned get merged and released.

pranavpudasaini commented 6 months ago

@jasonforal Thank you for your response!

I've tried running the scans against the same repo but I'm still getting the same issue. We will switch to a larger runner instance until the heap configuration is available.

juli1 commented 6 months ago

@pranavpudasaini thank you for the report. Could you please run the analyzer with the option --debug yes (if you use the GitHub action, you can specify it as an input).

This will gives the output about which rule is failing and will help us finding the culprit.

pranavpudasaini commented 6 months ago

Hi @juli1, Thank you for the response!

Here are the few last rules that were evaluated just before the crash, the last one being no-duplicate-imports.

typescript-code-style/ban-tslint-comment
typescript-code-style/ban-ts-comment
typescript-code-style/no-confusing-non-null-assertion
typescript-code-style/no-empty-interface
typescript-code-style/no-inferrable-types
typescript-code-style/no-useless-empty-export
typescript-code-style/class-name
typescript-code-style/function-naming
typescript-code-style/parameter-name
typescript-code-style/method-name
typescript-code-style/no-duplicate-imports
juli1 commented 5 months ago

Hi @juli1, Thank you for the response!

Here are the few last rules that were evaluated just before the crash, the last one being no-duplicate-imports.

Any change you can share the file that causes the crash?

SophisticaSean commented 5 months ago

same issue is also happening to us and it's very frustrating

juli1 commented 5 months ago

We are working on removing all operations that trigger these issues. We should be done by end of month.

jasonforal commented 3 months ago

Hello @pranavpudasaini and @SophisticaSean, we released a new version of the analyzer (v0.3.7) that should circumvent the initial issue reported (this update greatly reduces v8 allocations, so hitting the heap limit should "never" happen under normal usage).

However, I will be keeping this issue open because v8 will still crash if it hits the heap limit -- we will eventually be adding functionality to gracefully handle this and treat it much like we do an execution timeout.