ext/requests: Add instrumentor (#597)

Implement the BaseInstrumentor interface to make this library compatible with the opentelemetry-auto-instr command.

There is an issue about getting the span when the global tracer provider hasn't been configured, this should be changed in the future once we extend the opentelemetry-auto-instr command to also configure the SDK.

Author: mauriciovasquezbernal

docs/examples/http/client.py

@@ -28,15 +28,18 @@
 
 # The preferred tracer implementation must be set, as the opentelemetry-api
 # defines the interface with a no-op implementation.
+# It must be done before instrumenting any library.
 trace.set_tracer_provider(TracerProvider())
-tracer_provider = trace.get_tracer_provider()
 
+# Enable instrumentation in the requests library.
+http_requests.RequestsInstrumentor().instrument()
+
+# Configure a console span exporter.
 exporter = ConsoleSpanExporter()
 span_processor = BatchExportSpanProcessor(exporter)
-tracer_provider.add_span_processor(span_processor)
+trace.get_tracer_provider().add_span_processor(span_processor)
 
 # Integrations are the glue that binds the OpenTelemetry API and the
 # frameworks and libraries that are used together, automatically creating
 # Spans and propagating context as appropriate.
-http_requests.enable(tracer_provider)
 response = requests.get(url="http://127.0.0.1:5000/")

docs/examples/http/server.py

@@ -30,20 +30,23 @@
 
 # The preferred tracer implementation must be set, as the opentelemetry-api
 # defines the interface with a no-op implementation.
+# It must be done before instrumenting any library.
 trace.set_tracer_provider(TracerProvider())
-tracer = trace.get_tracer(__name__)
-
-exporter = ConsoleSpanExporter()
-span_processor = BatchExportSpanProcessor(exporter)
-trace.get_tracer_provider().add_span_processor(span_processor)
 
 # Integrations are the glue that binds the OpenTelemetry API and the
 # frameworks and libraries that are used together, automatically creating
 # Spans and propagating context as appropriate.
-http_requests.enable(trace.get_tracer_provider())
+http_requests.RequestsInstrumentor().instrument()
 app = flask.Flask(__name__)
 app.wsgi_app = OpenTelemetryMiddleware(app.wsgi_app)
 
+# Configure a console span exporter.
+exporter = ConsoleSpanExporter()
+span_processor = BatchExportSpanProcessor(exporter)
+trace.get_tracer_provider().add_span_processor(span_processor)
+
+tracer = trace.get_tracer(__name__)
+
 
 @app.route("/")
 def hello():

docs/examples/opentelemetry-example-app/src/opentelemetry_example_app/flask_example.py

@@ -28,14 +28,20 @@
     SimpleExportSpanProcessor,
 )
 
+# The preferred tracer implementation must be set, as the opentelemetry-api
+# defines the interface with a no-op implementation.
+# It must be done before instrumenting any library
 trace.set_tracer_provider(TracerProvider())
+
+opentelemetry.ext.http_requests.RequestsInstrumentor().instrument()
+FlaskInstrumentor().instrument()
+
 trace.get_tracer_provider().add_span_processor(
     SimpleExportSpanProcessor(ConsoleSpanExporter())
 )
 
-FlaskInstrumentor().instrument()
+
 app = flask.Flask(__name__)
-opentelemetry.ext.http_requests.enable(trace.get_tracer_provider())
 
 
 @app.route("/")

docs/getting-started.rst

@@ -202,7 +202,7 @@ And let's write a small Flask application that sends an HTTP request, activating
     )
 
     app = flask.Flask(__name__)
-    opentelemetry.ext.http_requests.enable(trace.get_tracer_provider())
+    opentelemetry.ext.http_requests.RequestsInstrumentor().instrument()
 
     @app.route("/")
     def hello():

ext/opentelemetry-ext-http-requests/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Implement instrumentor interface ([#597](https://github.com/open-telemetry/opentelemetry-python/pull/597))
+
 ## 0.3a0
 
 Released 2019-10-29

ext/opentelemetry-ext-http-requests/setup.cfg

@@ -41,6 +41,7 @@ package_dir=
 packages=find_namespace:
 install_requires =
     opentelemetry-api == 0.7.dev0
+    opentelemetry-auto-instrumentation == 0.7.dev0
     requests ~= 2.0
 
 [options.extras_require]
@@ -50,3 +51,7 @@ test =
 
 [options.packages.find]
 where = src
+
+[options.entry_points]
+opentelemetry_instrumentor =
+    requests = opentelemetry.ext.http_requests:RequestsInstrumentor

ext/opentelemetry-ext-http-requests/src/opentelemetry/ext/http_requests/__init__.py

@@ -14,7 +14,7 @@
 
 """
 This library allows tracing HTTP requests made by the
-`requests <https://requests.kennethreitz.org/en/master/>`_ library.
+`requests <https://requests.readthedocs.io/en/master/>`_ library.
 
 Usage
 -----
@@ -23,10 +23,10 @@
 
     import requests
     import opentelemetry.ext.http_requests
-    from opentelemetry.trace import TracerProvider
 
-    opentelemetry.ext.http_requests.enable(TracerProvider())
-    response = requests.get(url='https://www.example.org/')
+    # You can optionally pass a custom TracerProvider to RequestInstrumentor.instrument()
+    opentelemetry.ext.http_requests.RequestInstrumentor.instrument()
+    response = requests.get(url="https://www.example.org/")
 
 Limitations
 -----------
@@ -47,17 +47,15 @@
 
 from requests.sessions import Session
 
-from opentelemetry import context, propagators
+from opentelemetry import context, propagators, trace
+from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor
 from opentelemetry.ext.http_requests.version import __version__
-from opentelemetry.trace import SpanKind
+from opentelemetry.trace import SpanKind, get_tracer
+from opentelemetry.trace.status import Status, StatusCanonicalCode
 
 
-# NOTE: Currently we force passing a tracer. But in turn, this forces the user
-# to configure a SDK before enabling this integration. In turn, this means that
-# if the SDK/tracer is already using `requests` they may, in theory, bypass our
-# instrumentation when using `import from`, etc. (currently we only instrument
-# a instance method so the probability for that is very low).
-def enable(tracer_provider):
+# pylint: disable=unused-argument
+def _instrument(tracer_provider=None):
     """Enables tracing of all requests calls that go through
       :code:`requests.session.Session.request` (this includes
       :code:`requests.get`, etc.)."""
@@ -69,20 +67,17 @@ def enable(tracer_provider):
     # before v1.0.0, Dec 17, 2012, see
     # https://github.com/psf/requests/commit/4e5c4a6ab7bb0195dececdd19bb8505b872fe120)
 
-    # Guard against double instrumentation
-    disable()
-
-    tracer = tracer_provider.get_tracer(__name__, __version__)
-
     wrapped = Session.request
 
+    tracer = trace.get_tracer(__name__, __version__, tracer_provider)
+
     @functools.wraps(wrapped)
     def instrumented_request(self, method, url, *args, **kwargs):
         if context.get_value("suppress_instrumentation"):
             return wrapped(self, method, url, *args, **kwargs)
 
         # See
-        # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md#http-client
+        # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#http-client
         try:
             parsed_url = urlparse(url)
         except ValueError as exc:  # Invalid URL
@@ -103,12 +98,12 @@ def instrumented_request(self, method, url, *args, **kwargs):
 
             span.set_attribute("http.status_code", result.status_code)
             span.set_attribute("http.status_text", result.reason)
+            span.set_status(
+                Status(_http_status_to_canonical_code(result.status_code))
+            )
 
             return result
 
-        # TODO: How to handle exceptions? Should we create events for them? Set
-        # certain attributes?
-
     instrumented_request.opentelemetry_ext_requests_applied = True
 
     Session.request = instrumented_request
@@ -119,18 +114,59 @@ def instrumented_request(self, method, url, *args, **kwargs):
     # different, then push the current URL, pop it afterwards)
 
 
-def disable():
+def _uninstrument():
+    # pylint: disable=global-statement
     """Disables instrumentation of :code:`requests` through this module.
 
     Note that this only works if no other module also patches requests."""
-
     if getattr(Session.request, "opentelemetry_ext_requests_applied", False):
         original = Session.request.__wrapped__  # pylint:disable=no-member
         Session.request = original
 
 
-def disable_session(session):
-    """Disables instrumentation on the session object."""
-    if getattr(session.request, "opentelemetry_ext_requests_applied", False):
-        original = session.request.__wrapped__  # pylint:disable=no-member
-        session.request = types.MethodType(original, session)
+def _http_status_to_canonical_code(code: int, allow_redirect: bool = True):
+    # pylint:disable=too-many-branches,too-many-return-statements
+    if code < 100:
+        return StatusCanonicalCode.UNKNOWN
+    if code <= 299:
+        return StatusCanonicalCode.OK
+    if code <= 399:
+        if allow_redirect:
+            return StatusCanonicalCode.OK
+        return StatusCanonicalCode.DEADLINE_EXCEEDED
+    if code <= 499:
+        if code == 401:  # HTTPStatus.UNAUTHORIZED:
+            return StatusCanonicalCode.UNAUTHENTICATED
+        if code == 403:  # HTTPStatus.FORBIDDEN:
+            return StatusCanonicalCode.PERMISSION_DENIED
+        if code == 404:  # HTTPStatus.NOT_FOUND:
+            return StatusCanonicalCode.NOT_FOUND
+        if code == 429:  # HTTPStatus.TOO_MANY_REQUESTS:
+            return StatusCanonicalCode.RESOURCE_EXHAUSTED
+        return StatusCanonicalCode.INVALID_ARGUMENT
+    if code <= 599:
+        if code == 501:  # HTTPStatus.NOT_IMPLEMENTED:
+            return StatusCanonicalCode.UNIMPLEMENTED
+        if code == 503:  # HTTPStatus.SERVICE_UNAVAILABLE:
+            return StatusCanonicalCode.UNAVAILABLE
+        if code == 504:  # HTTPStatus.GATEWAY_TIMEOUT:
+            return StatusCanonicalCode.DEADLINE_EXCEEDED
+        return StatusCanonicalCode.INTERNAL
+    return StatusCanonicalCode.UNKNOWN
+
+
+class RequestsInstrumentor(BaseInstrumentor):
+    def _instrument(self, **kwargs):
+        _instrument(tracer_provider=kwargs.get("tracer_provider"))
+
+    def _uninstrument(self, **kwargs):
+        _uninstrument()
+
+    @staticmethod
+    def uninstrument_session(session):
+        """Disables instrumentation on the session object."""
+        if getattr(
+            session.request, "opentelemetry_ext_requests_applied", False
+        ):
+            original = session.request.__wrapped__  # pylint:disable=no-member
+            session.request = types.MethodType(original, session)

ext/opentelemetry-ext-http-requests/tests/test_requests_integration.py

@@ -18,8 +18,9 @@
 import requests
 import urllib3
 
-import opentelemetry.ext.http_requests
 from opentelemetry import context, propagators, trace
+from opentelemetry.ext import http_requests
+from opentelemetry.sdk import resources
 from opentelemetry.test.mock_httptextformat import MockHTTPTextFormat
 from opentelemetry.test.test_base import TestBase
 
@@ -29,23 +30,25 @@ class TestRequestsIntegration(TestBase):
 
     def setUp(self):
         super().setUp()
-        opentelemetry.ext.http_requests.enable(self.tracer_provider)
+        http_requests.RequestsInstrumentor().instrument()
         httpretty.enable()
         httpretty.register_uri(
             httpretty.GET, self.URL, body="Hello!",
         )
 
     def tearDown(self):
         super().tearDown()
-        opentelemetry.ext.http_requests.disable()
+        http_requests.RequestsInstrumentor().uninstrument()
         httpretty.disable()
 
     def test_basic(self):
         result = requests.get(self.URL)
         self.assertEqual(result.text, "Hello!")
+
         span_list = self.memory_exporter.get_finished_spans()
         self.assertEqual(len(span_list), 1)
         span = span_list[0]
+
         self.assertIs(span.kind, trace.SpanKind.CLIENT)
         self.assertEqual(span.name, "/status/200")
 
@@ -60,6 +63,32 @@ def test_basic(self):
             },
         )
 
+        self.assertIs(
+            span.status.canonical_code, trace.status.StatusCanonicalCode.OK
+        )
+
+        self.check_span_instrumentation_info(span, http_requests)
+
+    def test_not_foundbasic(self):
+        url_404 = "http://httpbin.org/status/404"
+        httpretty.register_uri(
+            httpretty.GET, url_404, status=404,
+        )
+        result = requests.get(url_404)
+        self.assertEqual(result.status_code, 404)
+
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)
+        span = span_list[0]
+
+        self.assertEqual(span.attributes.get("http.status_code"), 404)
+        self.assertEqual(span.attributes.get("http.status_text"), "Not Found")
+
+        self.assertIs(
+            span.status.canonical_code,
+            trace.status.StatusCanonicalCode.NOT_FOUND,
+        )
+
     def test_invalid_url(self):
         url = "http://[::1/nope"
         exception_type = requests.exceptions.InvalidURL
@@ -81,18 +110,18 @@ def test_invalid_url(self):
             {"component": "http", "http.method": "POST", "http.url": url},
         )
 
-    def test_disable(self):
-        opentelemetry.ext.http_requests.disable()
+    def test_uninstrument(self):
+        http_requests.RequestsInstrumentor().uninstrument()
         result = requests.get(self.URL)
         self.assertEqual(result.text, "Hello!")
         span_list = self.memory_exporter.get_finished_spans()
         self.assertEqual(len(span_list), 0)
+        # instrument again to avoid annoying warning message
+        http_requests.RequestsInstrumentor().instrument()
 
-        opentelemetry.ext.http_requests.disable()
-
-    def test_disable_session(self):
+    def test_uninstrument_session(self):
         session1 = requests.Session()
-        opentelemetry.ext.http_requests.disable_session(session1)
+        http_requests.RequestsInstrumentor().uninstrument_session(session1)
 
         result = session1.get(self.URL)
         self.assertEqual(result.text, "Hello!")
@@ -152,3 +181,21 @@ def test_distributed_context(self):
 
         finally:
             propagators.set_global_httptextformat(previous_propagator)
+
+    def test_custom_tracer_provider(self):
+        resource = resources.Resource.create({})
+        result = self.create_tracer_provider(resource=resource)
+        tracer_provider, exporter = result
+        http_requests.RequestsInstrumentor().uninstrument()
+        http_requests.RequestsInstrumentor().instrument(
+            tracer_provider=tracer_provider
+        )
+
+        result = requests.get(self.URL)
+        self.assertEqual(result.text, "Hello!")
+
+        span_list = exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)
+        span = span_list[0]
+
+        self.assertIs(span.resource, resource)

tests/w3c_tracecontext_validation_server.py

@@ -40,7 +40,7 @@
 # frameworks and libraries that are used together, automatically creating
 # Spans and propagating context as appropriate.
 trace.set_tracer_provider(TracerProvider())
-http_requests.enable(trace.get_tracer_provider())
+http_requests.RequestsInstrumentor().instrument()
 
 # SpanExporter receives the spans and send them to the target location.
 span_processor = SimpleExportSpanProcessor(ConsoleSpanExporter())