From ac6c22f3cb19f1ff6c1ec4d87b1eb8c1f58ce49d Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 20 Nov 2020 12:23:43 -0300 Subject: [PATCH 1/5] [Plugin] http servers set URL and status tags --- skywalking/plugins/sw_http_server.py | 51 +++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/skywalking/plugins/sw_http_server.py b/skywalking/plugins/sw_http_server.py index 47d1f356..3bd31fb5 100644 --- a/skywalking/plugins/sw_http_server.py +++ b/skywalking/plugins/sw_http_server.py @@ -41,6 +41,14 @@ def _sw_handle(handler: BaseHTTPRequestHandler): BaseHTTPRequestHandler.handle = _sw_handle + def _sw_send_response_only(self, code, *args, **kwargs): + self._status_code = code + + return _send_response_only(self, code, *args, **kwargs) + + _send_response_only = BaseHTTPRequestHandler.send_response_only + BaseHTTPRequestHandler.send_response_only = _sw_send_response_only + def wrap_werkzeug_request_handler(handler): """ @@ -53,15 +61,38 @@ def _wrap_run_wsgi(): carrier = Carrier() for item in carrier: item.val = handler.headers[item.key.capitalize()] - with context.new_entry_span(op=handler.path, carrier=carrier) as span: + path = handler.path or '/' + with context.new_entry_span(op=path.split("?")[0], carrier=carrier) as span: + url = 'http://' + handler.headers["Host"] + path if 'Host' in handler.headers else path span.layer = Layer.Http span.component = Component.General span.peer = '%s:%s' % handler.client_address span.tag(Tag(key=tags.HttpMethod, val=handler.command)) - return _run_wsgi() + span.tag(Tag(key=tags.HttpUrl, val=url)) + + try: + return _run_wsgi() + finally: + status_code = int(getattr(handler, '_status_code', -1)) + if status_code > -1: + span.tag(Tag(key=tags.HttpStatus, val=status_code)) + if status_code >= 400: + span.error_occurred = True handler.run_wsgi = _wrap_run_wsgi + def _sw_send_response(self, code, *args, **kwargs): + self._status_code = code + + return _send_response(self, code, *args, **kwargs) + + WSGIRequestHandler = handler.__class__ + + if not getattr(WSGIRequestHandler, '_sw_wrapped', False): + _send_response = WSGIRequestHandler.send_response + WSGIRequestHandler.send_response = _sw_send_response + WSGIRequestHandler._sw_wrapped = True + def wrap_default_request_handler(handler): http_methods = ('GET', 'HEAD', 'POST', 'PUT', 'DELETE', 'CONNECT', 'OPTIONS', 'TRACE', 'PATCH') @@ -78,12 +109,22 @@ def _sw_do_method(): carrier = Carrier() for item in carrier: item.val = handler.headers[item.key.capitalize()] - with context.new_entry_span(op=handler.path, carrier=carrier) as span: + path = handler.path or '/' + with context.new_entry_span(op=path.split("?")[0], carrier=carrier) as span: + url = 'http://' + handler.headers["Host"] + path if 'Host' in handler.headers else path span.layer = Layer.Http span.component = Component.General span.peer = '%s:%s' % handler.client_address span.tag(Tag(key=tags.HttpMethod, val=method)) - - _do_method() + span.tag(Tag(key=tags.HttpUrl, val=url)) + + try: + _do_method() + finally: + status_code = int(getattr(handler, '_status_code', -1)) + if status_code > -1: + span.tag(Tag(key=tags.HttpStatus, val=status_code)) + if status_code >= 400: + span.error_occurred = True setattr(handler, 'do_' + method, _sw_do_method) From 586c3b6f78d692fdee4de6dc45a19efa24fa18de Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 20 Nov 2020 13:09:27 -0300 Subject: [PATCH 2/5] [Plugin] made all status tags overridable --- skywalking/plugins/sw_django.py | 2 +- skywalking/plugins/sw_flask.py | 2 +- skywalking/plugins/sw_http_server.py | 4 ++-- skywalking/plugins/sw_requests.py | 2 +- skywalking/plugins/sw_tornado.py | 4 ++-- skywalking/plugins/sw_urllib3.py | 2 +- skywalking/plugins/sw_urllib_request.py | 4 ++-- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/skywalking/plugins/sw_django.py b/skywalking/plugins/sw_django.py index 619407b2..9229acca 100644 --- a/skywalking/plugins/sw_django.py +++ b/skywalking/plugins/sw_django.py @@ -66,7 +66,7 @@ def _sw_get_response(this, request): val=params_tostring(request.GET)[0:config.http_params_length_threshold])) resp = _get_response(this, request) - span.tag(Tag(key=tags.HttpStatus, val=resp.status_code)) + span.tag(Tag(key=tags.HttpStatus, val=resp.status_code, overridable=True)) if resp.status_code >= 400: span.error_occurred = True return resp diff --git a/skywalking/plugins/sw_flask.py b/skywalking/plugins/sw_flask.py index 3e6f26e6..320e104e 100644 --- a/skywalking/plugins/sw_flask.py +++ b/skywalking/plugins/sw_flask.py @@ -58,7 +58,7 @@ def _sw_full_dispatch_request(this: Flask): if resp.status_code >= 400: span.error_occurred = True - span.tag(Tag(key=tags.HttpStatus, val=resp.status_code)) + span.tag(Tag(key=tags.HttpStatus, val=resp.status_code, overridable=True)) return resp def _sw_handle_user_exception(this: Flask, e): diff --git a/skywalking/plugins/sw_http_server.py b/skywalking/plugins/sw_http_server.py index 3bd31fb5..28f83a77 100644 --- a/skywalking/plugins/sw_http_server.py +++ b/skywalking/plugins/sw_http_server.py @@ -75,7 +75,7 @@ def _wrap_run_wsgi(): finally: status_code = int(getattr(handler, '_status_code', -1)) if status_code > -1: - span.tag(Tag(key=tags.HttpStatus, val=status_code)) + span.tag(Tag(key=tags.HttpStatus, val=status_code, overridable=True)) if status_code >= 400: span.error_occurred = True @@ -123,7 +123,7 @@ def _sw_do_method(): finally: status_code = int(getattr(handler, '_status_code', -1)) if status_code > -1: - span.tag(Tag(key=tags.HttpStatus, val=status_code)) + span.tag(Tag(key=tags.HttpStatus, val=status_code, overridable=True)) if status_code >= 400: span.error_occurred = True diff --git a/skywalking/plugins/sw_requests.py b/skywalking/plugins/sw_requests.py index d53c2bf0..af885162 100644 --- a/skywalking/plugins/sw_requests.py +++ b/skywalking/plugins/sw_requests.py @@ -68,7 +68,7 @@ def _sw_request(this: Session, method, url, proxies, hooks, stream, verify, cert, json) - span.tag(Tag(key=tags.HttpStatus, val=res.status_code)) + span.tag(Tag(key=tags.HttpStatus, val=res.status_code, overridable=True)) if res.status_code >= 400: span.error_occurred = True diff --git a/skywalking/plugins/sw_tornado.py b/skywalking/plugins/sw_tornado.py index 45896b07..a7fae191 100644 --- a/skywalking/plugins/sw_tornado.py +++ b/skywalking/plugins/sw_tornado.py @@ -66,7 +66,7 @@ async def _sw_get_response(self, *args, **kwargs): span.tag( Tag(key=tags.HttpUrl, val='{}://{}{}'.format(request.protocol, request.host, request.path))) result = old_execute(self, *args, **kwargs) - span.tag(Tag(key=tags.HttpStatus, val=self._status_code)) + span.tag(Tag(key=tags.HttpStatus, val=self._status_code, overridable=True)) if isawaitable(result): result = await result if self._status_code >= 400: @@ -90,7 +90,7 @@ def _sw_get_response(self, *args, **kwargs): span.tag( Tag(key=tags.HttpUrl, val='{}://{}{}'.format(request.protocol, request.host, request.path))) result = yield from old_execute(self, *args, **kwargs) - span.tag(Tag(key=tags.HttpStatus, val=self._status_code)) + span.tag(Tag(key=tags.HttpStatus, val=self._status_code, overridable=True)) if self._status_code >= 400: span.error_occurred = True return result diff --git a/skywalking/plugins/sw_urllib3.py b/skywalking/plugins/sw_urllib3.py index f0ae749d..5ac1c2f4 100644 --- a/skywalking/plugins/sw_urllib3.py +++ b/skywalking/plugins/sw_urllib3.py @@ -54,7 +54,7 @@ def _sw_request(this: RequestMethods, method, url, fields=None, headers=None, ** res = _request(this, method, url, fields=fields, headers=headers, **urlopen_kw) - span.tag(Tag(key=tags.HttpStatus, val=res.status)) + span.tag(Tag(key=tags.HttpStatus, val=res.status, overridable=True)) if res.status >= 400: span.error_occurred = True diff --git a/skywalking/plugins/sw_urllib_request.py b/skywalking/plugins/sw_urllib_request.py index 5baad82f..fdd276f1 100644 --- a/skywalking/plugins/sw_urllib_request.py +++ b/skywalking/plugins/sw_urllib_request.py @@ -49,13 +49,13 @@ def _sw_open(this: OpenerDirector, fullurl, data, timeout): try: res = _open(this, fullurl, data, timeout) except HTTPError as e: - span.tag(Tag(key=tags.HttpStatus, val=e.code)) + span.tag(Tag(key=tags.HttpStatus, val=e.code, overridable=True)) raise finally: # we do this here because it may change in _open() span.tag(Tag(key=tags.HttpMethod, val=fullurl.get_method())) span.tag(Tag(key=tags.HttpUrl, val=fullurl.full_url)) - span.tag(Tag(key=tags.HttpStatus, val=res.code)) + span.tag(Tag(key=tags.HttpStatus, val=res.code, overridable=True)) if res.code >= 400: span.error_occurred = True From 44a3a992429ff2f10cf6f60efcc204718e4d53a5 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 20 Nov 2020 13:22:52 -0300 Subject: [PATCH 3/5] Fix: Span.tag() for overridable --- skywalking/trace/span.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/skywalking/trace/span.py b/skywalking/trace/span.py index 53c36c73..0b458b33 100644 --- a/skywalking/trace/span.py +++ b/skywalking/trace/span.py @@ -87,14 +87,13 @@ def log(self, ex: Exception) -> 'Span': return self def tag(self, tag: Tag) -> 'Span': - if not tag.overridable: - self.tags.append(deepcopy(tag)) - return self + if tag.overridable: + for i, t in enumerate(self.tags): + if t.key == tag.key: + self.tags[i] = deepcopy(tag) + return self - for t in self.tags: - if t.key == tag.key: - t.val = tag.val - break + self.tags.append(deepcopy(tag)) return self From 8eb810a6651bc94e39ab6d2aeb0910424826c4e6 Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 20 Nov 2020 14:38:40 -0300 Subject: [PATCH 4/5] updated tests for new tags --- tests/plugin/sw_http/expected.data.yml | 8 ++++++++ tests/plugin/sw_http_wsgi/expected.data.yml | 8 ++++++++ tests/plugin/sw_requests/expected.data.yml | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/tests/plugin/sw_http/expected.data.yml b/tests/plugin/sw_http/expected.data.yml index 509127e9..c73c8cb6 100644 --- a/tests/plugin/sw_http/expected.data.yml +++ b/tests/plugin/sw_http/expected.data.yml @@ -29,6 +29,10 @@ segmentItems: tags: - key: http.method value: POST + - key: url + value: http://provider:9091/users + - key: status.code + value: '200' refs: - parentEndpoint: /users networkAddress: provider:9091 @@ -75,6 +79,10 @@ segmentItems: tags: - key: http.method value: POST + - key: url + value: http://0.0.0.0:9090/ + - key: status.code + value: '200' startTime: gt 0 endTime: gt 0 componentId: 7000 diff --git a/tests/plugin/sw_http_wsgi/expected.data.yml b/tests/plugin/sw_http_wsgi/expected.data.yml index 509127e9..c73c8cb6 100644 --- a/tests/plugin/sw_http_wsgi/expected.data.yml +++ b/tests/plugin/sw_http_wsgi/expected.data.yml @@ -29,6 +29,10 @@ segmentItems: tags: - key: http.method value: POST + - key: url + value: http://provider:9091/users + - key: status.code + value: '200' refs: - parentEndpoint: /users networkAddress: provider:9091 @@ -75,6 +79,10 @@ segmentItems: tags: - key: http.method value: POST + - key: url + value: http://0.0.0.0:9090/ + - key: status.code + value: '200' startTime: gt 0 endTime: gt 0 componentId: 7000 diff --git a/tests/plugin/sw_requests/expected.data.yml b/tests/plugin/sw_requests/expected.data.yml index bfd3a760..cdfd260c 100644 --- a/tests/plugin/sw_requests/expected.data.yml +++ b/tests/plugin/sw_requests/expected.data.yml @@ -29,6 +29,10 @@ segmentItems: tags: - key: http.method value: POST + - key: url + value: http://provider:9091/users + - key: status.code + value: '200' refs: - parentEndpoint: /users networkAddress: provider:9091 @@ -75,6 +79,10 @@ segmentItems: tags: - key: http.method value: POST + - key: url + value: http://0.0.0.0:9090/ + - key: status.code + value: '200' startTime: gt 0 endTime: gt 0 componentId: 7000 From 9675cae54a26491c3d033c6bc3d304ed30c2d4aa Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Fri, 20 Nov 2020 19:30:08 -0300 Subject: [PATCH 5/5] Fix: _sw_open(), Span.__exit__() --- skywalking/plugins/sw_urllib_request.py | 15 ++++++++++----- skywalking/trace/span.py | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/skywalking/plugins/sw_urllib_request.py b/skywalking/plugins/sw_urllib_request.py index fdd276f1..a0e81d8d 100644 --- a/skywalking/plugins/sw_urllib_request.py +++ b/skywalking/plugins/sw_urllib_request.py @@ -28,12 +28,13 @@ def install(): + import socket from urllib.request import OpenerDirector from urllib.error import HTTPError _open = OpenerDirector.open - def _sw_open(this: OpenerDirector, fullurl, data, timeout): + def _sw_open(this: OpenerDirector, fullurl, data=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT): if isinstance(fullurl, str): fullurl = Request(fullurl, data) @@ -43,21 +44,25 @@ def _sw_open(this: OpenerDirector, fullurl, data, timeout): with context.new_exit_span(op=url, peer=fullurl.host, carrier=carrier) as span: span.layer = Layer.Http span.component = Component.General + code = None [fullurl.add_header(item.key, item.val) for item in carrier] try: res = _open(this, fullurl, data, timeout) + code = res.code except HTTPError as e: - span.tag(Tag(key=tags.HttpStatus, val=e.code, overridable=True)) + code = e.code raise finally: # we do this here because it may change in _open() span.tag(Tag(key=tags.HttpMethod, val=fullurl.get_method())) span.tag(Tag(key=tags.HttpUrl, val=fullurl.full_url)) - span.tag(Tag(key=tags.HttpStatus, val=res.code, overridable=True)) - if res.code >= 400: - span.error_occurred = True + if code is not None: + span.tag(Tag(key=tags.HttpStatus, val=code, overridable=True)) + + if code >= 400: + span.error_occurred = True return res diff --git a/skywalking/trace/span.py b/skywalking/trace/span.py index 0b458b33..2d2fcf5c 100644 --- a/skywalking/trace/span.py +++ b/skywalking/trace/span.py @@ -116,7 +116,7 @@ def __enter__(self): return self def __exit__(self, exc_type, exc_val, exc_tb): - if isinstance(exc_type, BaseException): + if isinstance(exc_val, BaseException): self.raised() self.stop() if exc_tb is not None: