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.

Sign up for GitHub

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

Jump to bottom

BLUE-273 Worker processed debug log flag #84

Merged
merged 1 commit into from
Sep 18, 2024
Merged

BLUE-273 Worker processed debug log flag #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export interface Config {
apiServerPort: number
txCronSchedule: string
}
workerProcessesDebugLog: boolean // To enable debug logs for worker processes managed by the main process
}

let config: Config = {
Expand Down Expand Up @@ -182,6 +183,7 @@ let config: Config = {
apiServerPort: 8084,
txCronSchedule: '*/5 * * * *',
},
workerProcessesDebugLog: false,
}
// Override default config params from config file, env vars, and cli args
export async function overrideDefaultConfig(file: string): Promise<void> {
Expand Down
37 changes: 22 additions & 15 deletions src/primary-process/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@
worker.kill()
}
if (receiptLoadTraker < config.receiptLoadTrakerLimit) {
console.log(`Receipt load is below the limit: ${receiptLoadTraker}/${config.receiptLoadTrakerLimit}`)
if (config.workerProcessesDebugLog)
console.log(`Receipt load is below the limit: ${receiptLoadTraker}/${config.receiptLoadTrakerLimit}`)

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI about 1 month ago

To fix the log injection issue, we need to sanitize the user-provided input before logging it. Specifically, we should remove any newline characters from the config.receiptLoadTrakerLimit value before including it in the log message. This can be done using String.prototype.replace to ensure no line endings are present in the user input.

Suggested changeset 1
src/primary-process/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/primary-process/index.ts b/src/primary-process/index.ts
--- a/src/primary-process/index.ts
+++ b/src/primary-process/index.ts
@@ -49,4 +49,6 @@
     if (receiptLoadTraker < config.receiptLoadTrakerLimit) {
-      if (config.workerProcessesDebugLog)
-        console.log(`Receipt load is below the limit: ${receiptLoadTraker}/${config.receiptLoadTrakerLimit}`)
+      if (config.workerProcessesDebugLog) {
+        const sanitizedLimit = config.receiptLoadTrakerLimit.toString().replace(/\n|\r/g, "");
+        console.log(`Receipt load is below the limit: ${receiptLoadTraker}/${sanitizedLimit}`);
+      }
       // Kill the extra workers from the end of the array
EOF
@@ -49,4 +49,6 @@
if (receiptLoadTraker < config.receiptLoadTrakerLimit) {
if (config.workerProcessesDebugLog)
console.log(`Receipt load is below the limit: ${receiptLoadTraker}/${config.receiptLoadTrakerLimit}`)
if (config.workerProcessesDebugLog) {
const sanitizedLimit = config.receiptLoadTrakerLimit.toString().replace(/\n|\r/g, "");
console.log(`Receipt load is below the limit: ${receiptLoadTraker}/${sanitizedLimit}`);
}
// Kill the extra workers from the end of the array
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
// Kill the extra workers from the end of the array
for (let i = workers.length - 1; i >= 0; i--) {
// console.log(`Killing worker ${workers[i].process.pid} with index ${i}`);
Expand All @@ -62,10 +63,12 @@
let neededWorkers = Math.ceil(receiptLoadTraker / config.receiptLoadTrakerLimit)
if (neededWorkers > MAX_WORKERS) neededWorkers = MAX_WORKERS
let currentWorkers = workers.length
console.log(`Needed workers: ${neededWorkers}`, `Current workers: ${currentWorkers}`)
if (config.workerProcessesDebugLog)
console.log(`Needed workers: ${neededWorkers}`, `Current workers: ${currentWorkers}`)
if (neededWorkers > currentWorkers) {
if (extraWorkers.size > 0) {
console.log(`Extra workers available: ${extraWorkers.size}, moving them to workers list`)
if (config.workerProcessesDebugLog)
console.log(`Extra workers available: ${extraWorkers.size}, moving them to workers list`)
// Move the extra workers to the workers list
for (const [pid, worker] of extraWorkers) {
workers.push(worker)
Expand All @@ -92,11 +95,12 @@
}
}
}
console.log(
`Adjusted worker count to ${
workers.length + newWorkers.size
}, based on ${receiptLoadTraker} receipts received.`
)
if (config.workerProcessesDebugLog)
console.log(
`Adjusted worker count to ${
workers.length + newWorkers.size
}, based on ${receiptLoadTraker} receipts received.`
)
receiptLoadTraker = 0 // Reset the count
}, config.receiptLoadTrakerInterval)
}
Expand Down Expand Up @@ -124,10 +128,11 @@
break
}
case 'child_close':
console.log(`Worker ${workerId} is requesting to close`)
if (config.workerProcessesDebugLog) console.log(`Worker ${workerId} is requesting to close`)
// Check if the worker is in the extraWorkers map
if (extraWorkers.has(workerId)) {
console.log(`Worker ${workerId} is in extraWorkers, killing it now`)
if (config.workerProcessesDebugLog)
console.log(`Worker ${workerId} is in extraWorkers, killing it now`)
const worker = extraWorkers.get(workerId)
if (worker) worker.kill()
} else {
Expand All @@ -144,7 +149,7 @@
}
break
case 'child_ready':
console.log(`Worker ${workerId} is ready for the duty`)
if (config.workerProcessesDebugLog) console.log(`Worker ${workerId} is ready for the duty`)
// Check if the worker is in the newWorkers map
if (newWorkers.has(workerId)) {
console.log(`Worker ${workerId} is in newWorkers, moving it to the workers list`)
Expand Down Expand Up @@ -172,7 +177,8 @@
console.log(`Worker ${worker.process.pid} died with code ${code} and signal ${signal}`)
let isExtraWorker = false
if (extraWorkers.has(workerId)) {
console.log(`Worker ${workerId} is in extraWorkers, removing it now`)
if (config.workerProcessesDebugLog)
console.log(`Worker ${workerId} is in extraWorkers, removing it now`)
isExtraWorker = true
extraWorkers.get(workerId)?.kill()
extraWorkers.delete(workerId)
Expand Down Expand Up @@ -246,8 +252,8 @@
}
if (workers.length === 0) {
mainProcessReceiptTracker++
console.log('Verifying on the main program 1', txId, timestamp)
verificationResult = await verifyArchiverReceipt(receipt, requiredSignatures)
if (config.workerProcessesDebugLog) console.log('Verifying on the main program 1', txId, timestamp)
verificationResult = await verifyArchiverReceipt(receipt, requiredSignatures)
mainProcessReceiptTracker--
} else {
mainProcessReceiptTracker = 0
Expand Down Expand Up @@ -276,7 +282,8 @@
verificationResult = await verifyArchiverReceipt(receipt, requiredSignatures)
}
} else {
console.log('Verifying on the worker process 1', txId, timestamp, worker.process.pid)
if (config.workerProcessesDebugLog)
console.log('Verifying on the worker process 1', txId, timestamp, worker.process.pid)
const cloneReceipt = Utils.deepCopy(receipt)
delete cloneReceipt.tx.originalTxData
delete cloneReceipt.executionShardKey
Expand Down
Loading