Conversation
kezhenxu94
left a comment
kezhenxu94
left a comment
There was a problem hiding this comment.
@alonelaval thanks very much
- We tend to keep a single blank line at the end of a file, please keep those
- It seems that the tests(
expected.data.yaml) are not adjusted according to this plugin
| def _sw_handle_user_exception(this: Flask, e): | ||
| if e is not None: | ||
| entry_span = get_context().active_span() | ||
| entry_span.log(e) |
There was a problem hiding this comment.
We don't actually have a log method on Span type, please implement it first
There was a problem hiding this comment.
And better to have a sanity check here, entry_span is possible to be None
There was a problem hiding this comment.
Please also mark the span as error by setting error_occurred to True
There was a problem hiding this comment.
And, can we get the response of the request, then we can determine the span's status by the http status code, like other plugins
There was a problem hiding this comment.
ok，i use skywalking 6.6 test... i will use skywalking 8.0.1 test first.
|
|
||
| except Exception: | ||
| logger.warning('failed to install plugin %s', __name__) | ||
| traceback.print_exc() |
There was a problem hiding this comment.
I suggest not to print the traceback in this plugin, other plugins are for built-in libraries, which are rarely failed to install, but this plugin is tend to be failed because not all users are using Flask
| import socketserver | ||
| from http.server import BaseHTTPRequestHandler | ||
|
|
||
| class SimpleHTTPRequestHandler(BaseHTTPRequestHandler): |
There was a problem hiding this comment.
It'd be better to use Flask as well in the consumer service, we can check the propagation of context by that
|
Forse push and close? What happens? |
3 participants
Closes apache/skywalking#4743