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.

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

ci: polyfill logger in edge cases for #541 #647

Merged
merged 1 commit into from
Jan 14, 2020

Conversation

jahkeup
Copy link
Member

@jahkeup jahkeup commented Jan 13, 2020

Issue #, if available:

#541

Description of changes:

This polyfill implementation of logger handles scenarios where the
environment isn't fully initialized or where the logger stub isn't able
to be run. Falling back to printf eliminates logged errors from these
places and in their place prints the expected messages.

This helps reach full coverage for #541

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This polyfill implementation of `logger` handles scenarios where the
environment isn't fully initialized or where the logger stub isn't able
to be run. Falling back to `printf` eliminates logged errors from these
places and in their place prints the expected messages.
@jahkeup jahkeup self-assigned this Jan 13, 2020
Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔩

@zmrow
Copy link
Contributor

zmrow commented Jan 14, 2020

I approved because I'm sure it will work and be fine. However, is there a technical need for us to use logger? I'm not sure I like the idea of needing to write tooling just to be able to have some sort of structured logging just to echo messages in our build process. But perhaps I'm missing a piece of the puzzle that requires this.

@jahkeup
Copy link
Member Author

jahkeup commented Jan 14, 2020

@zmrow this is for using the standard logger utility that normally logs using syslog facilities via the kernel's /dev/log device node that's not present in containerized environments that don't have the kernel listening on the socket.

This makes the logging output consistent across our scripts and was recommended for use in an earlier PR (can't point to where at the moment). This PR makes calls to logger, as used by our scripts, fall back to printing consistently but less so than when a socket is present (despite the flags directing the tool to not write to it, calls will otherwise print an error and not log anything at all).

fi

# Stream of messages sent to function as input
while read msg; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is msg in this context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$msg is a message that's read off of stdin piped into the "command" (the function at this point in the code). This is implemented here as you're able to pipe messages into (the real) logger and have it print out your messages with the same format for each read line:

logger -s -t INFO <<EOF
message 1 - hello, world!
message 2 - hi, there!
EOF
# <13>Jan 14 15:11:18 INFO: message 1 - hello, world!
# <13>Jan 14 15:11:18 INFO: message 2 - hi, there!

@jahkeup jahkeup merged commit 9245014 into ci-containers Jan 14, 2020
@jahkeup jahkeup deleted the ci-container-logger branch January 14, 2020 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants