Add request ID fields to all DB and auth logs #2683
Conversation
arielshaqed
added
the
include-changelog
| } | ||
| return logging.Default() | ||
| return log.WithContext(ctx).WithFields(fields) |
There was a problem hiding this comment.
General concern - not knowing when to call WithContext - looks like it is something we need to load on our logger as soon as possible or defer it to the last thing we do.
There was a problem hiding this comment.
I believe this is the right place to do it. I changed the signature of getLogger to say you need to know the context in which you will be logging. So as soon as you need it, you must give context and additional fields. Now outputting a bad DB log without context and/or fields is actually more difficult than outputting a good one :-)
| } | ||
| return logging.Default() | ||
| return log.WithContext(ctx).WithFields(fields) |
There was a problem hiding this comment.
I believe this is the right place to do it. I changed the signature of getLogger to say you need to know the context in which you will be logging. So as soon as you need it, you must give context and additional fields. Now outputting a bad DB log without context and/or fields is actually more difficult than outputting a good one :-)
Partly of #2682. Most important gap is causing AWS clients (and, less
crucially, Azure and GCP) to log the request ID.