Skip to content

Commit

Permalink
Remove http.status_text from span attributes (open-telemetry#406)
Browse files Browse the repository at this point in the history
Update instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py
Update .github/workflows/test.yml

Co-authored-by: Aaron Abbott <aaronabbott@google.com>
  • Loading branch information
2 people authored and owais committed Apr 13, 2021
1 parent 1ee8924 commit 4014879
Show file tree
Hide file tree
Showing 21 changed files with 112 additions and 57 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,6 @@ jobs:
uses: actions/cache@v2
with:
path: .tox
key: tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt') }}
key: tox-cache-${{ matrix.tox-environment }}-${{ hashFiles('tox.ini', 'dev-requirements.txt', 'docs-requirements.txt') }}
- name: run tox
run: tox -e ${{ matrix.tox-environment }}
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#299](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/299))
- `opentelemetry-instrumenation-django` now supports request and response hooks.
([#407](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/407))
- `opentelemetry-instrumentation-falcon` FalconInstrumentor now supports request/response hooks.
([#415](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/415))

### Removed
- Remove `http.status_text` from span attributes
([#406](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/406))

## [0.19b0](https://github.com/open-telemetry/opentelemetry-python-contrib/releases/tag/v0.19b0) - 2021-03-26

Expand Down
1 change: 1 addition & 0 deletions docs-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ boto~=2.0
botocore~=1.0
celery>=4.0
flask~=1.0
falcon~=2.0
grpcio~=1.27
mysql-connector-python~=8.0
pymongo~=3.1
Expand Down
7 changes: 7 additions & 0 deletions docs/instrumentation/falcon/falcon.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
OpenTelemetry Falcon Instrumentation
====================================

.. automodule:: opentelemetry.instrumentation.falcon
:members:
:undoc-members:
:show-inheritance:
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,6 @@ async def on_request_end(
trace_config_ctx.span.set_attribute(
"http.status_code", params.response.status
)
trace_config_ctx.span.set_attribute(
"http.status_text", params.response.reason
)
_end_trace(trace_config_ctx)

async def on_request_exception(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ def test_status_codes(self):
host, port
),
"http.status_code": int(status_code),
"http.status_text": status_code.phrase,
},
)
]
Expand Down Expand Up @@ -191,7 +190,6 @@ def test_span_name_option(self):
host, port, path
),
"http.status_code": int(HTTPStatus.OK),
"http.status_text": HTTPStatus.OK.phrase,
},
)
]
Expand Down Expand Up @@ -222,7 +220,6 @@ def strip_query_params(url: yarl.URL) -> str:
host, port
),
"http.status_code": int(HTTPStatus.OK),
"http.status_text": HTTPStatus.OK.phrase,
},
)
]
Expand Down Expand Up @@ -361,7 +358,6 @@ def test_instrument(self):
span.attributes["http.url"],
)
self.assertEqual(200, span.attributes["http.status_code"])
self.assertEqual("OK", span.attributes["http.status_text"])

def test_instrument_with_existing_trace_config(self):
trace_config = aiohttp.TraceConfig()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ def test_templated_route_get(self):
)
self.assertEqual(span.attributes["http.scheme"], "http")
self.assertEqual(span.attributes["http.status_code"], 200)
self.assertEqual(span.attributes["http.status_text"], "OK")

def test_traced_get(self):
Client().get("/traced/")
Expand All @@ -143,7 +142,6 @@ def test_traced_get(self):
self.assertEqual(span.attributes["http.route"], "^traced/")
self.assertEqual(span.attributes["http.scheme"], "http")
self.assertEqual(span.attributes["http.status_code"], 200)
self.assertEqual(span.attributes["http.status_text"], "OK")

def test_not_recording(self):
mock_tracer = Mock()
Expand Down Expand Up @@ -178,7 +176,6 @@ def test_traced_post(self):
self.assertEqual(span.attributes["http.route"], "^traced/")
self.assertEqual(span.attributes["http.scheme"], "http")
self.assertEqual(span.attributes["http.status_code"], 200)
self.assertEqual(span.attributes["http.status_text"], "OK")

def test_error(self):
with self.assertRaises(ValueError):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,36 @@
* The ``falcon.resource`` Span attribute is set so the matched resource.
* Error from Falcon resources are properly caught and recorded.
Configuration
-------------
Exclude lists
*************
To exclude certain URLs from being tracked, set the environment variable ``OTEL_PYTHON_FALCON_EXCLUDED_URLS`` with comma delimited regexes representing which URLs to exclude.
For example,
::
export OTEL_PYTHON_FALCON_EXCLUDED_URLS="client/.*/info,healthcheck"
will exclude requests such as ``https://site/client/123/info`` and ``https://site/xyz/healthcheck``.
Request attributes
********************
To extract certain attributes from Falcon's request object and use them as span attributes, set the environment variable ``OTEL_PYTHON_FALCON_TRACED_REQUEST_ATTRS`` to a comma
delimited list of request attribute names.
For example,
::
export OTEL_PYTHON_FALCON_TRACED_REQUEST_ATTRS='query_string,uri_template'
will extract query_string and uri_template attributes from every traced request and add them as span attritbues.
Falcon Request object reference: https://falcon.readthedocs.io/en/stable/api/request_and_response.html#id1
Usage
-----
Expand All @@ -39,10 +69,27 @@ def on_get(self, req, resp):
app.add_route('/hello', HelloWorldResource())
Request and Response hooks
***************************
The instrumentation supports specifying request and response hooks. These are functions that get called back by the instrumentation right after a Span is created for a request
and right before the span is finished while processing a response. The hooks can be configured as follows:
::
def request_hook(span, req):
pass
def response_hook(span, req, resp):
pass
FalconInstrumentation().instrument(request_hook=request_hook, response_hook=response_hook)
API
---
"""

from functools import partial
from logging import getLogger
from sys import exc_info

Expand Down Expand Up @@ -83,21 +130,25 @@ class FalconInstrumentor(BaseInstrumentor):

def _instrument(self, **kwargs):
self._original_falcon_api = falcon.API
falcon.API = _InstrumentedFalconAPI
falcon.API = partial(_InstrumentedFalconAPI, **kwargs)

def _uninstrument(self, **kwargs):
falcon.API = self._original_falcon_api


class _InstrumentedFalconAPI(falcon.API):
def __init__(self, *args, **kwargs):
# inject trace middleware
middlewares = kwargs.pop("middleware", [])
if not isinstance(middlewares, (list, tuple)):
middlewares = [middlewares]

self._tracer = trace.get_tracer(__name__, __version__)
trace_middleware = _TraceMiddleware(
self._tracer, kwargs.get("traced_request_attributes")
self._tracer,
kwargs.pop("traced_request_attributes", None),
kwargs.pop("request_hook", None),
kwargs.pop("response_hook", None),
)
middlewares.insert(0, trace_middleware)
kwargs["middleware"] = middlewares
Expand Down Expand Up @@ -148,12 +199,23 @@ def _start_response(status, response_headers, *args, **kwargs):
class _TraceMiddleware:
# pylint:disable=R0201,W0613

def __init__(self, tracer=None, traced_request_attrs=None):
def __init__(
self,
tracer=None,
traced_request_attrs=None,
request_hook=None,
response_hook=None,
):
self.tracer = tracer
self._traced_request_attrs = _traced_request_attrs
self._request_hook = request_hook
self._response_hook = response_hook

def process_request(self, req, resp):
span = req.env.get(_ENVIRON_SPAN_KEY)
if span and self._request_hook:
self._request_hook(span, req)

if not span or not span.is_recording():
return

Expand All @@ -178,6 +240,7 @@ def process_response(
self, req, resp, resource, req_succeeded=None
): # pylint:disable=R0201
span = req.env.get(_ENVIRON_SPAN_KEY)

if not span or not span.is_recording():
return

Expand Down Expand Up @@ -209,3 +272,6 @@ def process_response(
description=reason,
)
)

if self._response_hook:
self._response_hook(span, req, resp)
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@
from .app import make_app


class TestFalconInstrumentation(TestBase):
class TestFalconBase(TestBase):
def setUp(self):
super().setUp()
FalconInstrumentor().instrument()
FalconInstrumentor().instrument(
request_hook=getattr(self, "request_hook", None),
response_hook=getattr(self, "response_hook", None),
)
self.app = make_app()
# pylint: disable=protected-access
self.env_patch = patch.dict(
Expand Down Expand Up @@ -64,6 +67,8 @@ def tearDown(self):
self.exclude_patch.stop()
self.traced_patch.stop()


class TestFalconInstrumentation(TestFalconBase):
def test_get(self):
self._test_method("GET")

Expand Down Expand Up @@ -104,7 +109,6 @@ def _test_method(self, method):
"net.peer.port": "65133",
"http.flavor": "1.1",
"falcon.resource": "HelloWorldResource",
"http.status_text": "Created",
"http.status_code": 201,
},
)
Expand All @@ -129,7 +133,6 @@ def test_404(self):
"net.peer.ip": "127.0.0.1",
"net.peer.port": "65133",
"http.flavor": "1.1",
"http.status_text": "Not Found",
"http.status_code": 404,
},
)
Expand Down Expand Up @@ -206,3 +209,22 @@ def test_traced_not_recording(self):
self.assertTrue(mock_span.is_recording.called)
self.assertFalse(mock_span.set_attribute.called)
self.assertFalse(mock_span.set_status.called)


class TestFalconInstrumentationHooks(TestFalconBase):
# pylint: disable=no-self-use
def request_hook(self, span, req):
span.set_attribute("request_hook_attr", "value from hook")

def response_hook(self, span, req, resp):
span.update_name("set from hook")

def test_hooks(self):
self.client().simulate_get(path="/hello?q=abc")
span = self.memory_exporter.get_finished_spans()[0]

self.assertEqual(span.name, "set from hook")
self.assertIn("request_hook_attr", span.attributes)
self.assertEqual(
span.attributes["request_hook_attr"], "value from hook"
)
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def expected_attributes(override_attributes):
"http.host": "localhost",
"http.target": "/",
"http.flavor": "1.1",
"http.status_text": "OK",
"http.status_code": 200,
}
for key, val in override_attributes.items():
Expand Down Expand Up @@ -138,7 +137,6 @@ def test_404(self):
{
"http.method": "POST",
"http.target": "/bye",
"http.status_text": "NOT FOUND",
"http.status_code": 404,
}
)
Expand All @@ -157,7 +155,6 @@ def test_internal_error(self):
{
"http.target": "/hello/500",
"http.route": "/hello/<int:helloid>",
"http.status_text": "INTERNAL SERVER ERROR",
"http.status_code": 500,
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def expected_attributes(override_attributes):
"http.host": "localhost",
"http.target": "/",
"http.flavor": "1.1",
"http.status_text": "OK",
"http.status_code": 200,
}
for key, val in override_attributes.items():
Expand Down Expand Up @@ -118,7 +117,6 @@ def test_404(self):
{
"http.method": "POST",
"http.target": "/bye",
"http.status_text": "Not Found",
"http.status_code": 404,
}
)
Expand All @@ -137,7 +135,6 @@ def test_internal_error(self):
{
"http.target": "/hello/500",
"http.route": "/hello/{helloid}",
"http.status_text": "Internal Server Error",
"http.status_code": 500,
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ def _instrumented_requests_call(
if isinstance(result, Response):
if span.is_recording():
span.set_attribute("http.status_code", result.status_code)
span.set_attribute("http.status_text", result.reason)
span.set_status(
Status(http_status_to_status_code(result.status_code))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ def test_basic(self):
"http.method": "GET",
"http.url": self.URL,
"http.status_code": 200,
"http.status_text": "OK",
},
)

Expand Down Expand Up @@ -127,7 +126,6 @@ def test_not_foundbasic(self):
span = self.assert_span()

self.assertEqual(span.attributes.get("http.status_code"), 404)
self.assertEqual(span.attributes.get("http.status_text"), "Not Found")

self.assertIs(
span.status.status_code, trace.StatusCode.ERROR,
Expand Down Expand Up @@ -235,7 +233,6 @@ def span_callback(span, result: requests.Response):
"http.method": "GET",
"http.url": self.URL,
"http.status_code": 200,
"http.status_text": "OK",
"http.response.body": "Hello!",
},
)
Expand Down Expand Up @@ -304,7 +301,6 @@ def test_requests_exception_with_response(self, *_, **__):
"http.method": "GET",
"http.url": self.URL,
"http.status_code": 500,
"http.status_text": "Internal Server Error",
},
)
self.assertEqual(span.status.status_code, StatusCode.ERROR)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ def _finish_span(tracer, handler, error=None):
return

if ctx.span.is_recording():
if reason:
ctx.span.set_attribute("http.status_text", reason)
ctx.span.set_attribute("http.status_code", status_code)
ctx.span.set_status(
Status(
Expand Down
Loading

0 comments on commit 4014879

Please sign in to comment.