feat(app): #70 Add logging #252
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Docker image from this PR (0513395) created |
used a wrong variable name, and got an error that variable did not exist
AlexAxthelm
changed the title
web_tool_script_1.R
|
I’m very much not in favor of this. From my perspective, this style of logging causes more trouble than added value. log(“running foo()”)
foo()
log(“running bar()”)
bar()
log(“running baz()”)
baz()For one, it nearly doubles the lines of code in each script, which makes it harder to read, harder to quickly understand, and therefore harder to maintain. It also increases the likelihood of creating tech debt, by either forgetting to update log messages when code changes are made, or vice versa. I also find it more difficult to figure out a problem when I’m looking at a massive flood of log messages versus targeted warnings and errors. A few years ago I actually put in a significant effort into reducing the unnecessary output for precisely this purpose, and in my opinion it has been very successful and very helpful. This is in addition to the existing two modes of logging that we already have in place, one logging stdout to a file and one doing extra, special logging to a specific log file. Those have been instrumental in solving problems in the past, but they are specifically targeted so that we only get messages when it matters, not a flood. I’ll make you a deal: if there is one single problem in this upcoming COP that I can’t easily solve with the existing setup we have to deal with problems (excluding stuff Constructiva does with the portfolio files before our code even touches it), I’ll allow you to log every single line of code in this repo. |
|
Moving this back to draft. I hear what you're saying about not wanting to sift through log messages, but the fact remains that logs are our primary (and often only) mechanism for diagnosing problems remotely. Please note that every current and planned production environment is running non-interactively meaning that logs are the only direct information we have about their state. Importantly, they are emitted as the code in situ, and not in an artificial recreation (in silico). Regardless of how much we attempt to work in a reproducible environment, having the diagnostics from the actual event is always preferable to not having them. That said, sifting through log messages shouldn't be a challenge, as most terminals (or Having log messages about the progress of a process is comparable to having line numbers in a code editor. It allows you to quickly diagnose what parts of the process ran successfully and what did not, so that debugging efforts can start from an informed position. Along that line, the messages to stdout that are emitted during the normal processing of these scripts is practically non-existent (just a line letting us know which script is running, and build version if there is one). This means that during local development, there are extended periods with no indicators of progress. The changes in this PR supplement the current messages to stdout. I don't see any calls to other logging mechanisms other than CLI in these scripts, so I'm assuming that those are handled in the Overall, this PR is a stopgap/workaround to the problem that you identified. A fair number of these logs probably shouldn't be here. They should be in the functions that are being called. They would be much easier to maintain there, and would almost certainly be able to provide much more useful diagnostic information. But given the scope of adding good logging to all of our packages, this is what will have to do in the meantime. |
Docker build status
History
History JSON`[{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"GENERAL","holdings_date":"2022Q4","language":"EN","peer_group":"other","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2022q4_general-20240313T143456Z/EN/other/1/working_dir/50_Outputs/rmi_pacta_2022q4_general/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2022q4_general:20240313T143456Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"GENERAL","holdings_date":"2023Q4","language":"EN","peer_group":"other","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_general-20240313T143457Z/EN/other/1/working_dir/50_Outputs/rmi_pacta_2023q4_general/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_general:20240313T143457Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"DE","peer_group":"asset_manager","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/DE/asset_manager/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"DE","peer_group":"civil_society","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/DE/civil_society/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"DE","peer_group":"other","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/DE/other/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"DE","peer_group":"pension_fund","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/DE/pension_fund/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"DE","peer_group":"politician","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/DE/politician/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"DE","peer_group":"researcher","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/DE/researcher/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"EN","peer_group":"asset_manager","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/EN/asset_manager/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"EN","peer_group":"civil_society","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/EN/civil_society/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"EN","peer_group":"other","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/EN/other/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"EN","peer_group":"pension_fund","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/EN/pension_fund/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"EN","peer_group":"politician","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/EN/politician/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"EN","peer_group":"researcher","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/EN/researcher/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"FR","peer_group":"asset_manager","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/FR/asset_manager/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"FR","peer_group":"civil_society","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/FR/civil_society/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"FR","peer_group":"other","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/FR/other/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"FR","peer_group":"pension_fund","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/FR/pension_fund/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"FR","peer_group":"politician","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/FR/politician/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","project_code":"PA2024CH","holdings_date":"2023Q4","language":"FR","peer_group":"researcher","report":"[Report](https://pactadatadev.blob.core.windows.net/ghactions-workflow-transition-monitor-results-reports/rmi_pacta_2023q4_pa2024ch-20240313T143452Z/FR/researcher/1/working_dir/50_Outputs/rmi_pacta_2023q4_pa2024ch/report/index.html)","image":"`transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20240313T143452Z`"},{"commit_time":"2024-03-13T14:34:17Z","git_sha":"28838fbc27dde948afcff6dece3d5a8d2fb1b59b","image":"ghcr.io/rmi-pacta/workflow.transition.monitor:pr252"}]` |
|
Closing this one out since it's not likely to ever be merged. We can revive the branch if needed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add log messages to
web_tool_scripts which will help in diagnosing issues when running Docker image on remote machines (CTM Platform allows for log download)I was pretty generous logging at INFO (rather than DEBUG or TRACE) level, so the output is a lot busier than previously, but it give a very clear picture of where in the process it is at any given moment (until it falls into the
create_interactive_reportlong-running black hole).Tested with 2022Q2 Data.
Closes #70
Relates to RMI-PACTA/workflow.pacta#47