Improve logging #1171
Merged
Improve logging #1171
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
ZauberNerd
force-pushed
the
improve-logging
branch
from
e6245fd
to
07ef62a
Compare
MegaLinter status: ✅ SUCCESS
See errors details in artifact MegaLinter reports on CI Job page |
ZauberNerd
force-pushed
the
improve-logging
branch
from
07ef62a
to
f146f94
Compare
Codecov Report
@@ Coverage Diff @@
## master #1171 +/- ##
==========================================
+ Coverage 57.50% 62.40% +4.89%
==========================================
Files 32 40 +8
Lines 4594 5402 +808
==========================================
+ Hits 2642 3371 +729
- Misses 1729 1769 +40
- Partials 223 262 +39
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
KnisterPeter
previously approved these changes
May 18, 2022
ZauberNerd
force-pushed
the
improve-logging
branch
from
f146f94
to
3f72b7c
Compare
ZauberNerd
force-pushed
the
improve-logging
branch
from
3f72b7c
to
6418bde
Compare
ZauberNerd
force-pushed
the
improve-logging
branch
3 times, most recently
from
8292684
to
53cceb7
Compare
KnisterPeter
force-pushed
the
improve-logging
branch
from
53cceb7
to
fface0a
Compare
ZauberNerd
force-pushed
the
improve-logging
branch
from
0c75474
to
9ddded4
Compare
KnisterPeter
force-pushed
the
implement-pre-and-post-steps
branch
from
12bc9a1
to
d243b19
Compare
KnisterPeter
force-pushed
the
improve-logging
branch
from
9ddded4
to
70c1a8a
Compare
KnisterPeter
previously approved these changes
May 25, 2022
|
@ZauberNerd this pull request is now in conflict 😩 |
|
@ZauberNerd this pull request has failed checks 🛠 |
ZauberNerd
force-pushed
the
improve-logging
branch
from
4f49405
to
d0e8969
Compare
|
@ZauberNerd this pull request is now in conflict 😩 |
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
The container gets injected a job logger, but during the time that steps are run, we want to use the step logger. This commit wraps pre/main/post steps in an executor that replaces the job logger with a step logger. Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
KnisterPeter
force-pushed
the
improve-logging
branch
from
d0e8969
to
ce8f113
Compare
| } else { | ||
| err := info.stopContainer()(ctx) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| info.result("success") | ||
| logger.WithField("jobResult", "success").Infof("\U0001F3C1 Job succeeded") |
There was a problem hiding this comment.
I get a misleading logging message, this job doesn't succeed. I cancelled it.
🚀 Start image=ghcr.io/catthehacker/ubuntu:act-latest
🐳 docker pull image=ghcr.io/catthehacker/ubuntu:act-latest platform= username= forcePull=false
🏁 Job succeeded
Warning: unable to get git repo: unable to find git repo
Error: context canceled
Probably a bug, which hasn't surfaced earlier.
I'm testing this change against my selfhosted runner code
I cancelled while it was downloading the docker image for the job
ChristopherHX
approved these changes
Jun 16, 2022
There was a problem hiding this comment.
Seems to work fine.
I wonder why this single warning bypassed my logger, both global and context logger have an custom formatter. The later warnings of the same kind are working as expected and call my formatter.
time="2022-06-16T21:19:42+02:00" level=warning msg="unable to get git repo: unable to find git repo"
cplee
approved these changes
Jun 17, 2022
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.
Replace more of the global logger call sites with contextualized job/step loggers and add job/step id and results as fields to the JSON logger output.
This causes most debug logs to be printed with the job logger, which means they are now correctly prefixed with the job they belong to.
Also, having the ids and results in the JSON output helps in cases where the log is read by machines, because we now can attribute each line to a job/step.