fix(settings): It's not possible to switch the log level #13309
Conversation
Jenkins BuildsClick to see older builds (62)
|
|
But if I use a runtime CLI option or an ENV variable, will it override the database setting? |
|
Let me dot all i's and cross all t's. We have a few debug switchers (and there's a mess with this):
Currently this options are interconnected in a hard-to-track manner. Some use each other as default value, some override each other values in runtime. What to we want?We want:
ProposalThere're no requirements so far, so we can try to make them up ourselves. Proposal 1: separate controls (my opinion: 👎 )The easiest way is to separate the "log level" from "show ui debug controls".
This sounds too complicated to me. Most of the time we want to control these together. Proposal 2: define strict rules and dependencies (my opinion: 👍)
I believe this will keep all the "wants" and make things straightforward. |
|
Hi Igor 👋🏾 Amazing comment, I also like Proposal 2, because it will confuse the user even more to have separate controls for controlling 2 items (IsDebugEnabled, and LogLevel) which are really coupled. Since I agree with proposal 2 high level idea, and to make sure I understand correctly, let me add my questions quoting your points :
Should this be interpreted as, running the Status application and setting ENV variables or CLI should affect the values of IsDebugEnabled and LogLevel that could have been set through another mean (UI here), right? This make sense to me 👍🏾 Note : What is the precedence between CLI and ENV, I guess ENV is at the top right?
Here, it's a bit confusing, but I got the point, I like the disabling of the button all together. Because let's suppose I don't disable the button. If I set the IsDebugEnabled through CLI/ENV variable, then we fall into the current ticket situation where updating on the UI doesn't produce changes as the ENV variable is having a priority. This is the bug that I was trying to fix, and it is contradictory action if the disabling is excluded. Action : I can add explanation like you said at worse. And at best, I think disbaling the button in the ENV variable is ideal.
Fair enough, thanks for this. However, why this is related to
I am not sure I understand this point. Are you talking about the PROD --> INFO relationship or something else? If yes, this works already today even with my change. |
|
NOTE: in my reply I by runtime options I mean
Yep. And the database value should remain the same as it was.
Don't bother about this. This is already held automatically by the nim config library we use.
Yep!
I'd go with both.
Hmm. Yes, runtime options are temporary, for current session only. They shouldn't affect the
Yes, I'm about this. Yes, I believe it already works 🙂 |
|
Confirmed with Ben about what we want in terms of UI :
Result :Screencast.from.2024-01-30.05.04.17.PM.webm |
|
-[x] the default value to depend on the build type (Already worked) Verified that when building Status in release mode, example by overriding Line 515 in c122456 RESOURCES_LAYOUT ?= -d:production then the status application has default DebugEnabled false and the logs are INFO.
-[x] set desired log level from CLI option or ENV variable (NEW) Verified that when setting a CLI variable example -[x] enable "debug" from UI: this should switch (AFTER Restart) the logs to DEBUG and enable debug UI controls Solved |
kounkou
force-pushed
the
fix-debug-log-setting
branch
from
c122456
to
d4ffa01
Compare
kounkou
force-pushed
the
fix-debug-log-setting
branch
4 times, most recently
from
064fec4
to
10b0a83
Compare
kounkou
force-pushed
the
fix-debug-log-setting
branch
2 times, most recently
from
97759ca
to
5e10371
Compare
kounkou
force-pushed
the
fix-debug-log-setting
branch
2 times, most recently
from
f3fd6ac
to
797e596
Compare
kounkou
force-pushed
the
fix-debug-log-setting
branch
2 times, most recently
from
435a6bc
to
6efd68f
Compare
Closes #13139
kounkou
force-pushed
the
fix-debug-log-setting
branch
from
6efd68f
to
9edc9b7
Compare
kounkou
force-pushed
the
fix-debug-log-setting
branch
from
9edc9b7
to
52faea5
Compare
Previously it was not possible to change the state of the Debug toggle. This is because the code forced the setting the default value, ignoring the database setup, hence always setting the DEBUG as LogLevel. Closes #13139
kounkou
force-pushed
the
fix-debug-log-setting
branch
from
52faea5
to
5c7f76f
Compare
Description
Previously it was not possible to change the state of the Debug toggle in settings. This was because the code forced the setting the default value (DEBUG or INFO depending on the environment) at every login, ignoring the database setup, hence always setting the DEBUG or INFO as LogLevel.
Closes #13139
What does the PR do
The PR removes forced initialization to avoid overwriting the updated config in the node_config table in database and recently set by the user. To achieve this, the PR uses a newly created entity called
runTimeLogLevelthat isNOTupdated in Database, and that is used to set the logger in status-go. The PR still makes the Debug to be default in dev environments and disbales debug in prod environmentTest plan
Happy path ✅
No CLI, no ENV constant ✅
./bin/nim_status_clientThe application displays the last Database state
With env constant DEBUG ✅
export STATUS_RUNTIME_LOG_LEVEL="DEBUG" && ./bin/nim_status_client && unset STATUS_RUNTIME_LOG_LEVELThe application disables the Switch and set the Switch to Debug
With env constant INFO ✅
export STATUS_RUNTIME_LOG_LEVEL="INFO" && ./bin/nim_status_client && unset STATUS_RUNTIME_LOG_LEVELThe application disables the Switch and set the Switch to Info
With CLI parameter DEBUG ✅
./bin/nim_status_client --LOG_LEVEL=DEBUGThe application disables the switch and set the switch to Debug
With CLI parameter INFO ✅
./bin/nim_status_client --LOG_LEVEL=INFOThe application disables the switch and set the switch to INFO
Failure cases ✅
With wrong env constant
INfor instance ✅export STATUS_RUNTIME_LOG_LEVEL="IN" && ./bin/nim_status_client && unset STATUS_RUNTIME_LOG_LEVELThe application displays an error
With wrong CLI parameter
--for instance ✅./bin/nim_status_client --LOG_LEVEL=--The application displays an error
Affected areas
Setting Debug
Screenshot of functionality
Screencast.from.2024-01-25.02.06.00.PM.webm