-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add logs to CPVM connection process #8924
base: main
Are you sure you want to change the base?
Add logs to CPVM connection process #8924
Conversation
...ices/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java
Outdated
Show resolved
Hide resolved
...ices/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java
Outdated
Show resolved
Hide resolved
...ices/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java
Outdated
Show resolved
Hide resolved
...ices/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
Co-authored-by: dahn <daan.hoogland@gmail.com>
0264b28
to
4743666
Compare
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@DaanHoogland, thanks for your review! As logging standards in CloudStack are still under discussion (#8746), I'd rather keep logging the exceptions stacktraces at the Honestly, I do not see the point in turning off the stacktraces at the On the other hand, if the operator has no knowledge of development, he might not understand the stacktraces, and that is fine, causing no harm. Besides that, following along with what you've proposed in #8746, what happens when the operator chooses to turn the stacktraces down by setting the logs level to Honestly, I'm looking forward to see the outcome of the #8746 discussion and I commit myself to participate on it. However, as I've mentioned before, I'd rather keep the proposed changes related to the stacktraces at the |
...ices/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java
Outdated
Show resolved
Hide resolved
...ices/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java
Outdated
Show resolved
Hide resolved
...ices/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java
Outdated
Show resolved
Hide resolved
...ices/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8924 +/- ##
=============================================
+ Coverage 13.17% 15.28% +2.11%
- Complexity 9214 11521 +2307
=============================================
Files 2725 5425 +2700
Lines 258235 474021 +215786
Branches 40249 58630 +18381
=============================================
+ Hits 34013 72439 +38426
- Misses 219913 393539 +173626
- Partials 4309 8043 +3734
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm, can we now remove com.cloud.consoleproxy.util.Logger
from the code base @bernardodemarco ? Just to deal with the technical debt. (we can always add a new proxy once we get wise ;)
Description
The logs related to the CPVM connection are not enough to perform a great troubleshooting process.
This PR, therefore, adds logs and improves the existing ones in order to facilitate the CPVM connection troubleshooting.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?