fix: Check APPMAP_RECORDER_PROCESS_ALWAYS truthy value #168
Conversation
zermelo-wisen
force-pushed
the
fix/process-always-envar-truthy-value
branch
2 times, most recently
from
1f5eb4d to
02d7bc6
Compare
|
Hi @zermelo-wisen - I did a quick test of this branch to see if it was working based on my expectations, but I still seem to get a process recording no matter what the value being set as. Testing with the mern-ecommerce application. Basically no matter the value of that environment variable i still seem to have process recordings getting created. But perhaps i'm not fully understanding the scope of this change. Here is a loom where i explain more and show what i'm seeing. https://www.loom.com/share/237004ab6afd4988b21fd134823b0c0f?sid=36c49bdd-ff56-4f22-814a-143f0cd774ac |
src/message.ts
Outdated
| return chalk.magentaBright`(Λ) ` + (typeof message === "string" ? message : inspect(message)); | ||
| return ( | ||
| "pid:" + | ||
| process.pid + |
There was a problem hiding this comment.
This seems like leftover debugging? It's not very user-friendly.
There was a problem hiding this comment.
I added this with a temporary commit for @petecheslock to check to be sure. I'll remove it now.
src/recorder.ts
Outdated
| const processRecordingShouldAlwaysBeActive = "APPMAP_RECORDER_PROCESS_ALWAYS" in process.env; | ||
| const kAppmapRecorderProcessAlwaysEnvar = "APPMAP_RECORDER_PROCESS_ALWAYS"; | ||
| const processRecordingShouldAlwaysBeActive = | ||
| kAppmapRecorderProcessAlwaysEnvar in process.env && |
There was a problem hiding this comment.
I don't think you need this check, isTruthy() is going to correctly return false on undefined.
This commit updates the interpretation of the APPMAP_RECORDER_PROCESS_ALWAYS environment variable so that process recording remains active only if the variable is set to a truthy value (one of ‘true’, ‘1’, ‘on’, ‘yes’). Previously, the existence of the APPMAP_RECORDER_PROCESS_ALWAYS environment variable, regardless of its value, was interpreted as true.
zermelo-wisen
force-pushed
the
fix/process-always-envar-truthy-value
branch
from
daddc77 to
e640100
Compare
|
🎉 This PR is included in version 2.24.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR updates the interpretation of the
APPMAP_RECORDER_PROCESS_ALWAYSenvironment variable so that process recording remains active only if the variable is set to a truthy value (one of ‘true’, ‘1’, ‘on’, ‘yes’).Previously, the existence of the
APPMAP_RECORDER_PROCESS_ALWAYSenvironment variable, regardless of its value, was interpreted as true.Behavior regarding this environment variable is as follows:
Default Behavior (When
APPMAP_RECORDER_PROCESS_ALWAYSis not set to a truthy value)When the
APPMAP_RECORDER_PROCESS_ALWAYSenvironment variable is not set to one of the truthy values (true,1,on,yes),appmap-nodebehaves as follows:appmap-nodeinitially creates a process recording AppMap for the entire process. Ifappmap-nodedetects a request, a test or a remote recording, it abandons the process recording. Instead, for example, if requests are detected, it creates individual request recordings for each detected request. In this case, you end up with multiple request recordings (one per request) and no process recording. Otherwise there will be just one process recording AppMap.Behavior with
APPMAP_RECORDER_PROCESS_ALWAYSset to a truthy ValueWhen
APPMAP_RECORDER_PROCESS_ALWAYSis set to one of the truthy values (true,1,on,yes):appmap-nodecontinues to record the entire process even when a request, a test or a remote recording is detected. As a result, for example, if requests are detected, you get one process recording AppMap that covers the entire process and multiple request recording AppMaps, with one AppMap for each request.