Switch all logging to use containerd log pkg#45799
Conversation
ab113b7 to
6f5eaec
Compare
|
This is mostly a grep->sed to replace things but needed some help to clean things up after # function signature messed up
syntaxErr1='unexpected ( in parameter list; possibly missing comma or )'
syntaxErr2='syntax error: unexpected ( after top level declaration'
fix() {
infile="${1}"
# fix weird issue where goimports gives us the wrong import...
# Note: this may not be propperly grouped after this
grep "command-line-arguments" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,command-line-arguments/endor/,," $file; done'
# Replace ctx with context.TODO() because "ctx" doesn't exist
grep "undefined: ctx" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,ctx,context.TODO()," $file; done'
# Fix our original sed broke things
grep "log\.G(ctx)\.ErrorKey" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(ctx)\.ErrorKey,logrus.ErrorKey," $file; done'
grep "log\.G(ctx)\.Level" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(ctx)\.Level,logrus.Level," $file; done'
grep "log\.G(context\.TODO())\.ErrorKey" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(context\.TODO())\.ErrorKey,logrus.ErrorKey," $file; done'
grep 'log\.G(context.TODO())\..*Level' "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(context\.TODO())\.\(.*Level\),logrus.\1," $file; done'
grep 'log\.G(ctx)\..*Level' "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(ctx)\.\(.*Level\),logrus.\1," $file; done'
grep 'log.G(context.TODO()).SetOutput' "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(ctx)\.\(.*SetOutput\),logrus.\1," $file; done'
grep 'log.G(context.TODO()).AddHook' "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(ctx)\.\(.*AddHook\),logrus.\1," $file; done'
grep 'etwlog\.G(context.TODO())' "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,etwlog\.G(context\.TODO()),etwlog," $file; done'
grep 'log.G undefined (type *eventlog.Log has no field or method G)' "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(ctx),log," $file; done'
grep "${syntaxErr1}" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(ctx),logrus," $file; done'
grep "${syntaxErr2}" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file num extra <<<$line; sed -i "${num}s,log\.G(ctx),logrus," $file; done'
# fix goimports
rm go.mod # goimports doesn't like the empty go.mod hack
grep "undefined: context" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file extra <<<$line; goimports -w ./${file}; done'
grep "undefined: logrus" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file extra <<<$line; goimports -w ./${file}; done'
grep "imported and not used" "${infile}" | bash -xc 'while IFS= read -r line; do IFS=: read -r file extra <<<$line; goimports -w ./${file}; done'
}
tmp="$(mktemp)"
cleanup() {
cat "${tmp}"
rm -f "${tmp}"
}
trap 'cleanup' EXIT
for goos in linux windows; do
while ! (
printf '%s\n\n%s' 'module github.com/docker/docker' 'go 1.19' >"go.mod"
GOOS="${goos}" GO111MODULE=on go build -mod=vendor -modfile=vendor.mod -gcflags='-e' ./cmd/dockerd 2>"${tmp}"
); do
fix "${tmp}"
done
doneFound goimports was giving me the wrong import for logrus... which was really strange. See the script above for that correction as well (hint: |
|
Also really fun... |
83c691c to
5b96f4b
Compare
This unifies our logging and allows us to propagate logging and trace contexts together. Signed-off-by: Brian Goff <[email protected]>
5b96f4b to
74da6a6
Compare
|
All green. |
|
So really #45799 (comment) was not enough and go tooling is so bad for this. |
|
In general, I'm a big "plus 1" to doing this (and removing the logrus from everywhere). I'm wondering if we should consider making the A couple of things that I'm wondering (also with in the back of my mind that
|
I'm not too worried about that here especially since it will be trivial to change the imports if needed after this is in.
I can't say exactly why it was removed with certainty, but I do know there are those in the Go community (with a lot of sway) who do not like this pattern (they are wrong, but 🤷 🤣 😇 ) For slog in general, that is a much larger change and isn't really buying us anything except maybe some extra CPU time due to allocations (nor does it get us log+tracing propagation). I think this will be more "oh well go has a half-way decent logging story now in the stdlib we should use that at some point". |
|
Related (from long ago; probably should be closed, but still attacking a related issue): #36155 |
|
For reference, we discussed changing Here's the incantation to make that happen, though: |
|
We discussed using the log.Fields alias, but that's not yet in the 1.6 branch, so we should look at getting that backported. |
This unifies our logging and allows us to propagate logging and trace contexts together.