[GitHub] [skywalking-python] tom-pytel commented on a change in pull request #84: [Plugin] moved Method & Url tags before requests

[GitBox <gi...@apache.org>]

tom-pytel commented on a change in pull request #84:
URL: https://github.com/apache/skywalking-python/pull/84#discussion_r527680270



##########
File path: skywalking/plugins/sw_urllib_request.py
##########
@@ -38,15 +39,23 @@ def _sw_open(this: OpenerDirector, fullurl, data, timeout):
 
         context = get_context()
         carrier = Carrier()
-        with context.new_exit_span(op=fullurl.selector or '/', peer=fullurl.host, carrier=carrier) as span:
+        url = fullurl.selector.split("?")[0] if fullurl.selector else '/'
+        with context.new_exit_span(op=url, peer=fullurl.host, carrier=carrier) as span:
             span.layer = Layer.Http
             span.component = Component.General
 
             [fullurl.add_header(item.key, item.val) for item in carrier]
 
-            res = _open(this, fullurl, data, timeout)
-            span.tag(Tag(key=tags.HttpMethod, val=fullurl.get_method()))
-            span.tag(Tag(key=tags.HttpUrl, val=fullurl.full_url))
+            try:
+                res = _open(this, fullurl, data, timeout)
+                span.tag(Tag(key=tags.HttpMethod, val=fullurl.get_method()))
+            except HTTPError as e:
+                span.tag(Tag(key=tags.HttpStatus, val=e.code))
+                raise
+            finally:  # we do this here because it may change in _open()
+                span.tag(Tag(key=tags.HttpMethod, val=fullurl.get_method()))

Review comment:
       You are right I forgot to remove that, but I don't think that is the source of the problem, lets see...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org