Improved server logging #1521
Improved server logging #1521
Conversation
|
Changes Unknown when pulling 740c36c on necaris:brain-dead-server-logging into * on blaze:master*. |
|
@kwmsmith Without knowing more about what users want to track in the server this is pretty much good to go -- I just added a couple of debug logging calls and defaulting to |
|
@necaris great. @llllllllll thoughts? |
|
Changes Unknown when pulling 0a43fd6 on necaris:brain-dead-server-logging into * on blaze:master*. |
|
I like it! ping @richafrank |
| logger = flask.current_app.logger | ||
| logger.debug("Calling %s" % func.__name__) | ||
| ret = func(*args, **kwargs) | ||
| logger.debug("Leaving %s" % func.__name__) |
There was a problem hiding this comment.
Would it be useful to log "Leaving" in the exception case, i.e. should this be in a finally?
|
I was looking for any danger in returning exception tracebacks to endpoint consumers, but since those consumers would have passed |
| except Exception as e: | ||
| return ("Computation failed with message:\n%s: %s" % (type(e).__name__, e), | ||
| RC.INTERNAL_SERVER_ERROR) | ||
| error_msg = "Computation failed with message:\n%s: %s\n%s" % (type(e).__name__, e, traceback.format_exc()) |
There was a problem hiding this comment.
Could we make the format_exc function configurable? The use case that we have is that we run an extension of blaze server which may potentially have sensitive information in the traceback, we would like to filter the information the client sees before sending it over. I was thinking a function from traceback to str.
To pass the traceback to the function in a py2/3 compat way we could do:
tbmsg = serialize_traceback(sys.exc_info()[2]) # it is important not to store exc_info or the tb in a localBy default it could be toolz.compose(''.join, traceback.format_tb) which will do the same as it is currently doing.
Also, how do you feel about printing the traceback and then the error. Right now the format will look like:
client tb
server error message
server tb
and I think it may be nicer as:
client tb
server tb
server error message
|
@kwmsmith @llllllllll Added requested configurability in be85e1a |
|
@necaris thoughts on the travis failures? They aren't obvious to me... |
|
@kwmsmith Thanks for pointing them out! Looking into them now, I'm doing some refactoring to make them pass and adding some tests too. |
|
@kwmsmith Apologies for the half-baked PR. It should be good to go now -- refactored the configurable exception logging and added tests for it. And rebased so each commit should be a logical chunk of work. |
Also add documentation for how the parameters should be used.
Ensure that if desired logs can capture server startup and shutdown, as well as detect when a dataset was added. Note that the compute operation already has timing log output, and the default Flask logs for the datashape check operation should contain all the information that was in the request.
As suggested at #1521 (diff) Includes tests for the logged output
Takes #1492 and adds to it, defaulting logging to
sys.stdoutand adding documentation strings for the new parameters.As far as basic logging of output this is already done, however we will probably want to add more specific logging for different scenarios -- this can be in this PR or in a future update.