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

Misc log fixes #5330

Merged
merged 3 commits into from
Nov 1, 2023
Merged

Misc log fixes #5330

merged 3 commits into from
Nov 1, 2023

Conversation

eysi09
Copy link
Collaborator

@eysi09 eysi09 commented Nov 1, 2023

What this PR does / why we need it:

Fixes a handful of small logging issues I bumped into while using the web dashboard. See individual commits:

Commit 1:

fix(log): ensure action log inherits fixLevel

Before this fix, action log entries created from an input log with
'fixLevel' set, would not inherit that level but instead default to the
info level.

This was only really noticeable around some edge in the web dashboard
were logs that should've been verbose were rendered at the info level.

This fixes that and adds corresponding tests.

Commit 2:

improvement(server): log internal command requests at debug level

Before this fix, requests for internal commands like
_get-deploy-statuses weren't logged at all.

Now we log them at the debug level which is useful for, well, debugging
but also makes them visible in the web dashboard (if the appropriate
level is set) where they originate from.

Commit 3:

fix(cloud): correctly send error messages to server

Before this fix, we'd "render" error logs before sending them to
the server which meant that the "error" part was kept but the "msg" part
was dropped.

The user still sees the error but the message can be a bit chopped. This
fixes that by sending the "msg" field as is and adding an "error" field
to the payload as well that cloud handles separately.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Before this fix, action log entries created from an input log with
'fixLevel' set, would not inherit that level but instead default to the
info level.

This was only really noticeable around some edge in the web dashboard
were logs that should've been verbose were rendered at the info level.

This fixes that and adds corresponding tests.
Before this fix, requests for internal commands like
_get-deploy-statuses weren't logged at all.

Now we log them at the debug level which is useful for, well, debugging
but also makes them visible in the web dashboard (if the appropriate
level is set) where they originate from.
Before this fix, we'd "render" error logs before sending them to
the server which meant that the "error" part was kept but the "msg" part
was dropped.

The user still sees the error but the message can be a bit chopped. This
fixes that by sending the "msg" field as is and adding an "error" field
to the payload as well that cloud handles separately.
@eysi09 eysi09 requested a review from thsig November 1, 2023 17:45
@eysi09 eysi09 added this pull request to the merge queue Nov 1, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 1, 2023
@eysi09 eysi09 added this pull request to the merge queue Nov 1, 2023
Merged via the queue into main with commit d9b0169 Nov 1, 2023
@eysi09 eysi09 deleted the misc-log-fixes branch November 1, 2023 22:30
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.

2 participants