chore: Improve and simplify logging #4194
Conversation
m-horky
force-pushed
the
better-logging
branch
from
cf4b55c
to
a83be7e
Compare
xiangce
added
the
client
m-horky
force-pushed
the
better-logging
branch
4 times, most recently
from
f5e1eee
to
e44e4be
Compare
m-horky
force-pushed
the
better-logging
branch
from
e44e4be
to
a68dafa
Compare
|
Hi @m-horky - Sorry for bringing this trouble to you, as we just resolved an urgent bug within the repo. Please rebase the master branch of your fork of insights-core and then rebase this branch again. Please reach out to me for any problems when doing the rebase. Apologize again. |
m-horky
force-pushed
the
better-logging
branch
from
a68dafa
to
1b11218
Compare
| phase=os.getenv("INSIGHTS_PHASE", "unknown"), | ||
| version=package_info.get("VERSION", "unknown"), | ||
| path=client.__file__, | ||
| arguments=" ".join(sys.argv[1:]), |
There was a problem hiding this comment.
hm are you sure logging sys.argv is a good idea? it might contain credentials, eg --username and --password currently
There was a problem hiding this comment.
Those are not settable via CLI, only through the configuration file. Should we try to filter them anyways, in case someone enters it there by mistake?
* Card ID: CCT-705 This patch will help us with identifying what is happening at given times. - Include phase, version and arguments on each phase load. Some of the information may not be necessary, as newer builds of insights-client contain logs that carry the information about phase and version. However, older builds do not, and these bits are valuable when debugging. - Include payloads, headers and file attachments in the log message, if present. This should make it easier to debug PATCH calls. BASIC authentication credentials are not included in these headers, the library adds them later when constructing the request. - Disable logging of HTTP responses for checkin. It is verbose and do not need to be logged in a file every time. Signed-off-by: mhorky <[email protected]>
m-horky
force-pushed
the
better-logging
branch
from
1b11218
to
ed4aaff
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4194 +/- ##
==========================================
- Coverage 76.88% 76.86% -0.03%
==========================================
Files 757 757
Lines 41342 41354 +12
Branches 9526 9532 +6
==========================================
Hits 31785 31785
- Misses 8479 8492 +13
+ Partials 1078 1077 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Also please wait for our QE ACK before merging. Thanks. |
All Pull Requests:
Check all that apply:
Complete Description of Additions/Changes:
This patch will help us with identifying what is happening at given times.