From 21bdf4c6c35c70a8763428462663be6ac54801e3 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 00:43:55 -0500 Subject: [PATCH 01/22] Enable compatibility w/ Falcon 3 --- .../opentelemetry/instrumentation/falcon/__init__.py | 11 +++++++++++ .../opentelemetry/instrumentation/falcon/package.py | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 8d05ea73b3..9d6064fc71 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -140,11 +140,22 @@ def instrumentation_dependencies(self) -> Collection[str]: return _instruments def _instrument(self, **kwargs): + self._original_falcon_api = None + self._original_falcon_app = None + if hasattr(falcon, "App"): + # Falcon 3 + falcon.App = partial( + _InstrumentedFalconAPI, **kwargs + ) + self._original_falcon_api = falcon.API + return + # Falcon 2 self._original_falcon_api = falcon.API falcon.API = partial(_InstrumentedFalconAPI, **kwargs) def _uninstrument(self, **kwargs): falcon.API = self._original_falcon_api + falcon.App = self._original_falcon_app class _InstrumentedFalconAPI(falcon.API): diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/package.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/package.py index 7e554264ff..9cdd0f17cd 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/package.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/package.py @@ -13,4 +13,4 @@ # limitations under the License. -_instruments = ("falcon ~= 2.0",) +_instruments = ("falcon >= 2.0.0, < 4.0.0",) From beb8bc5fe47b235a2d3a5602a5b175097fa13f0e Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 00:46:46 -0500 Subject: [PATCH 02/22] add Falcon 3 to tox.ini --- .../opentelemetry-instrumentation-falcon/README.rst | 2 -- tox.ini | 8 +++++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/README.rst b/instrumentation/opentelemetry-instrumentation-falcon/README.rst index 24cb1cf45a..c192a2ae78 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/README.rst +++ b/instrumentation/opentelemetry-instrumentation-falcon/README.rst @@ -9,8 +9,6 @@ OpenTelemetry Falcon Tracing This library builds on the OpenTelemetry WSGI middleware to track web requests in Falcon applications. -Currently, only Falcon v2 is supported. - Installation ------------ diff --git a/tox.ini b/tox.ini index 70a8968607..b49da51f46 100644 --- a/tox.ini +++ b/tox.ini @@ -38,8 +38,8 @@ envlist = pypy3-test-instrumentation-elasticsearch{2,5,6} ; opentelemetry-instrumentation-falcon - py3{4,5,6,7,8,9}-test-instrumentation-falcon - pypy3-test-instrumentation-falcon + py3{4,5,6,7,8,9}-test-instrumentation-falcon{2,3} + pypy3-test-instrumentation-falcon{2,3} ; opentelemetry-instrumentation-fastapi ; fastapi only supports 3.6 and above. @@ -174,6 +174,8 @@ deps = ; FIXME: Elasticsearch >=7 causes CI workflow tests to hang, see open-telemetry/opentelemetry-python-contrib#620 ; elasticsearch7: elasticsearch-dsl>=7.0,<8.0 ; elasticsearch7: elasticsearch>=7.0,<8.0 + falcon2: falcon >=2.0.0,<=3.0.0 + falcon2: falcon >=3.0.0,<=4.0.0 sqlalchemy11: sqlalchemy>=1.1,<1.2 sqlalchemy14: aiosqlite sqlalchemy14: sqlalchemy~=1.4 @@ -192,7 +194,7 @@ changedir = test-instrumentation-dbapi: instrumentation/opentelemetry-instrumentation-dbapi/tests test-instrumentation-django: instrumentation/opentelemetry-instrumentation-django/tests test-instrumentation-elasticsearch{2,5,6}: instrumentation/opentelemetry-instrumentation-elasticsearch/tests - test-instrumentation-falcon: instrumentation/opentelemetry-instrumentation-falcon/tests + test-instrumentation-falcon{2,3}: instrumentation/opentelemetry-instrumentation-falcon/tests test-instrumentation-fastapi: instrumentation/opentelemetry-instrumentation-fastapi/tests test-instrumentation-flask: instrumentation/opentelemetry-instrumentation-flask/tests test-instrumentation-urllib: instrumentation/opentelemetry-instrumentation-urllib/tests From b630585da82fd80ff6db3362b9b135710bb848b0 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 00:49:12 -0500 Subject: [PATCH 03/22] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffe410a6a1..be847ce4fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.5.0-0.24b0...HEAD) +### Changed +- Tests for Falcon 3 support + ([#644](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/644)) + ## [1.5.0-0.24b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.5.0-0.24b0) - 2021-08-26 ### Added From 4e00693ea4ca817b8f9a0d80efb2a3132baa95d0 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 00:56:41 -0500 Subject: [PATCH 04/22] run generate because why not make this more complicated and convoluted to contribute to --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 9d6064fc71..994e26a210 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -144,9 +144,7 @@ def _instrument(self, **kwargs): self._original_falcon_app = None if hasattr(falcon, "App"): # Falcon 3 - falcon.App = partial( - _InstrumentedFalconAPI, **kwargs - ) + falcon.App = partial(_InstrumentedFalconAPI, **kwargs) self._original_falcon_api = falcon.API return # Falcon 2 From af1075f2001046661b4c90488902219efbe36b17 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 00:58:01 -0500 Subject: [PATCH 05/22] manually apply formatting because everything has to be broken --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 994e26a210..9d6064fc71 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -144,7 +144,9 @@ def _instrument(self, **kwargs): self._original_falcon_app = None if hasattr(falcon, "App"): # Falcon 3 - falcon.App = partial(_InstrumentedFalconAPI, **kwargs) + falcon.App = partial( + _InstrumentedFalconAPI, **kwargs + ) self._original_falcon_api = falcon.API return # Falcon 2 From b2a09fe73128febfaeb123bf32471c1dece4fb36 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 09:46:21 -0500 Subject: [PATCH 06/22] use single attribute --- .../instrumentation/falcon/__init__.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 9d6064fc71..03622471f4 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -141,22 +141,24 @@ def instrumentation_dependencies(self) -> Collection[str]: def _instrument(self, **kwargs): self._original_falcon_api = None - self._original_falcon_app = None if hasattr(falcon, "App"): # Falcon 3 falcon.App = partial( _InstrumentedFalconAPI, **kwargs ) self._original_falcon_api = falcon.API - return - # Falcon 2 - self._original_falcon_api = falcon.API - falcon.API = partial(_InstrumentedFalconAPI, **kwargs) + else: + # Falcon 2 + self._original_falcon_api = falcon.API + falcon.API = partial(_InstrumentedFalconAPI, **kwargs) def _uninstrument(self, **kwargs): - falcon.API = self._original_falcon_api - falcon.App = self._original_falcon_app - + if hasattr(falcon, "App"): + # Falcon 3 + falcon.App = self._original_falcon_api + else: + # Falcon 2 + falcon.API = self._original_falcon_api class _InstrumentedFalconAPI(falcon.API): def __init__(self, *args, **kwargs): From 9ad8d43e501cbd0d83f31bba3ceaf37be717828b Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 10:06:39 -0500 Subject: [PATCH 07/22] update installs: --- tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index b49da51f46..96cd14bf16 100644 --- a/tox.ini +++ b/tox.ini @@ -238,7 +238,7 @@ commands_pre = grpc: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc[test] falcon,flask,django,pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test] - wsgi,falcon,flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] + wsgi,falcon{2,3},flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] asgi,starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] asyncpg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asyncpg[test] @@ -246,7 +246,7 @@ commands_pre = boto: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-botocore[test] boto: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-boto[test] - falcon: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-falcon[test] + falcon{2,3}: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-falcon[test] flask: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-flask[test] From 189f6f55a7033fd7f7cfb874c03108effe6bee01 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 10:14:15 -0500 Subject: [PATCH 08/22] Update tox.ini Co-authored-by: Owais Lone --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 96cd14bf16..167ce3de50 100644 --- a/tox.ini +++ b/tox.ini @@ -175,7 +175,7 @@ deps = ; elasticsearch7: elasticsearch-dsl>=7.0,<8.0 ; elasticsearch7: elasticsearch>=7.0,<8.0 falcon2: falcon >=2.0.0,<=3.0.0 - falcon2: falcon >=3.0.0,<=4.0.0 + falcon3: falcon >=3.0.0,<=4.0.0 sqlalchemy11: sqlalchemy>=1.1,<1.2 sqlalchemy14: aiosqlite sqlalchemy14: sqlalchemy~=1.4 From 34887a4719bf29e7025d7b27227fe79cf183204d Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 10:18:37 -0500 Subject: [PATCH 09/22] Update tox.ini Co-authored-by: Owais Lone --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 167ce3de50..e3f5f19af0 100644 --- a/tox.ini +++ b/tox.ini @@ -174,7 +174,7 @@ deps = ; FIXME: Elasticsearch >=7 causes CI workflow tests to hang, see open-telemetry/opentelemetry-python-contrib#620 ; elasticsearch7: elasticsearch-dsl>=7.0,<8.0 ; elasticsearch7: elasticsearch>=7.0,<8.0 - falcon2: falcon >=2.0.0,<=3.0.0 + falcon2: falcon >=2.0.0,<3.0.0 falcon3: falcon >=3.0.0,<=4.0.0 sqlalchemy11: sqlalchemy>=1.1,<1.2 sqlalchemy14: aiosqlite From 7e76de3baf45b9f61267248144d107413193c47c Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 10:18:42 -0500 Subject: [PATCH 10/22] Update tox.ini Co-authored-by: Owais Lone --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index e3f5f19af0..8c8859dbe6 100644 --- a/tox.ini +++ b/tox.ini @@ -175,7 +175,7 @@ deps = ; elasticsearch7: elasticsearch-dsl>=7.0,<8.0 ; elasticsearch7: elasticsearch>=7.0,<8.0 falcon2: falcon >=2.0.0,<3.0.0 - falcon3: falcon >=3.0.0,<=4.0.0 + falcon3: falcon >=3.0.0,<4.0.0 sqlalchemy11: sqlalchemy>=1.1,<1.2 sqlalchemy14: aiosqlite sqlalchemy14: sqlalchemy~=1.4 From a7e82d5fe5c82703022a778525d99b80c0eded79 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 1 Sep 2021 10:31:37 -0500 Subject: [PATCH 11/22] run black manually --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 03622471f4..fea460e09a 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -143,9 +143,7 @@ def _instrument(self, **kwargs): self._original_falcon_api = None if hasattr(falcon, "App"): # Falcon 3 - falcon.App = partial( - _InstrumentedFalconAPI, **kwargs - ) + falcon.App = partial(_InstrumentedFalconAPI, **kwargs) self._original_falcon_api = falcon.API else: # Falcon 2 @@ -160,6 +158,7 @@ def _uninstrument(self, **kwargs): # Falcon 2 falcon.API = self._original_falcon_api + class _InstrumentedFalconAPI(falcon.API): def __init__(self, *args, **kwargs): # inject trace middleware From afac5c9a4f5c22f840424e14c4e4bf630a82fbb9 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 2 Sep 2021 10:45:34 -0500 Subject: [PATCH 12/22] Use falcon.App for Falcon 3, inject remote_adr since Falcon 3 does not auto-inject it by default --- .../opentelemetry-instrumentation-falcon/tests/app.py | 7 ++++++- .../tests/test_falcon.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py index dcbfe11b49..839c535fc8 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py @@ -34,7 +34,12 @@ def on_get(self, req, resp): def make_app(): - app = falcon.API() + if hasattr(falcon, "App"): + # Falcon 3 + app = falcon.App() + else: + # Falcon 2 + app = falcon.API() app.add_route("/hello", HelloWorldResource()) app.add_route("/ping", HelloWorldResource()) app.add_route("/error", ErrorResource()) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 4ffa4ee0bd..45c24bb2df 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -78,7 +78,7 @@ def test_head(self): self._test_method("HEAD") def _test_method(self, method): - self.client().simulate_request(method=method, path="/hello") + self.client().simulate_request(method=method, path="/hello", remote_addr="127.0.0.1") spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] From 139ccbd153f2dae7349164528e9957fafbde696c Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 2 Sep 2021 11:35:12 -0500 Subject: [PATCH 13/22] Just missing exception info now --- .../opentelemetry-instrumentation-falcon/tests/test_falcon.py | 4 ++-- pytest.ini | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 45c24bb2df..c922b8962d 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -105,7 +105,7 @@ def _test_method(self, method): self.memory_exporter.clear() def test_404(self): - self.client().simulate_get("/does-not-exist") + self.client().simulate_get("/does-not-exist", remote_addr="127.0.0.1") spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] @@ -129,7 +129,7 @@ def test_404(self): def test_500(self): try: - self.client().simulate_get("/error") + self.client().simulate_get("/error", remote_addr="127.0.0.1") except NameError: pass spans = self.memory_exporter.get_finished_spans() diff --git a/pytest.ini b/pytest.ini index 013d2c555c..136015a6b0 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,3 +2,4 @@ addopts = -rs -v log_cli = true log_cli_level = warning +testpaths = instrumentation/opentelemetry-instrumentation-falcon/tests/ From cc9927705b11d4ea7ae393ca26c7fdbccdbed14b Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 10:41:48 -0500 Subject: [PATCH 14/22] Get exc_info from _handle_exception --- .../instrumentation/falcon/__init__.py | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 463a116167..21f26dc50a 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -126,6 +126,13 @@ def response_hook(span, req, resp): _response_propagation_setter = FuncSetter(falcon.Response.append_header) +if hasattr(falcon, "App"): + # Falcon 3 + _instrument_app = "App" +else: + # Falcon 2 + _instrument_app = "API" + class FalconInstrumentor(BaseInstrumentor): # pylint: disable=protected-access,attribute-defined-outside-init @@ -138,15 +145,8 @@ def instrumentation_dependencies(self) -> Collection[str]: return _instruments def _instrument(self, **kwargs): - self._original_falcon_api = None - if hasattr(falcon, "App"): - # Falcon 3 - falcon.App = partial(_InstrumentedFalconAPI, **kwargs) - self._original_falcon_api = falcon.API - else: - # Falcon 2 - self._original_falcon_api = falcon.API - falcon.API = partial(_InstrumentedFalconAPI, **kwargs) + self._original_falcon_api = getattr(falcon, _instrument_app) + setattr(falcon, _instrument_app, partial(_InstrumentedFalconAPI, **kwargs)) def _uninstrument(self, **kwargs): if hasattr(falcon, "App"): @@ -157,7 +157,7 @@ def _uninstrument(self, **kwargs): falcon.API = self._original_falcon_api -class _InstrumentedFalconAPI(falcon.API): +class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): def __init__(self, *args, **kwargs): # inject trace middleware middlewares = kwargs.pop("middleware", []) @@ -181,6 +181,12 @@ def __init__(self, *args, **kwargs): self._excluded_urls = get_excluded_urls("FALCON") super().__init__(*args, **kwargs) + def _handle_exception(self, req, resp, ex, params): + _, exc, _ = exc_info() + req.env[_ENVIRON_EXC] = exc + return super()._handle_exception(req, resp, ex, params) + + def __call__(self, env, start_response): # pylint: disable=E1101 if self._excluded_urls.url_disabled(env.get("PATH_INFO", "/")): @@ -277,7 +283,11 @@ def process_response( status = "404" reason = "NotFound" - exc_type, exc, _ = exc_info() + if _ENVIRON_EXC in req.env: + exc = req.env[_ENVIRON_EXC] + exc_type = type(exc) + else: + exc_type, exc, _ = exc_info() if exc_type and not req_succeeded: if "HTTPNotFound" in exc_type.__name__: status = "404" @@ -289,9 +299,6 @@ def process_response( status = status.split(" ")[0] try: status_code = int(status) - except ValueError: - pass - finally: span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) span.set_status( Status( @@ -299,6 +306,8 @@ def process_response( description=reason, ) ) + except ValueError: + pass propagator = get_global_response_propagator() if propagator: From b6ef52d4a0f0496aceeaae979f482e2b2dec3281 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 10:44:31 -0500 Subject: [PATCH 15/22] run tox generate --- instrumentation/README.md | 2 +- .../opentelemetry/instrumentation/falcon/__init__.py | 12 ++++-------- .../tests/test_falcon.py | 4 +++- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/instrumentation/README.md b/instrumentation/README.md index 8c59661d55..fba704c81d 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -11,7 +11,7 @@ | [opentelemetry-instrumentation-dbapi](./opentelemetry-instrumentation-dbapi) | dbapi | | [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 2.0 | -| [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon ~= 2.0 | +| [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 2.0.0, < 4.0.0 | | [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | | [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0, < 3.0 | | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 21f26dc50a..da7c365ce8 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -146,15 +146,12 @@ def instrumentation_dependencies(self) -> Collection[str]: def _instrument(self, **kwargs): self._original_falcon_api = getattr(falcon, _instrument_app) - setattr(falcon, _instrument_app, partial(_InstrumentedFalconAPI, **kwargs)) + setattr( + falcon, _instrument_app, partial(_InstrumentedFalconAPI, **kwargs) + ) def _uninstrument(self, **kwargs): - if hasattr(falcon, "App"): - # Falcon 3 - falcon.App = self._original_falcon_api - else: - # Falcon 2 - falcon.API = self._original_falcon_api + setattr(falcon, _instrument_app, self._original_falcon_api) class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): @@ -186,7 +183,6 @@ def _handle_exception(self, req, resp, ex, params): req.env[_ENVIRON_EXC] = exc return super()._handle_exception(req, resp, ex, params) - def __call__(self, env, start_response): # pylint: disable=E1101 if self._excluded_urls.url_disabled(env.get("PATH_INFO", "/")): diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index c922b8962d..5c4f7c576c 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -78,7 +78,9 @@ def test_head(self): self._test_method("HEAD") def _test_method(self, method): - self.client().simulate_request(method=method, path="/hello", remote_addr="127.0.0.1") + self.client().simulate_request( + method=method, path="/hello", remote_addr="127.0.0.1" + ) spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] From 5e14752038b4643f926beede93023a0226c3650e Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 10:51:08 -0500 Subject: [PATCH 16/22] ignore pylint error --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index da7c365ce8..fab4c46842 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -178,7 +178,7 @@ def __init__(self, *args, **kwargs): self._excluded_urls = get_excluded_urls("FALCON") super().__init__(*args, **kwargs) - def _handle_exception(self, req, resp, ex, params): + def _handle_exception(self, req, resp, ex, params): # pylint: disable=C0103 _, exc, _ = exc_info() req.env[_ENVIRON_EXC] = exc return super()._handle_exception(req, resp, ex, params) From 16f91cf3b2fb4c757d7bcf3c99656466c02c9817 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 10:57:38 -0500 Subject: [PATCH 17/22] Use the same method everywhere --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index fab4c46842..43cfe9f698 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -179,6 +179,8 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def _handle_exception(self, req, resp, ex, params): # pylint: disable=C0103 + # Falcon 3 does not execute middleware within the context of the exception + # so we capture the exception here and save it into the env dict _, exc, _ = exc_info() req.env[_ENVIRON_EXC] = exc return super()._handle_exception(req, resp, ex, params) @@ -279,11 +281,8 @@ def process_response( status = "404" reason = "NotFound" - if _ENVIRON_EXC in req.env: - exc = req.env[_ENVIRON_EXC] - exc_type = type(exc) - else: - exc_type, exc, _ = exc_info() + exc = req.env[_ENVIRON_EXC] + exc_type = type(exc) if exc_type and not req_succeeded: if "HTTPNotFound" in exc_type.__name__: status = "404" From 89d1c23455226d43e2a8475e047be6c7f0986e22 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 10:59:17 -0500 Subject: [PATCH 18/22] check for exc --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 43cfe9f698..d2dc7c4a3f 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -281,8 +281,11 @@ def process_response( status = "404" reason = "NotFound" - exc = req.env[_ENVIRON_EXC] - exc_type = type(exc) + if _ENVIRON_EXC in req.env: + exc = req.env[_ENVIRON_EXC] + exc_type = type(exc) + else: + exc_type, exc = None, None if exc_type and not req_succeeded: if "HTTPNotFound" in exc_type.__name__: status = "404" From 9b2e8c5134439b87f512fe11c6c89bde238fb77a Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 11:29:36 -0500 Subject: [PATCH 19/22] Now always add attributes in middleware --- .../instrumentation/falcon/__init__.py | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index d2dc7c4a3f..2e447507ed 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -209,7 +209,6 @@ def __call__(self, env, start_response): env[_ENVIRON_ACTIVATION_KEY] = activation def _start_response(status, response_headers, *args, **kwargs): - otel_wsgi.add_response_attributes(span, status, response_headers) response = start_response( status, response_headers, *args, **kwargs ) @@ -280,19 +279,19 @@ def process_response( if resource is None: status = "404" reason = "NotFound" - - if _ENVIRON_EXC in req.env: - exc = req.env[_ENVIRON_EXC] - exc_type = type(exc) else: - exc_type, exc = None, None - if exc_type and not req_succeeded: - if "HTTPNotFound" in exc_type.__name__: - status = "404" - reason = "NotFound" + if _ENVIRON_EXC in req.env: + exc = req.env[_ENVIRON_EXC] + exc_type = type(exc) else: - status = "500" - reason = "{}: {}".format(exc_type.__name__, exc) + exc_type, exc = None, None + if exc_type and not req_succeeded: + if "HTTPNotFound" in exc_type.__name__: + status = "404" + reason = "NotFound" + else: + status = "500" + reason = "{}: {}".format(exc_type.__name__, exc) status = status.split(" ")[0] try: From 26f5d32a6ed22eb1b9b60c61d35752b8fff8442b Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 11:31:49 -0500 Subject: [PATCH 20/22] update generated code --- .../src/opentelemetry/instrumentation/falcon/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 2e447507ed..dc39e03bf3 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -178,7 +178,9 @@ def __init__(self, *args, **kwargs): self._excluded_urls = get_excluded_urls("FALCON") super().__init__(*args, **kwargs) - def _handle_exception(self, req, resp, ex, params): # pylint: disable=C0103 + def _handle_exception( + self, req, resp, ex, params + ): # pylint: disable=C0103 # Falcon 3 does not execute middleware within the context of the exception # so we capture the exception here and save it into the env dict _, exc, _ = exc_info() From 8eda7b6916989b74d03c9d3f506532e8b833e614 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 12:59:53 -0500 Subject: [PATCH 21/22] add status.description assertions --- .../tests/test_falcon.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 5c4f7c576c..d0b2b09d6f 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -88,6 +88,10 @@ def _test_method(self, method): span.name, "HelloWorldResource.on_{0}".format(method.lower()) ) self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual( + span.status.description, + None, + ) self.assertSpanHasAttributes( span, { @@ -113,6 +117,10 @@ def test_404(self): span = spans[0] self.assertEqual(span.name, "HTTP GET") self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual( + span.status.description, + "NotFound", + ) self.assertSpanHasAttributes( span, { From f3ce4f97a4c2df83e2a5e23d50c8684e6eff4925 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Thu, 16 Sep 2021 13:16:30 -0500 Subject: [PATCH 22/22] tox -e generate again --- .../tests/test_falcon.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index d0b2b09d6f..3f9c2975de 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -89,8 +89,7 @@ def _test_method(self, method): ) self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertEqual( - span.status.description, - None, + span.status.description, None, ) self.assertSpanHasAttributes( span, @@ -118,8 +117,7 @@ def test_404(self): self.assertEqual(span.name, "HTTP GET") self.assertEqual(span.status.status_code, StatusCode.ERROR) self.assertEqual( - span.status.description, - "NotFound", + span.status.description, "NotFound", ) self.assertSpanHasAttributes( span,