logging standards in CloudStack

[DaanHoogland]

@JoaoJandre , maybe we can set some standards here ;)
re: #8728 (comment)

as a first set:

one functional requirement that I have is: As an non-developping operator I want stacktraces to be turned of by default.

[JoaoJandre]

Here are some guidelines I think would be a good start (adapted from @DaanHoogland 's post on the mailing list)

Fatal:

• Include a reason why the system can not function anymore;
• Be followed by a System.exit() call;
• Include a stack trace.

Error:

• Include a reason why the system can not execute a user request or
  why part of the system is degraded;
• If the error is happening because a user request cannot be executed, do not include a stack trace; however, if the system is degraded in some form, that is, some feature will stop working or some component of the system is compromised, and the stack trace might bring helpful information, then include the stack trace.

Warn:

• Include a reason why the system might not behave as the user is
  expecting;
• Should not include a stack trace.

Info:

• Informational messages to tell the operator what the system is doing at the moment;
• Should not include a stack trace.

Debug:

• Useful troubleshooting information. This information should not be needed for normal operation;
• If a thrown exception is caught, not rethrown, and the Fatal/Error levels do not apply, the stack trace of such exception.

Trace:

• Granular information that is useful during development of a feature/enhancement/fix/improvement that is not needed in any kind of operational way.

[DaanHoogland]

I like your summary @JoaoJandre except for the error bit:

Error:

* Include a reason why the system can not execute a user request or
  why part of the system is degraded;

* If the error is happening because a user request cannot be executed, do not include a stack trace; however, if the system is degraded in some form, that is, some feature will stop working or some component of the system is compromised, and the stack trace might bring helpful information, then include the stack trace.

Though it looks good in general there is not a clear distiction if something is caused by a user and how directly: i.e.

• Was it because of an API call; clear.
• Was it because of a background cleanup job; clear in the other direction (unless it is an expunge of a user resource).
• Was it because of a scheduled task; grey area

My take on this dilemma is to never add a stack trace to error messages and let them be followed by a debug message with thew stacktrace, so operators can turn them on and off at will.

[GutoVeronezi]

I believe that adding an error message in ERROR followed by the stack trace in DEBUG is not so interesting. For instance, if we have the INFO level enabled and an exception occurs, we would not see it and would have to enable DEBUG and reproduce the situation to see the stack trace. Some situations are not so easy to reproduce; thus, we would lose track of the situation. Another point is that we would have to change the whole code base to comply with this standard. Furthermore, while in DEBUG, ACS prints a lot of logs; if one is concerned with the stack traces, however, still wants to keep the log level in INFO, it would not be possible.

I understand @DaanHoogland's point about non-developer operators not wanting to see stack traces around the logs, and we can achieve that with less effort. Via the log pattern, Log4j allows us to define how many lines of the stack trace will be printed in the logs. Therefore, to address @DaanHoogland's point, this pattern could be defaulted to 0 stack trace lines. If desired, one would still be able to see the stack traces by changing the pattern, independently of the log level. This approach can address both use cases presented in a flexible and effortless fashion.

[DaanHoogland]

I didn't know about the stacktrace limit option, that is good.

Another point is that we would have to change the whole code base to comply with this standard.

This is true what ever we decide, but your implied point, to minimise the effort is valid though.

[weizhouapache]

@lucas-a-martins and me had some discussion on #8603

I think it is better to raise the discussion here.

Basically the question is, when an error happens, what error message should be displayed to the root admin and regular users or domain admin ?

• as root admin, I want to get information as much/clear/direct as possible. some errors like "internal communication error" are not acceptable.
• as user, I want to get clear information. moreover, some sensitive information should NOT be displayed to the end users, for example, the id of resource (resource uuid or name should be used), the internal name of VM instance, the exception on kvm hosts, etc.

Currently most error messages are for root admin, but some of them confuse regular users. Sometimes it is hard to find an error message which is good for both root admin and regular users.

Back to #8603 , the current error message "Global setting endpoint.url has to be set to the Management Server's API end point" is not good for regular users, but the new error message "Unable to complete this operation. Contact your cloud admin." does not give any information to root admin which is also not good I think. sorry @lucas-a-martins

Do we need a logging mechanism to display different error messages to root admin and regular users ?

for example

• for InvalidParameterValueException, display the same error message to root admin and regular user
• for CloudRuntimeException, display the error message of the exception to root admin, but an error "Unable to complete this operation. Contact your cloud admin." to regular user

cc @DaanHoogland @rohityadavcloud @GutoVeronezi @JoaoJandre

[DaanHoogland]

@weizhouapache a possible solution to this could be to add an error uuid to the exception and use this in the loging. Users don't see the logging but only exceptions thrown. idea?

[lucas-a-martins]

Hey, @weizhouapache. Sorry for the delay;

About your sugestion, I think you have a good idea here. Also, we already have something like this with com.cloud.api.ApiServer#handleRequest.

I didn't have the opportunity to test if this works as intended, but if it is actually working, I believe that we just need to standardize how which exception should be used, without the need to develop something from scratch. Furthermore, I think that is really important to look at each exception individually in order to decide whether it makes sense for an exception to be exclusive to Root Admin or not.