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

[chart] Fix default log level #5796

Merged
merged 3 commits into from
Sep 22, 2021
Merged

[chart] Fix default log level #5796

merged 3 commits into from
Sep 22, 2021

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Sep 21, 2021

Description

The invocation to the template was missing the context (.)

Also introduces two different flags for cli binaries, one for verbose logs and a different one to set json-log output format.

Related Issue(s)

Fixes #

How to test

Release Notes

[chart] Fix default log level template
[chart] Do not enable verbose log level by default in containers

@aledbf
Copy link
Member Author

aledbf commented Sep 21, 2021

/approve no-issue

@aledbf
Copy link
Member Author

aledbf commented Sep 21, 2021

/assign @csweichel

@aledbf
Copy link
Member Author

aledbf commented Sep 21, 2021

/werft run

👍 started the job as gitpod-build-aledbf-chartlog.3

@roboquat roboquat added size/S and removed size/XS labels Sep 21, 2021
@aledbf
Copy link
Member Author

aledbf commented Sep 21, 2021

/hold

@codecov
Copy link

codecov bot commented Sep 21, 2021

Codecov Report

Merging #5796 (4d79b91) into main (88e4f7b) will decrease coverage by 9.85%.
The diff coverage is n/a.

❗ Current head 4d79b91 differs from pull request most recent head d125319. Consider uploading reports for the commit d125319 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5796      +/-   ##
==========================================
- Coverage   31.97%   22.11%   -9.86%     
==========================================
  Files          72       11      -61     
  Lines       14979     1967   -13012     
==========================================
- Hits         4789      435    -4354     
+ Misses       9765     1474    -8291     
+ Partials      425       58     -367     
Flag Coverage Δ
components-content-service-app ?
components-content-service-lib ?
components-image-builder-app ?
components-image-builder-mk3-app ?
components-local-app-app-darwin ?
components-local-app-app-linux ?
components-local-app-app-windows ?
components-supervisor-app ?
components-ws-daemon-app 22.11% <ø> (ø)
components-ws-manager-app ?
components-ws-proxy-app ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/content-service/pkg/initializer/git.go
components/supervisor/pkg/supervisor/user.go
components/ws-manager/pkg/manager/status.go
components/supervisor/pkg/ports/exposed-ports.go
...ents/image-builder-mk3/pkg/orchestrator/monitor.go
...mponents/supervisor/pkg/supervisor/notification.go
components/ws-proxy/pkg/proxy/cookies.go
components/supervisor/pkg/terminal/service.go
components/supervisor/pkg/ports/tunnel.go
components/content-service/pkg/storage/noop.go
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88e4f7b...d125319. Read the comment docs.

@aledbf
Copy link
Member Author

aledbf commented Sep 21, 2021

/werft run

👍 started the job as gitpod-build-aledbf-chartlog.10

@aledbf
Copy link
Member Author

aledbf commented Sep 21, 2021

/werft run

👍 started the job as gitpod-build-aledbf-chartlog.12

@aledbf
Copy link
Member Author

aledbf commented Sep 21, 2021

/werft run

👍 started the job as gitpod-build-aledbf-chartlog.13

@aledbf
Copy link
Member Author

aledbf commented Sep 21, 2021

/hold cancel

@csweichel
Copy link
Contributor

csweichel commented Sep 22, 2021

/werft run

👍 started the job as gitpod-build-aledbf-chartlog.16

@csweichel
Copy link
Contributor

This change breaks the JSON logging for ws-daemon, which would lead to a misinterpretation of all log messages as ERROR.

@aledbf
Copy link
Member Author

aledbf commented Sep 22, 2021

This change breaks the JSON logging for ws-daemon, which would lead to a misinterpretation of all log messages as ERROR.

Please review/try again

@aledbf
Copy link
Member Author

aledbf commented Sep 22, 2021

With #5789 merged, this is the output of ws-daemon without no errors

{"level":"info","location":"/mnt/workingarea","message":"restored workspaces from disk","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:51:57Z","workspacesLoaded":3,"workspacesOnDisk":3}
{"addr":":10115","level":"info","message":"started gRPC server","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:51:57Z"}
{"addr":"127.0.0.1:9500","level":"info","message":"started Prometheus metrics server","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:51:57Z"}
{"addr":":6060","level":"info","message":"serving pprof service","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:51:57Z"}
{"level":"info","message":"containerd subscription established","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:51:57Z"}
{"addr":":9999","level":"info","message":"started readiness signal","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:51:57Z"}
{"level":"info","message":"🧫 ws-daemon is up and running. Stop with SIGINT or CTRL+C","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:51:57Z"}
{"level":"info","message":"start hosts source","name":"registryFacade","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:51:57Z"}
{"level":"info","message":"Received SIGINT - shutting down","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:56:14Z"}
{"level":"info","message":"hosts source shutting down","name":"registryFacade","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:56:14Z"}
{"level":"info","message":"hosts updater shutting down","serviceContext":{"service":"ws-daemon","version":""},"severity":"INFO","time":"2021-09-22T10:56:14Z"}

@csweichel
Copy link
Contributor

/lgtm

@roboquat roboquat added the lgtm label Sep 22, 2021
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: b20cae8c2eb10ced14cef3b7c2b11aba9ff2792a

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, csweichel

Associated issue requirement bypassed by: aledbf

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit f25de2c into main Sep 22, 2021
@roboquat roboquat deleted the aledbf/chartlog branch September 22, 2021 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants