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

JSON output format for the entrypoint script #1066

Merged
merged 1 commit into from
Jul 1, 2020

Conversation

jayanthvn
Copy link
Contributor

@jayanthvn jayanthvn commented Jun 29, 2020

*Issue #907 , if available: JSON o/p format for the logs generated from entrypoint script

Description of changes:
dev-dsk-varavaj-2b-72f02457 % kubectl logs aws-node-2sq6w -n kube-system
{"level":"info","ts":"2020-07-01T02:21:56.104Z","caller":"entrypoint.sh","msg":"Copying CNI plugin binaries ... "}
{"level":"info","ts":"2020-07-01T02:21:56.120Z","caller":"entrypoint.sh","msg":"Starting IPAM daemon in the background ... "}
{"level":"info","ts":"2020-07-01T02:21:56.127Z","caller":"entrypoint.sh","msg":"Checking for IPAM connectivity ... "}
{"level":"info","ts":"2020-07-01T02:21:58.148Z","caller":"entrypoint.sh","msg":"Copying CNI plugin binary and config file ... "}
{"level":"info","ts":"2020-07-01T02:21:58.167Z","caller":"entrypoint.sh","msg":"Successfully copied CNI plugin binary and config file."}
{"level":"info","ts":"2020-07-01T02:21:58.168Z","caller":"entrypoint.sh","msg":"Foregrounding IPAM daemon ..."}

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jayanthvn jayanthvn requested a review from mogren June 29, 2020 22:17
@mogren
Copy link
Contributor

mogren commented Jun 30, 2020

I think I need to update #907 a bit. Having these log messages definitely helps us debug issues, so it would be better to have a function that logs messages in JSON format instead, so that it matches what zap outputs. Something like:

{"level":"info","ts":"2020-06-30T03:45:53.337Z","caller":"entrypoint.sh","msg":"Copying CNI plugin binary and config file ... "}
{"level":"info","ts":"2020-06-30T03:45:53.422Z","caller":"entrypoint.sh","msg":"Foregrounding IPAM daemon ... "}

@jayanthvn
Copy link
Contributor Author

I think I need to update #907 a bit. Having these log messages definitely helps us debug issues, so it would be better to have a function that logs messages in JSON format instead, so that it matches what zap outputs. Something like:

{"level":"info","ts":"2020-06-30T03:45:53.337Z","caller":"entrypoint.sh","msg":"Copying CNI plugin binary and config file ... "}
{"level":"info","ts":"2020-06-30T03:45:53.422Z","caller":"entrypoint.sh","msg":"Foregrounding IPAM daemon ... "}

Thanks @mogren . It makes sense and I can look into it.

@jayanthvn jayanthvn force-pushed the entry_point_changes branch from a5446a4 to 58d1788 Compare June 30, 2020 23:06
@jayanthvn jayanthvn self-assigned this Jun 30, 2020
@jayanthvn jayanthvn changed the title removed echoes from entry point script JSON o/p format for the logs generated from entrypoint script Jun 30, 2020
Copy link
Contributor

@nithu0115 nithu0115 left a comment

Choose a reason for hiding this comment

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

Thanks @jayanthvn for the fixing this. Need some more changes. Please see comments inline..

scripts/entrypoint.sh Show resolved Hide resolved
@jayanthvn jayanthvn force-pushed the entry_point_changes branch 2 times, most recently from d86a487 to 1ce136e Compare July 1, 2020 02:13
@jayanthvn jayanthvn force-pushed the entry_point_changes branch from d3d77f6 to 8756bba Compare July 1, 2020 02:19
Copy link
Contributor

@nithu0115 nithu0115 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

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

Thanks @jayanthvn and @nithu0115

@@ -26,13 +26,22 @@ set -eu
# turn on bash's job control
set -m

log_in_json()
{
FILENAME="${0##*/}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hadn't seen "${0##*/}" before 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From stack overflow - "for the variable $0, and the pattern '/', the two hashes mean from the beginning of the parameter, delete the longest (or greedy) match—up to and including the pattern. So, where $0 is the name of a file, eg., $HOME/documents/doc.txt, then the parameter would be expanded as: doc.txt" :D

@mogren mogren merged commit d938e5e into aws:master Jul 1, 2020
@jayanthvn jayanthvn deleted the entry_point_changes branch July 1, 2020 17:40
@jayanthvn jayanthvn modified the milestone: v1.7.0 Jul 1, 2020
@mogren mogren added this to the v1.7.0 milestone Jul 1, 2020
@mogren mogren changed the title JSON o/p format for the logs generated from entrypoint script JSON output format for the entrypoint script Jul 8, 2020
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