From ab16d1f1a60bcaeda95f5f97b78939f80dffac02 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 6 Nov 2020 01:35:17 +0530 Subject: [PATCH 1/6] Add custom span name hook for Flask --- .../instrumentation/flask/__init__.py | 83 ++++++++++--------- .../tests/test_programmatic.py | 25 ++++++ 2 files changed, 69 insertions(+), 39 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 1235b09a30..4c817ba0b9 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -20,7 +20,7 @@ in Flask applications. In addition to opentelemetry-instrumentation-wsgi, it supports flask-specific features such as: -* The Flask endpoint name is used as the Span name. +* The Flask url rule pattern is used as the Span name. * The ``http.route`` Span attribute is set so that one can see which URL rule matched a request. @@ -74,6 +74,13 @@ def get_excluded_urls(): _excluded_urls = get_excluded_urls() +def get_default_span_name(): + span_name = None + try: + span_name = flask.request.url_rule.rule + except AttributeError: + span_name = otel_wsgi.get_default_span_name(flask.request.environ) + return span_name def _rewrapped_app(wsgi_app): def _wrapped_app(environ, start_response): @@ -104,45 +111,39 @@ def _start_response(status, response_headers, *args, **kwargs): return _wrapped_app +def _wrapped_before_request(name_callback): + def _before_request(): + if _excluded_urls.url_disabled(flask.request.url): + return -def _before_request(): - if _excluded_urls.url_disabled(flask.request.url): - return + environ = flask.request.environ + span_name = name_callback() + token = context.attach( + propagators.extract(otel_wsgi.carrier_getter, environ) + ) - environ = flask.request.environ - span_name = None - try: - span_name = flask.request.url_rule.rule - except AttributeError: - pass - if span_name is None: - span_name = otel_wsgi.get_default_span_name(environ) - token = context.attach( - propagators.extract(otel_wsgi.carrier_getter, environ) - ) - - tracer = trace.get_tracer(__name__, __version__) - - span = tracer.start_span( - span_name, - kind=trace.SpanKind.SERVER, - start_time=environ.get(_ENVIRON_STARTTIME_KEY), - ) - if span.is_recording(): - attributes = otel_wsgi.collect_request_attributes(environ) - if flask.request.url_rule: - # For 404 that result from no route found, etc, we - # don't have a url_rule. - attributes["http.route"] = flask.request.url_rule.rule - for key, value in attributes.items(): - span.set_attribute(key, value) - - activation = tracer.use_span(span, end_on_exit=True) - activation.__enter__() - environ[_ENVIRON_ACTIVATION_KEY] = activation - environ[_ENVIRON_SPAN_KEY] = span - environ[_ENVIRON_TOKEN] = token + tracer = trace.get_tracer(__name__, __version__) + span = tracer.start_span( + span_name, + kind=trace.SpanKind.SERVER, + start_time=environ.get(_ENVIRON_STARTTIME_KEY), + ) + if span.is_recording(): + attributes = otel_wsgi.collect_request_attributes(environ) + if flask.request.url_rule: + # For 404 that result from no route found, etc, we + # don't have a url_rule. + attributes["http.route"] = flask.request.url_rule.rule + for key, value in attributes.items(): + span.set_attribute(key, value) + + activation = tracer.use_span(span, end_on_exit=True) + activation.__enter__() + environ[_ENVIRON_ACTIVATION_KEY] = activation + environ[_ENVIRON_SPAN_KEY] = span + environ[_ENVIRON_TOKEN] = token + return _before_request def _teardown_request(exc): if _excluded_urls.url_disabled(flask.request.url): @@ -173,6 +174,8 @@ def __init__(self, *args, **kwargs): self._original_wsgi_ = self.wsgi_app self.wsgi_app = _rewrapped_app(self.wsgi_app) + _before_request = _wrapped_before_request(get_default_span_name) + self._before_request = _before_request self.before_request(_before_request) self.teardown_request(_teardown_request) @@ -188,7 +191,7 @@ def _instrument(self, **kwargs): self._original_flask = flask.Flask flask.Flask = _InstrumentedFlask - def instrument_app(self, app): # pylint: disable=no-self-use + def instrument_app(self, app, name_callback=get_default_span_name): # pylint: disable=no-self-use if not hasattr(app, "_is_instrumented"): app._is_instrumented = False @@ -196,6 +199,8 @@ def instrument_app(self, app): # pylint: disable=no-self-use app._original_wsgi_app = app.wsgi_app app.wsgi_app = _rewrapped_app(app.wsgi_app) + _before_request = _wrapped_before_request(name_callback) + app._before_request = _before_request app.before_request(_before_request) app.teardown_request(_teardown_request) app._is_instrumented = True @@ -215,7 +220,7 @@ def uninstrument_app(self, app): # pylint: disable=no-self-use app.wsgi_app = app._original_wsgi_app # FIXME add support for other Flask blueprints that are not None - app.before_request_funcs[None].remove(_before_request) + app.before_request_funcs[None].remove(app._before_request) app.teardown_request_funcs[None].remove(_teardown_request) del app._original_wsgi_app diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index a907890523..478b7167ba 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -178,3 +178,28 @@ def test_exclude_lists(self): self.client.get("/excluded_noarg2") span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) + +class TestProgrammaticCustomSpanName(InstrumentationTest, TestBase, WsgiTestBase): + def setUp(self): + super().setUp() + + def custom_span_name(): + return "flask-custom-span-name" + + self.app = Flask(__name__) + + FlaskInstrumentor().instrument_app(self.app, name_callback=custom_span_name) + + self._common_initialization() + + def tearDown(self): + super().tearDown() + with self.disable_logging(): + FlaskInstrumentor().uninstrument_app(self.app) + + def test_custom_span_name(self): + self.client.get("/hello/123") + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual(span_list[0].name, "flask-custom-span-name") From 6cf6e77f76b6c86aa74b160dacae7d264338f213 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 9 Nov 2020 19:32:36 +0530 Subject: [PATCH 2/6] Fix lint issues --- .../src/opentelemetry/instrumentation/flask/__init__.py | 9 ++++++++- .../tests/test_programmatic.py | 9 +++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 4c817ba0b9..e673a3bc1e 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -74,6 +74,7 @@ def get_excluded_urls(): _excluded_urls = get_excluded_urls() + def get_default_span_name(): span_name = None try: @@ -82,6 +83,7 @@ def get_default_span_name(): span_name = otel_wsgi.get_default_span_name(flask.request.environ) return span_name + def _rewrapped_app(wsgi_app): def _wrapped_app(environ, start_response): # We want to measure the time for route matching, etc. @@ -111,6 +113,7 @@ def _start_response(status, response_headers, *args, **kwargs): return _wrapped_app + def _wrapped_before_request(name_callback): def _before_request(): if _excluded_urls.url_disabled(flask.request.url): @@ -143,8 +146,10 @@ def _before_request(): environ[_ENVIRON_ACTIVATION_KEY] = activation environ[_ENVIRON_SPAN_KEY] = span environ[_ENVIRON_TOKEN] = token + return _before_request + def _teardown_request(exc): if _excluded_urls.url_disabled(flask.request.url): return @@ -191,7 +196,9 @@ def _instrument(self, **kwargs): self._original_flask = flask.Flask flask.Flask = _InstrumentedFlask - def instrument_app(self, app, name_callback=get_default_span_name): # pylint: disable=no-self-use + def instrument_app( + self, app, name_callback=get_default_span_name + ): # pylint: disable=no-self-use if not hasattr(app, "_is_instrumented"): app._is_instrumented = False diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 478b7167ba..172a1d7447 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -179,7 +179,10 @@ def test_exclude_lists(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) -class TestProgrammaticCustomSpanName(InstrumentationTest, TestBase, WsgiTestBase): + +class TestProgrammaticCustomSpanName( + InstrumentationTest, TestBase, WsgiTestBase +): def setUp(self): super().setUp() @@ -188,7 +191,9 @@ def custom_span_name(): self.app = Flask(__name__) - FlaskInstrumentor().instrument_app(self.app, name_callback=custom_span_name) + FlaskInstrumentor().instrument_app( + self.app, name_callback=custom_span_name + ) self._common_initialization() From fd8699aa3e0a9258091be20575145343e79490a7 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 9 Nov 2020 20:15:45 +0530 Subject: [PATCH 3/6] Add CHANGELOG entry for custom span name --- .../opentelemetry-instrumentation-flask/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-flask/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-flask/CHANGELOG.md index 4c7e7a055b..3ff39fb72f 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-flask/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Add span name callback + ([#152](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/152)) + ## Version 0.15b0 Released 2020-11-02 From 45677150329291033e7bb943d65b5be7c2575d88 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 9 Nov 2020 20:16:15 +0530 Subject: [PATCH 4/6] Resolve review comment --- .../src/opentelemetry/instrumentation/flask/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index e673a3bc1e..ad0e9d9318 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -76,7 +76,7 @@ def get_excluded_urls(): def get_default_span_name(): - span_name = None + span_name = "" try: span_name = flask.request.url_rule.rule except AttributeError: From cee66ec04fd1aa67d454b3168a80a748b16f6ced Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 19:06:25 +0530 Subject: [PATCH 5/6] Add callback for direct instrument --- .../instrumentation/flask/__init__.py | 10 ++++++- .../tests/test_programmatic.py | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index ad0e9d9318..bfc1b3d798 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -173,13 +173,18 @@ def _teardown_request(exc): class _InstrumentedFlask(flask.Flask): + + name_callback = get_default_span_name + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._original_wsgi_ = self.wsgi_app self.wsgi_app = _rewrapped_app(self.wsgi_app) - _before_request = _wrapped_before_request(get_default_span_name) + _before_request = _wrapped_before_request( + _InstrumentedFlask.name_callback + ) self._before_request = _before_request self.before_request(_before_request) self.teardown_request(_teardown_request) @@ -194,6 +199,9 @@ class FlaskInstrumentor(BaseInstrumentor): def _instrument(self, **kwargs): self._original_flask = flask.Flask + name_callback = kwargs.get("name_callback") + if callable(name_callback): + _InstrumentedFlask.name_callback = name_callback flask.Flask = _InstrumentedFlask def instrument_app( diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 172a1d7447..c78feac7d0 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -208,3 +208,32 @@ def test_custom_span_name(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) self.assertEqual(span_list[0].name, "flask-custom-span-name") + + +class TestProgrammaticCustomSpanNameCallbackWithoutApp( + InstrumentationTest, TestBase, WsgiTestBase +): + def setUp(self): + super().setUp() + + def custom_span_name(): + return "instrument-without-app" + + FlaskInstrumentor().instrument(name_callback=custom_span_name) + from flask import Flask + + self.app = Flask(__name__) + + self._common_initialization() + + def tearDown(self): + super().tearDown() + with self.disable_logging(): + FlaskInstrumentor().uninstrument() + + def test_custom_span_name(self): + self.client.get("/hello/123") + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual(span_list[0].name, "instrument-without-app") From 9657e3809a4fe0a94f5662c13dc713da435b707b Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 19:33:28 +0530 Subject: [PATCH 6/6] Lint fix --- .../tests/test_programmatic.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index c78feac7d0..0bed5d20d8 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -220,6 +220,7 @@ def custom_span_name(): return "instrument-without-app" FlaskInstrumentor().instrument(name_callback=custom_span_name) + # pylint: disable=import-outside-toplevel,reimported,redefined-outer-name from flask import Flask self.app = Flask(__name__)