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

[server] Introduce /debug/logging #5697

Merged
merged 3 commits into from
Sep 20, 2021
Merged

[server] Introduce /debug/logging #5697

merged 3 commits into from
Sep 20, 2021

Conversation

geropl
Copy link
Member

@geropl geropl commented Sep 14, 2021

Description

Introduces an endpoint for changing log levels at runtime, similar to the one introduced here.

Currently this is server-only. DebugApp can and should easily be moved to a common-ts package, but did not want to make this part of this PR.

Related Issue(s)

Context: #5551

How to test

  1. kubectl port-forward deployment server 6060
  2. curl localhost:6060/debug/logging -d '{"level":"info"}'

Release Notes

runtime configurable log-level for `server`

@svenefftinge
Copy link
Member

Can't we just change the LOG_LEVEL on the deployment? Having an endpoint that mutates the state of one server instance (which one?) seems unnecessary, because restarts shouldn't be a problem with servers.

@geropl
Copy link
Member Author

geropl commented Sep 15, 2021

Can't we just change the LOG_LEVEL on the deployment? Having an endpoint that mutates the state of one server instance (which one?) seems unnecessary, because restarts shouldn't be a problem with servers.

Sadly restarts are a problem with servers: reconncets from dashboard and workspaces are not seemless, but lead to temporary unavailability.

Also, I don't feel that this endpoint adds a lot of complexity. And if shifts the whole debate around logging from "reduce amount of logging" to "improve quality of logging" (bc of the argument "zero cost loglevel switch"), which I really really like.

Furthermore, we should re-use DebugApp for ws-manage-bridge and payment-endpoint where we rather not restart as well.

@geropl geropl force-pushed the gpl/5551-debug-endpoint branch from 7015936 to c9d3a4e Compare September 15, 2021 09:51
@aledbf
Copy link
Member

aledbf commented Sep 15, 2021

Having an endpoint that mutates the state of one server instance

That is not possible in Kubernetes. Any update to Deployments or Daemonsets updates all the pods and also implies reconnections due to the replacement of pods.

@aledbf
Copy link
Member

aledbf commented Sep 15, 2021

/approve

@aledbf
Copy link
Member

aledbf commented Sep 15, 2021

@geropl thank you for working on this ❤️

@aledbf
Copy link
Member

aledbf commented Sep 17, 2021

/approve

@geropl geropl force-pushed the gpl/5551-debug-endpoint branch from c9d3a4e to 6ab6375 Compare September 20, 2021 08:11
@geropl geropl force-pushed the gpl/5551-debug-endpoint branch from 6ab6375 to aad8538 Compare September 20, 2021 13:35
@geropl
Copy link
Member Author

geropl commented Sep 20, 2021

The last force-pushes were conflict resolutions.

@aledbf
Copy link
Member

aledbf commented Sep 20, 2021

/lgtm

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 5bb286baa012e9ad6135b02c168f73bf7a6a0b2f

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: aledbf

Associated issue: #5551

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 7be34c5 into main Sep 20, 2021
@roboquat roboquat deleted the gpl/5551-debug-endpoint branch September 20, 2021 14:48
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.

5 participants