From 09bb0912d188dc09fc5a23442644ffb405a668d7 Mon Sep 17 00:00:00 2001
From: Owais Lone <owais@users.noreply.github.com>
Date: Thu, 8 Oct 2020 08:22:58 +0530
Subject: [PATCH]  Added ability to extract span attributes from tornado
 request objects (#1178)

OTEL_PYTHON_TONADO_TRACED_REQUEST_ATTRS env var can be set to a command
separated list of attributes names that will be extracted from Tornado's
request object and set as attributes on spans.

Co-authored-by: (Eliseo) Nathaniel Ruiz Nowell <enruizno@uwaterloo.ca>
---
 .../CHANGELOG.md                                  |  2 ++
 .../README.rst                                    | 12 ++++++++++++
 .../instrumentation/tornado/__init__.py           | 14 +++++++++++++-
 .../tests/test_instrumentation.py                 | 15 +++++++++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/instrumentation/opentelemetry-instrumentation-tornado/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-tornado/CHANGELOG.md
index 7cc628718..e82446d66 100644
--- a/instrumentation/opentelemetry-instrumentation-tornado/CHANGELOG.md
+++ b/instrumentation/opentelemetry-instrumentation-tornado/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Added support for `OTEL_PYTHON_TORNADO_TRACED_REQUEST_ATTRS` ([#1178](https://github.com/open-telemetry/opentelemetry-python/pull/1178))
+
 ## Version 0.13b0
 
 Released 2020-09-17
diff --git a/instrumentation/opentelemetry-instrumentation-tornado/README.rst b/instrumentation/opentelemetry-instrumentation-tornado/README.rst
index d84fbd041..088c7f0e8 100644
--- a/instrumentation/opentelemetry-instrumentation-tornado/README.rst
+++ b/instrumentation/opentelemetry-instrumentation-tornado/README.rst
@@ -31,6 +31,18 @@ A comma separated list of paths that should not be automatically traced. For exa
 
 Then any requests made to ``/healthz`` and ``/ping`` will not be automatically traced.
 
+Request attributes
+********************
+To extract certain attributes from Tornado's request object and use them as span attributes, set the environment variable ``OTEL_PYTHON_TORNADO_TRACED_REQUEST_ATTRS`` to a comma
+delimited list of request attribute names. 
+
+For example,
+
+::
+
+    export OTEL_PYTHON_TORNADO_TRACED_REQUEST_ATTRS='uri,query'
+
+will extract path_info and content_type attributes from every traced request and add them as span attributes.
 
 References
 ----------
diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
index 6379d841a..5357be6d0 100644
--- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
+++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
@@ -50,6 +50,7 @@ from opentelemetry import configuration, context, propagators, trace
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
 from opentelemetry.instrumentation.tornado.version import __version__
 from opentelemetry.instrumentation.utils import (
+    extract_attributes_from_object,
     http_status_to_canonical_code,
     unwrap,
 )
@@ -71,7 +72,17 @@ def get_excluded_urls():
     return ExcludeList(urls)
 
 
+def get_traced_request_attrs():
+    attrs = configuration.Configuration().TORNADO_TRACED_REQUEST_ATTRS or ""
+    if attrs:
+        attrs = [attr.strip() for attr in attrs.split(",")]
+    else:
+        attrs = []
+    return attrs
+
+
 _excluded_urls = get_excluded_urls()
+_traced_attrs = get_traced_request_attrs()
 
 
 class TornadoInstrumentor(BaseInstrumentor):
@@ -196,7 +207,7 @@ def _get_attributes_from_request(request):
     if request.remote_ip:
         attrs["net.peer.ip"] = request.remote_ip
 
-    return attrs
+    return extract_attributes_from_object(request, _traced_attrs, attrs)
 
 
 def _get_operation_name(handler, request):
@@ -211,6 +222,7 @@ def _start_span(tracer, handler, start_time) -> _TraceContext:
             _get_header_from_request_headers, handler.request.headers,
         )
     )
+
     span = tracer.start_span(
         _get_operation_name(handler, handler.request),
         kind=trace.SpanKind.SERVER,
diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py
index d900b5d36..eb2852f11 100644
--- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py
+++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py
@@ -354,6 +354,21 @@ class TestTornadoInstrumentation(TornadoTest):
         test_excluded("/healthz")
         test_excluded("/ping")
 
+    @patch(
+        "opentelemetry.instrumentation.tornado._traced_attrs",
+        ["uri", "full_url", "query"],
+    )
+    def test_traced_attrs(self):
+        self.fetch("/ping?q=abc&b=123")
+        spans = self.sorted_spans(self.memory_exporter.get_finished_spans())
+        self.assertEqual(len(spans), 2)
+        server_span = spans[0]
+        self.assertEqual(server_span.kind, SpanKind.SERVER)
+        self.assert_span_has_attributes(
+            server_span, {"uri": "/ping?q=abc&b=123", "query": "q=abc&b=123"}
+        )
+        self.memory_exporter.clear()
+
 
 class TestTornadoUninstrument(TornadoTest):
     def test_uninstrument(self):