Skip to content

Commit

Permalink
Log memory configuration when bridge starts (#4263)
Browse files Browse the repository at this point in the history
  • Loading branch information
saberduck authored Oct 11, 2023
1 parent 215332f commit a50cad0
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.List;
import java.util.regex.Pattern;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -256,6 +257,32 @@ void jsFileNamedAsTsFile() {
);
}

@Test
void should_log_memory_config() {
var projectKey = "eslint_based_rules";
var projectDir = TestUtils.projectDir(projectKey);
var build = getSonarScanner()
.setProjectKey(projectKey)
.setSourceEncoding("UTF-8")
.setSourceDirs(".")
.setProperty("sonar.javascript.node.maxspace", "500000")
.setProjectDir(projectDir);

var buildResult = orchestrator.executeBuild(build);
assertThat(buildResult.isSuccess()).isTrue();
assertThat(buildResult.getLogs()).contains("Configured Node.js --max-old-space-size=500000.");
var osMem = Pattern.compile(
".*OS memory \\d+ MB\\. Node.js heap size limit: \\d+ MB\\..*",
Pattern.DOTALL
);
assertThat(buildResult.getLogs()).matches(osMem);
var warn = Pattern.compile(
".*WARN: Node.js heap size limit \\d+ is higher than available memory \\d+. Check your configuration of sonar\\.javascript\\.node\\.maxspace.*",
Pattern.DOTALL
);
assertThat(buildResult.getLogs()).matches(warn);
}

@NotNull
private static List<Issue> getIssueList(String projectKey, String ruleKey) {
SearchRequest request = new SearchRequest();
Expand Down
16 changes: 15 additions & 1 deletion packages/bridge/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ import express from 'express';
import http from 'http';
import router from './router';
import { errorMiddleware } from './errors';
import { debug } from '@sonar/shared/helpers';
import { debug, info, warn } from '@sonar/shared/helpers';
import { timeoutMiddleware } from './timeout';
import { AddressInfo } from 'net';
import * as v8 from 'v8';
import * as os from 'os';

/**
* The maximum request body size
Expand All @@ -44,6 +46,17 @@ const MAX_REQUEST_SIZE = '50mb';
*/
const SHUTDOWN_TIMEOUT = 15_000;

function logMemoryConfiguration() {
const osMem = Math.floor(os.totalmem() / 1_000_000);
const heapSize = Math.floor(v8.getHeapStatistics().heap_size_limit / 1_000_000);
info(`OS memory ${osMem} MB. Node.js heap size limit: ${heapSize} MB.`);
if (heapSize > osMem) {
warn(
`Node.js heap size limit ${heapSize} is higher than available memory ${osMem}. Check your configuration of sonar.javascript.node.maxspace`,
);
}
}

/**
* Starts the bridge
*
Expand All @@ -67,6 +80,7 @@ export function start(
host = '127.0.0.1',
timeout = SHUTDOWN_TIMEOUT,
): Promise<http.Server> {
logMemoryConfiguration();
return new Promise(resolve => {
debug(`starting the bridge server at port ${port}`);

Expand Down
10 changes: 7 additions & 3 deletions packages/bridge/tests/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,25 @@ describe('server', () => {
});

it('should start', async () => {
expect.assertions(4);
expect.assertions(5);

console.log = jest.fn();

const server = await start(undefined, undefined);
const close = promisify(server.close.bind(server));

expect(server.listening).toBeTruthy();
expect(console.log).toHaveBeenCalledTimes(2);
expect(console.log).toHaveBeenCalledTimes(3);
expect(console.log).toHaveBeenNthCalledWith(
1,
`DEBUG starting the bridge server at port ${port}`,
expect.stringMatching('OS memory \\d+ MB. Node.js heap size limit: \\d+ MB.'),
);
expect(console.log).toHaveBeenNthCalledWith(
2,
`DEBUG starting the bridge server at port ${port}`,
);
expect(console.log).toHaveBeenNthCalledWith(
3,
`DEBUG the bridge server is running at port ${(server.address() as AddressInfo)?.port}`,
);

Expand Down
8 changes: 8 additions & 0 deletions packages/shared/src/helpers/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,11 @@
export function debug(message: string) {
console.log(`DEBUG ${message}`);
}

export function info(message: string) {
console.log(message);
}

export function warn(message: string) {
console.log(`WARN ${message}`);
}
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ private static boolean isDifferent(InputStream newVersionIs, Path currentVersion
var newVersionString = new String(newVersionIs.readAllBytes(), StandardCharsets.UTF_8);
var currentVersionString = Files.readString(currentVersionPath);
LOG.debug(
"Currently installed Node.JS version: " +
"Currently installed Node.js version: " +
currentVersionString +
". Available version in analyzer: " +
newVersionString
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public NodeCommandBuilder configuration(Configuration configuration) {
@Override
public NodeCommandBuilder maxOldSpaceSize(int maxOldSpaceSize) {
nodeJsArgs("--max-old-space-size=" + maxOldSpaceSize);
LOG.info("Configured Node.js --max-old-space-size={}.", maxOldSpaceSize);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,9 @@ void test_version_check() {
void test_max_old_space_size_setting() throws IOException {
String request = "v8.getHeapStatistics()";
StringBuilder output = new StringBuilder();
int maxOldSpaceSize = 2048;
NodeCommand command = builder()
.maxOldSpaceSize(2048)
.maxOldSpaceSize(maxOldSpaceSize)
.nodeJsArgs("-p", request)
.outputConsumer(output::append)
.pathResolver(getPathResolver())
Expand All @@ -165,7 +166,9 @@ void test_max_old_space_size_setting() throws IOException {
command.waitFor();
Map map = new Gson().fromJson(output.toString(), Map.class);
double total_available_size = (double) map.get("total_available_size");
assertThat(total_available_size).isGreaterThan(2048 * 1000);
assertThat(logTester.logs(LoggerLevel.INFO))
.contains("Configured Node.js --max-old-space-size=" + maxOldSpaceSize + ".");
assertThat(total_available_size).isGreaterThan(maxOldSpaceSize * 1000);
}

@Test
Expand Down

0 comments on commit a50cad0

Please sign in to comment.