You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Sheng Yang <sh...@yasker.org> on 2012/07/09 19:44:03 UTC
Add ExceptionResponse in the log?
After we add ExceptionResponse for user to better understand what's
wrong, there has been nothing left in the log for developer to know
what's wrong...
What we got now is only something like:
2012-07-09 19:22:10,098 DEBUG [cloud.async.AsyncJobManagerImpl]
(Job-Executor-32:job-29) Complete async job-29, jobStatus: 2,
resultCode: 530, result:
com.cloud.api.response.ExceptionResponse@3fa28ae
And nothing else.
Could we just output every ExceptionResponse in the log? That's would
be much more helpful.
--Sheng
RE: Add ExceptionResponse in the log?
Posted by Vijayendra Bhamidipati <vi...@citrix.com>.
Sure, thanks Alex, Alena! I'll submit the required changes.
Regards,
Vijay
-----Original Message-----
From: Alena Prokharchyk
Sent: Monday, July 09, 2012 5:14 PM
To: cloudstack-dev@incubator.apache.org; Frank Zhang; Vijayendra Bhamidipati
Subject: Re: Add ExceptionResponse in the log?
Based on follow up with Alex.
When we log the result of Async job execution, we don't have to print the Exception stack trace; only print error code and exception error message (errorCode + errorText). Print this in DEBUG mode.
Exception stack trace is being printed prior to Async job result log automatically once the exception is raised, and it always comes in WARN.
The exception can be located in the log by following "jobId-<id>" of the async job.
For the exceptions we know how to handle (InvalidParameterValueException, PermissionDeniedExceptinon, etc), we don't log the stack trace.
So Vijay, just modify toString() method of the Exception response to include errorCode+errorText.
-Alena.
On 7/9/12 4:00 PM, "Alex Huang" <Al...@citrix.com> wrote:
>We should NOT print stack traces in debug. I'm not sure exactly what
>this is but all exception traces should be logged with at least a WARNING.
>
>--Alex
>
>> -----Original Message-----
>> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
>> Sent: Monday, July 09, 2012 3:18 PM
>> To: cloudstack-dev@incubator.apache.org; Frank Zhang
>> Subject: Re: Add ExceptionResponse in the log?
>>
>> On 7/9/12 2:58 PM, "Vijayendra Bhamidipati"
>> <vi...@citrix.com> wrote:
>>
>> >Hi Frank/Alena/Sheng!
>> >
>> >Actually I hadn't modified that part of the code when making changes
>> >earlier, but we definitely should be seeing more than just
>> >classname@hash-of-object which is toString()'s default output when
>> >it's passed an Object, so I'll do what Alena suggested and override
>> >toString in the ExceptionResponse class. However, I have a question
>> >for Frank - is it necessary for us to print out the exception stack
>> >trace, since that would surely show up in the mgmt. server log file?
>> >Also, wanting to print the stack trace would mean having to change
>> >quite a few parts of the code to pass the exception object to the
>> >calls that build the responses to the exception. Any specific reason
>> >why this wasn't the approach to begin with, other than that the
>> >exception stack trace would show up in the mgmt. server log file?
>> >
>> >Finally, the function CompleteAsyncJob() which prints out the
>> >message Sheng referred to:
>> >
>> >s_logger.debug("Complete async job-" + jobId + ", jobStatus: " +
>> >jobStatus +
>> > ", resultCode: " + resultCode + ", result: " +
>> >resultObject);
>>
>>
>> Once you implement toString() for Exception response, this debug
>> statement will include the reason for the failure. Add errorText to
>> the method.
>>
>>
>> >
>> >is invoked at different parts of the code that builds resultObject
>> >differently - in extractVolume() in ManagementServerImpl.java and
>> >updateDatabase() in UploadListener.java, it's of type
>> >ExtractResponse instead of ExceptionResponse. So I think I'll have
>> >to override
>> >toString() in both ExceptionResponse and ExtractResponse classes -
>> >Alena, does that sound right?
>>
>>
>> Nope, you do it just in ExceptionResponse. Extract functionality
>>needs some refactoring as it didn't follow the API Async design from
>>the very beginning.
>>
>>
>> >
>> >
>> >Regards,
>> >Vijay
>> >
>> >-----Original Message-----
>> >From: Frank Zhang
>> >Sent: Monday, July 09, 2012 11:12 AM
>> >To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
>> >Subject: RE: Add ExceptionResponse in the log?
>> >
>> >Vijay, please make sure stack trace is logged as well. Text of
>> >Exception.getMessage() is definitely not enough
>> >
>> >> -----Original Message-----
>> >> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
>> >> Sent: Monday, July 09, 2012 10:53 AM
>> >> To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
>> >> Subject: Re: Add ExceptionResponse in the log?
>> >>
>> >> We should def. print out the reason.
>> >>
>> >> Vijay, just override toString() method in the Exception response
>> >> to log the reason for the failure in addition to the error code.
>> >>
>> >> -Alena.
>> >>
>> >>
>> >> On 7/9/12 10:44 AM, "Sheng Yang" <sh...@yasker.org> wrote:
>> >>
>> >> >After we add ExceptionResponse for user to better understand
>> >> >what's wrong, there has been nothing left in the log for
>> >> >developer to know what's wrong...
>> >> >
>> >> >What we got now is only something like:
>> >> >
>> >> >2012-07-09 19:22:10,098 DEBUG [cloud.async.AsyncJobManagerImpl]
>> >> >(Job-Executor-32:job-29) Complete async job-29, jobStatus: 2,
>> >> >resultCode: 530, result:
>> >> >com.cloud.api.response.ExceptionResponse@3fa28ae
>> >> >
>> >> >And nothing else.
>> >> >
>> >> >Could we just output every ExceptionResponse in the log? That's
>> >> >would be much more helpful.
>> >> >
>> >> >--Sheng
>> >> >
>> >>
>> >
>> >
>>
>
>
Re: Add ExceptionResponse in the log?
Posted by Alena Prokharchyk <Al...@citrix.com>.
Based on follow up with Alex.
When we log the result of Async job execution, we don't have to print the
Exception stack trace; only print error code and exception error message
(errorCode + errorText). Print this in DEBUG mode.
Exception stack trace is being printed prior to Async job result log
automatically once the exception is raised, and it always comes in WARN.
The exception can be located in the log by following "jobId-<id>" of the
async job.
For the exceptions we know how to handle (InvalidParameterValueException,
PermissionDeniedExceptinon, etc), we don't log the stack trace.
So Vijay, just modify toString() method of the Exception response to
include errorCode+errorText.
-Alena.
On 7/9/12 4:00 PM, "Alex Huang" <Al...@citrix.com> wrote:
>We should NOT print stack traces in debug. I'm not sure exactly what
>this is but all exception traces should be logged with at least a WARNING.
>
>--Alex
>
>> -----Original Message-----
>> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
>> Sent: Monday, July 09, 2012 3:18 PM
>> To: cloudstack-dev@incubator.apache.org; Frank Zhang
>> Subject: Re: Add ExceptionResponse in the log?
>>
>> On 7/9/12 2:58 PM, "Vijayendra Bhamidipati"
>> <vi...@citrix.com> wrote:
>>
>> >Hi Frank/Alena/Sheng!
>> >
>> >Actually I hadn't modified that part of the code when making changes
>> >earlier, but we definitely should be seeing more than just
>> >classname@hash-of-object which is toString()'s default output when it's
>> >passed an Object, so I'll do what Alena suggested and override toString
>> >in the ExceptionResponse class. However, I have a question for Frank -
>> >is it necessary for us to print out the exception stack trace, since
>> >that would surely show up in the mgmt. server log file? Also, wanting
>> >to print the stack trace would mean having to change quite a few parts
>> >of the code to pass the exception object to the calls that build the
>> >responses to the exception. Any specific reason why this wasn't the
>> >approach to begin with, other than that the exception stack trace would
>> >show up in the mgmt. server log file?
>> >
>> >Finally, the function CompleteAsyncJob() which prints out the message
>> >Sheng referred to:
>> >
>> >s_logger.debug("Complete async job-" + jobId + ", jobStatus: " +
>> >jobStatus +
>> > ", resultCode: " + resultCode + ", result: " +
>> >resultObject);
>>
>>
>> Once you implement toString() for Exception response, this debug
>> statement will include the reason for the failure. Add errorText to the
>> method.
>>
>>
>> >
>> >is invoked at different parts of the code that builds resultObject
>> >differently - in extractVolume() in ManagementServerImpl.java and
>> >updateDatabase() in UploadListener.java, it's of type ExtractResponse
>> >instead of ExceptionResponse. So I think I'll have to override
>> >toString() in both ExceptionResponse and ExtractResponse classes -
>> >Alena, does that sound right?
>>
>>
>> Nope, you do it just in ExceptionResponse. Extract functionality needs
>>some
>> refactoring as it didn't follow the API Async design from the very
>>beginning.
>>
>>
>> >
>> >
>> >Regards,
>> >Vijay
>> >
>> >-----Original Message-----
>> >From: Frank Zhang
>> >Sent: Monday, July 09, 2012 11:12 AM
>> >To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
>> >Subject: RE: Add ExceptionResponse in the log?
>> >
>> >Vijay, please make sure stack trace is logged as well. Text of
>> >Exception.getMessage() is definitely not enough
>> >
>> >> -----Original Message-----
>> >> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
>> >> Sent: Monday, July 09, 2012 10:53 AM
>> >> To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
>> >> Subject: Re: Add ExceptionResponse in the log?
>> >>
>> >> We should def. print out the reason.
>> >>
>> >> Vijay, just override toString() method in the Exception response to
>> >> log the reason for the failure in addition to the error code.
>> >>
>> >> -Alena.
>> >>
>> >>
>> >> On 7/9/12 10:44 AM, "Sheng Yang" <sh...@yasker.org> wrote:
>> >>
>> >> >After we add ExceptionResponse for user to better understand what's
>> >> >wrong, there has been nothing left in the log for developer to know
>> >> >what's wrong...
>> >> >
>> >> >What we got now is only something like:
>> >> >
>> >> >2012-07-09 19:22:10,098 DEBUG [cloud.async.AsyncJobManagerImpl]
>> >> >(Job-Executor-32:job-29) Complete async job-29, jobStatus: 2,
>> >> >resultCode: 530, result:
>> >> >com.cloud.api.response.ExceptionResponse@3fa28ae
>> >> >
>> >> >And nothing else.
>> >> >
>> >> >Could we just output every ExceptionResponse in the log? That's
>> >> >would be much more helpful.
>> >> >
>> >> >--Sheng
>> >> >
>> >>
>> >
>> >
>>
>
>
RE: Add ExceptionResponse in the log?
Posted by Alex Huang <Al...@citrix.com>.
We should NOT print stack traces in debug. I'm not sure exactly what this is but all exception traces should be logged with at least a WARNING.
--Alex
> -----Original Message-----
> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
> Sent: Monday, July 09, 2012 3:18 PM
> To: cloudstack-dev@incubator.apache.org; Frank Zhang
> Subject: Re: Add ExceptionResponse in the log?
>
> On 7/9/12 2:58 PM, "Vijayendra Bhamidipati"
> <vi...@citrix.com> wrote:
>
> >Hi Frank/Alena/Sheng!
> >
> >Actually I hadn't modified that part of the code when making changes
> >earlier, but we definitely should be seeing more than just
> >classname@hash-of-object which is toString()'s default output when it's
> >passed an Object, so I'll do what Alena suggested and override toString
> >in the ExceptionResponse class. However, I have a question for Frank -
> >is it necessary for us to print out the exception stack trace, since
> >that would surely show up in the mgmt. server log file? Also, wanting
> >to print the stack trace would mean having to change quite a few parts
> >of the code to pass the exception object to the calls that build the
> >responses to the exception. Any specific reason why this wasn't the
> >approach to begin with, other than that the exception stack trace would
> >show up in the mgmt. server log file?
> >
> >Finally, the function CompleteAsyncJob() which prints out the message
> >Sheng referred to:
> >
> >s_logger.debug("Complete async job-" + jobId + ", jobStatus: " +
> >jobStatus +
> > ", resultCode: " + resultCode + ", result: " +
> >resultObject);
>
>
> Once you implement toString() for Exception response, this debug
> statement will include the reason for the failure. Add errorText to the
> method.
>
>
> >
> >is invoked at different parts of the code that builds resultObject
> >differently - in extractVolume() in ManagementServerImpl.java and
> >updateDatabase() in UploadListener.java, it's of type ExtractResponse
> >instead of ExceptionResponse. So I think I'll have to override
> >toString() in both ExceptionResponse and ExtractResponse classes -
> >Alena, does that sound right?
>
>
> Nope, you do it just in ExceptionResponse. Extract functionality needs some
> refactoring as it didn't follow the API Async design from the very beginning.
>
>
> >
> >
> >Regards,
> >Vijay
> >
> >-----Original Message-----
> >From: Frank Zhang
> >Sent: Monday, July 09, 2012 11:12 AM
> >To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
> >Subject: RE: Add ExceptionResponse in the log?
> >
> >Vijay, please make sure stack trace is logged as well. Text of
> >Exception.getMessage() is definitely not enough
> >
> >> -----Original Message-----
> >> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
> >> Sent: Monday, July 09, 2012 10:53 AM
> >> To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
> >> Subject: Re: Add ExceptionResponse in the log?
> >>
> >> We should def. print out the reason.
> >>
> >> Vijay, just override toString() method in the Exception response to
> >> log the reason for the failure in addition to the error code.
> >>
> >> -Alena.
> >>
> >>
> >> On 7/9/12 10:44 AM, "Sheng Yang" <sh...@yasker.org> wrote:
> >>
> >> >After we add ExceptionResponse for user to better understand what's
> >> >wrong, there has been nothing left in the log for developer to know
> >> >what's wrong...
> >> >
> >> >What we got now is only something like:
> >> >
> >> >2012-07-09 19:22:10,098 DEBUG [cloud.async.AsyncJobManagerImpl]
> >> >(Job-Executor-32:job-29) Complete async job-29, jobStatus: 2,
> >> >resultCode: 530, result:
> >> >com.cloud.api.response.ExceptionResponse@3fa28ae
> >> >
> >> >And nothing else.
> >> >
> >> >Could we just output every ExceptionResponse in the log? That's
> >> >would be much more helpful.
> >> >
> >> >--Sheng
> >> >
> >>
> >
> >
>
Re: Add ExceptionResponse in the log?
Posted by Alena Prokharchyk <Al...@citrix.com>.
On 7/9/12 2:58 PM, "Vijayendra Bhamidipati"
<vi...@citrix.com> wrote:
>Hi Frank/Alena/Sheng!
>
>Actually I hadn't modified that part of the code when making changes
>earlier, but we definitely should be seeing more than just
>classname@hash-of-object which is toString()'s default output when it's
>passed an Object, so I'll do what Alena suggested and override toString
>in the ExceptionResponse class. However, I have a question for Frank - is
>it necessary for us to print out the exception stack trace, since that
>would surely show up in the mgmt. server log file? Also, wanting to print
>the stack trace would mean having to change quite a few parts of the code
>to pass the exception object to the calls that build the responses to the
>exception. Any specific reason why this wasn't the approach to begin
>with, other than that the exception stack trace would show up in the
>mgmt. server log file?
>
>Finally, the function CompleteAsyncJob() which prints out the message
>Sheng referred to:
>
>s_logger.debug("Complete async job-" + jobId + ", jobStatus: " +
>jobStatus +
> ", resultCode: " + resultCode + ", result: " +
>resultObject);
Once you implement toString() for Exception response, this debug statement
will include the reason for the failure. Add errorText to the method.
>
>is invoked at different parts of the code that builds resultObject
>differently - in extractVolume() in ManagementServerImpl.java and
>updateDatabase() in UploadListener.java, it's of type ExtractResponse
>instead of ExceptionResponse. So I think I'll have to override toString()
>in both ExceptionResponse and ExtractResponse classes - Alena, does that
>sound right?
Nope, you do it just in ExceptionResponse. Extract functionality needs
some refactoring as it didn't follow the API Async design from the very
beginning.
>
>
>Regards,
>Vijay
>
>-----Original Message-----
>From: Frank Zhang
>Sent: Monday, July 09, 2012 11:12 AM
>To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
>Subject: RE: Add ExceptionResponse in the log?
>
>Vijay, please make sure stack trace is logged as well. Text of
>Exception.getMessage() is definitely not enough
>
>> -----Original Message-----
>> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
>> Sent: Monday, July 09, 2012 10:53 AM
>> To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
>> Subject: Re: Add ExceptionResponse in the log?
>>
>> We should def. print out the reason.
>>
>> Vijay, just override toString() method in the Exception response to
>> log the reason for the failure in addition to the error code.
>>
>> -Alena.
>>
>>
>> On 7/9/12 10:44 AM, "Sheng Yang" <sh...@yasker.org> wrote:
>>
>> >After we add ExceptionResponse for user to better understand what's
>> >wrong, there has been nothing left in the log for developer to know
>> >what's wrong...
>> >
>> >What we got now is only something like:
>> >
>> >2012-07-09 19:22:10,098 DEBUG [cloud.async.AsyncJobManagerImpl]
>> >(Job-Executor-32:job-29) Complete async job-29, jobStatus: 2,
>> >resultCode: 530, result:
>> >com.cloud.api.response.ExceptionResponse@3fa28ae
>> >
>> >And nothing else.
>> >
>> >Could we just output every ExceptionResponse in the log? That's would
>> >be much more helpful.
>> >
>> >--Sheng
>> >
>>
>
>
RE: Add ExceptionResponse in the log?
Posted by Vijayendra Bhamidipati <vi...@citrix.com>.
Hi Frank/Alena/Sheng!
Actually I hadn't modified that part of the code when making changes earlier, but we definitely should be seeing more than just classname@hash-of-object which is toString()'s default output when it's passed an Object, so I'll do what Alena suggested and override toString in the ExceptionResponse class. However, I have a question for Frank - is it necessary for us to print out the exception stack trace, since that would surely show up in the mgmt. server log file? Also, wanting to print the stack trace would mean having to change quite a few parts of the code to pass the exception object to the calls that build the responses to the exception. Any specific reason why this wasn't the approach to begin with, other than that the exception stack trace would show up in the mgmt. server log file?
Finally, the function CompleteAsyncJob() which prints out the message Sheng referred to:
s_logger.debug("Complete async job-" + jobId + ", jobStatus: " + jobStatus +
", resultCode: " + resultCode + ", result: " + resultObject);
is invoked at different parts of the code that builds resultObject differently - in extractVolume() in ManagementServerImpl.java and updateDatabase() in UploadListener.java, it's of type ExtractResponse instead of ExceptionResponse. So I think I'll have to override toString() in both ExceptionResponse and ExtractResponse classes - Alena, does that sound right?
Regards,
Vijay
-----Original Message-----
From: Frank Zhang
Sent: Monday, July 09, 2012 11:12 AM
To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
Subject: RE: Add ExceptionResponse in the log?
Vijay, please make sure stack trace is logged as well. Text of Exception.getMessage() is definitely not enough
> -----Original Message-----
> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
> Sent: Monday, July 09, 2012 10:53 AM
> To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
> Subject: Re: Add ExceptionResponse in the log?
>
> We should def. print out the reason.
>
> Vijay, just override toString() method in the Exception response to
> log the reason for the failure in addition to the error code.
>
> -Alena.
>
>
> On 7/9/12 10:44 AM, "Sheng Yang" <sh...@yasker.org> wrote:
>
> >After we add ExceptionResponse for user to better understand what's
> >wrong, there has been nothing left in the log for developer to know
> >what's wrong...
> >
> >What we got now is only something like:
> >
> >2012-07-09 19:22:10,098 DEBUG [cloud.async.AsyncJobManagerImpl]
> >(Job-Executor-32:job-29) Complete async job-29, jobStatus: 2,
> >resultCode: 530, result:
> >com.cloud.api.response.ExceptionResponse@3fa28ae
> >
> >And nothing else.
> >
> >Could we just output every ExceptionResponse in the log? That's would
> >be much more helpful.
> >
> >--Sheng
> >
>
RE: Add ExceptionResponse in the log?
Posted by Frank Zhang <Fr...@citrix.com>.
Vijay, please make sure stack trace is logged as well. Text of Exception.getMessage() is definitely not enough
> -----Original Message-----
> From: Alena Prokharchyk [mailto:Alena.Prokharchyk@citrix.com]
> Sent: Monday, July 09, 2012 10:53 AM
> To: cloudstack-dev@incubator.apache.org; Vijayendra Bhamidipati
> Subject: Re: Add ExceptionResponse in the log?
>
> We should def. print out the reason.
>
> Vijay, just override toString() method in the Exception response to log the
> reason for the failure in addition to the error code.
>
> -Alena.
>
>
> On 7/9/12 10:44 AM, "Sheng Yang" <sh...@yasker.org> wrote:
>
> >After we add ExceptionResponse for user to better understand what's
> >wrong, there has been nothing left in the log for developer to know
> >what's wrong...
> >
> >What we got now is only something like:
> >
> >2012-07-09 19:22:10,098 DEBUG [cloud.async.AsyncJobManagerImpl]
> >(Job-Executor-32:job-29) Complete async job-29, jobStatus: 2,
> >resultCode: 530, result:
> >com.cloud.api.response.ExceptionResponse@3fa28ae
> >
> >And nothing else.
> >
> >Could we just output every ExceptionResponse in the log? That's would
> >be much more helpful.
> >
> >--Sheng
> >
>
Re: Add ExceptionResponse in the log?
Posted by Alena Prokharchyk <Al...@citrix.com>.
We should def. print out the reason.
Vijay, just override toString() method in the Exception response to log
the reason for the failure in addition to the error code.
-Alena.
On 7/9/12 10:44 AM, "Sheng Yang" <sh...@yasker.org> wrote:
>After we add ExceptionResponse for user to better understand what's
>wrong, there has been nothing left in the log for developer to know
>what's wrong...
>
>What we got now is only something like:
>
>2012-07-09 19:22:10,098 DEBUG [cloud.async.AsyncJobManagerImpl]
>(Job-Executor-32:job-29) Complete async job-29, jobStatus: 2,
>resultCode: 530, result:
>com.cloud.api.response.ExceptionResponse@3fa28ae
>
>And nothing else.
>
>Could we just output every ExceptionResponse in the log? That's would
>be much more helpful.
>
>--Sheng
>