From b014e3b0494c123ee01b52b3ceeaa1a4cefd1275 Mon Sep 17 00:00:00 2001 From: nirsky Date: Thu, 7 May 2020 10:19:08 +0300 Subject: [PATCH 1/6] Implement apply_custom_attributes_on_span on requests ext --- .../opentelemetry/ext/requests/__init__.py | 11 ++++- .../tests/test_requests_integration.py | 42 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py index c98a24cc88..cc59f40684 100644 --- a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py +++ b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py @@ -55,7 +55,7 @@ # pylint: disable=unused-argument -def _instrument(tracer_provider=None): +def _instrument(tracer_provider=None, apply_custom_attributes_on_span=None): """Enables tracing of all requests calls that go through :code:`requests.session.Session.request` (this includes :code:`requests.get`, etc.).""" @@ -101,6 +101,8 @@ def instrumented_request(self, method, url, *args, **kwargs): span.set_status( Status(_http_status_to_canonical_code(result.status_code)) ) + if apply_custom_attributes_on_span is not None: + apply_custom_attributes_on_span(span, result) return result @@ -157,7 +159,12 @@ def _http_status_to_canonical_code(code: int, allow_redirect: bool = True): class RequestsInstrumentor(BaseInstrumentor): def _instrument(self, **kwargs): - _instrument(tracer_provider=kwargs.get("tracer_provider")) + _instrument( + tracer_provider=kwargs.get("tracer_provider"), + apply_custom_attributes_on_span=kwargs.get( + "apply_custom_attributes_on_span" + ), + ) def _uninstrument(self, **kwargs): _uninstrument() diff --git a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py index 7764aad3ec..16b6634617 100644 --- a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import sys import httpretty @@ -36,6 +37,7 @@ def setUp(self): httpretty.register_uri( httpretty.GET, self.URL, body="Hello!", ) + httpretty.register_uri(httpretty.POST, self.URL, body="World!") def tearDown(self): super().tearDown() @@ -183,6 +185,46 @@ def test_distributed_context(self): finally: propagators.set_global_httptextformat(previous_propagator) + def test_apply_custom_attributes_on_span(self): + resource = resources.Resource.create({}) + result = self.create_tracer_provider(resource=resource) + tracer_provider, exporter = result + RequestsInstrumentor().uninstrument() + + def outgoing_http_custom_attributes(span, result: requests.Response): + request_body = result.request.body + span.set_attribute("http.request.body", result.request.body) + span.set_attribute( + "http.response.body", result.content.decode("utf-8") + ) + + RequestsInstrumentor().instrument( + tracer_provider=tracer_provider, + apply_custom_attributes_on_span=outgoing_http_custom_attributes, + ) + + result = requests.post(self.URL, data=json.dumps({"hello": "world"})) + self.assertEqual(result.text, "World!") + + span_list = exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + span = span_list[0] + + self.assertEqual( + span.attributes, + { + "component": "http", + "http.method": "POST", + "http.url": self.URL, + "http.status_code": 200, + "http.status_text": "OK", + "http.request.body": '{"hello": "world"}', + "http.response.body": "World!", + }, + ) + + self.assertIs(span.resource, resource) + def test_custom_tracer_provider(self): resource = resources.Resource.create({}) result = self.create_tracer_provider(resource=resource) From 2b7b8b0ea4bacf78d7b2881d0ff82f7bdefb9fbb Mon Sep 17 00:00:00 2001 From: nirsky Date: Thu, 7 May 2020 10:24:12 +0300 Subject: [PATCH 2/6] remove redundant line from test --- .../tests/test_requests_integration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py index 16b6634617..ac0bd94a29 100644 --- a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py @@ -192,7 +192,6 @@ def test_apply_custom_attributes_on_span(self): RequestsInstrumentor().uninstrument() def outgoing_http_custom_attributes(span, result: requests.Response): - request_body = result.request.body span.set_attribute("http.request.body", result.request.body) span.set_attribute( "http.response.body", result.content.decode("utf-8") From 99cf7478bb2d0539cdcdf43593f1f25577ec4de9 Mon Sep 17 00:00:00 2001 From: nirsky Date: Sun, 10 May 2020 10:22:23 +0300 Subject: [PATCH 3/6] CR Fixes --- .../opentelemetry/ext/requests/__init__.py | 10 +++----- .../tests/test_requests_integration.py | 25 ++++++------------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py index cc59f40684..f32f51a583 100644 --- a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py +++ b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py @@ -55,7 +55,7 @@ # pylint: disable=unused-argument -def _instrument(tracer_provider=None, apply_custom_attributes_on_span=None): +def _instrument(tracer_provider=None, span_callback=None): """Enables tracing of all requests calls that go through :code:`requests.session.Session.request` (this includes :code:`requests.get`, etc.).""" @@ -101,8 +101,8 @@ def instrumented_request(self, method, url, *args, **kwargs): span.set_status( Status(_http_status_to_canonical_code(result.status_code)) ) - if apply_custom_attributes_on_span is not None: - apply_custom_attributes_on_span(span, result) + if span_callback is not None: + span_callback(span, result) return result @@ -161,9 +161,7 @@ class RequestsInstrumentor(BaseInstrumentor): def _instrument(self, **kwargs): _instrument( tracer_provider=kwargs.get("tracer_provider"), - apply_custom_attributes_on_span=kwargs.get( - "apply_custom_attributes_on_span" - ), + span_callback=kwargs.get("span_callback"), ) def _uninstrument(self, **kwargs): diff --git a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py index ac0bd94a29..cbce88e7d5 100644 --- a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py @@ -37,7 +37,6 @@ def setUp(self): httpretty.register_uri( httpretty.GET, self.URL, body="Hello!", ) - httpretty.register_uri(httpretty.POST, self.URL, body="World!") def tearDown(self): super().tearDown() @@ -185,27 +184,22 @@ def test_distributed_context(self): finally: propagators.set_global_httptextformat(previous_propagator) - def test_apply_custom_attributes_on_span(self): - resource = resources.Resource.create({}) - result = self.create_tracer_provider(resource=resource) - tracer_provider, exporter = result + def test_span_callback(self): RequestsInstrumentor().uninstrument() - def outgoing_http_custom_attributes(span, result: requests.Response): - span.set_attribute("http.request.body", result.request.body) + def span_callback(span, result: requests.Response): span.set_attribute( "http.response.body", result.content.decode("utf-8") ) RequestsInstrumentor().instrument( - tracer_provider=tracer_provider, - apply_custom_attributes_on_span=outgoing_http_custom_attributes, + tracer_provider=self.tracer_provider, span_callback=span_callback, ) - result = requests.post(self.URL, data=json.dumps({"hello": "world"})) - self.assertEqual(result.text, "World!") + result = requests.get(self.URL) + self.assertEqual(result.text, "Hello!") - span_list = exporter.get_finished_spans() + span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) span = span_list[0] @@ -213,17 +207,14 @@ def outgoing_http_custom_attributes(span, result: requests.Response): span.attributes, { "component": "http", - "http.method": "POST", + "http.method": "GET", "http.url": self.URL, "http.status_code": 200, "http.status_text": "OK", - "http.request.body": '{"hello": "world"}', - "http.response.body": "World!", + "http.response.body": "Hello!", }, ) - self.assertIs(span.resource, resource) - def test_custom_tracer_provider(self): resource = resources.Resource.create({}) result = self.create_tracer_provider(resource=resource) From fd1614078eb5cd9d3e7aa01f631a4a632075d044 Mon Sep 17 00:00:00 2001 From: nirsky Date: Sun, 10 May 2020 10:34:47 +0300 Subject: [PATCH 4/6] Added docstring --- .../src/opentelemetry/ext/requests/__init__.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py index f32f51a583..d59b0d1977 100644 --- a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py +++ b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py @@ -158,7 +158,18 @@ def _http_status_to_canonical_code(code: int, allow_redirect: bool = True): class RequestsInstrumentor(BaseInstrumentor): + """An instrumentor for requests + See `BaseInstrumentor` + """ + def _instrument(self, **kwargs): + """Instruments requests module + + Args: + **kwargs: Optional arguments + ``tracer_provider``: a TracerProvider, defaults to global + ``span_callback``: On optional callback invoked before returning the http response. Invoked with Span and requests.Response + """ _instrument( tracer_provider=kwargs.get("tracer_provider"), span_callback=kwargs.get("span_callback"), From b9f4bcfa6e105f45b1d67c5a9bfa3ee5742a863b Mon Sep 17 00:00:00 2001 From: nirsky Date: Sun, 10 May 2020 13:30:24 +0300 Subject: [PATCH 5/6] fix typo --- .../src/opentelemetry/ext/requests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py index d59b0d1977..2a82e9820c 100644 --- a/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py +++ b/ext/opentelemetry-ext-requests/src/opentelemetry/ext/requests/__init__.py @@ -168,7 +168,7 @@ def _instrument(self, **kwargs): Args: **kwargs: Optional arguments ``tracer_provider``: a TracerProvider, defaults to global - ``span_callback``: On optional callback invoked before returning the http response. Invoked with Span and requests.Response + ``span_callback``: An optional callback invoked before returning the http response. Invoked with Span and requests.Response """ _instrument( tracer_provider=kwargs.get("tracer_provider"), From 493670b345d7038f11e3337e74e5313ac2ec8376 Mon Sep 17 00:00:00 2001 From: nirsky Date: Sun, 10 May 2020 13:31:51 +0300 Subject: [PATCH 6/6] remove unused import --- .../tests/test_requests_integration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py index cbce88e7d5..28359d8f38 100644 --- a/ext/opentelemetry-ext-requests/tests/test_requests_integration.py +++ b/ext/opentelemetry-ext-requests/tests/test_requests_integration.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import json import sys import httpretty