From ab16d1f1a60bcaeda95f5f97b78939f80dffac02 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 6 Nov 2020 01:35:17 +0530 Subject: [PATCH 01/12] Add custom span name hook for Flask --- .../instrumentation/flask/__init__.py | 81 ++++++++++--------- .../tests/test_programmatic.py | 25 ++++++ 2 files changed, 68 insertions(+), 38 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 1235b09a3..4c817ba0b 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 @@ This library builds on the OpenTelemetry WSGI middleware to track web requests 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 _rewrapped_app(wsgi_app): 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__) - 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 + 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 @@ class _InstrumentedFlask(flask.Flask): 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 @@ class FlaskInstrumentor(BaseInstrumentor): 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 @@ class FlaskInstrumentor(BaseInstrumentor): 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 @@ class FlaskInstrumentor(BaseInstrumentor): 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 a90789052..478b7167b 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -178,3 +178,28 @@ class TestProgrammatic(InstrumentationTest, TestBase, WsgiTestBase): 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 02/12] 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 4c817ba0b..e673a3bc1 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 _rewrapped_app(wsgi_app): 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 _wrapped_before_request(name_callback): 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 @@ class FlaskInstrumentor(BaseInstrumentor): 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 478b7167b..172a1d744 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -179,7 +179,10 @@ class TestProgrammatic(InstrumentationTest, TestBase, WsgiTestBase): 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 @@ class TestProgrammaticCustomSpanName(InstrumentationTest, TestBase, WsgiTestBase 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 03/12] 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 4c7e7a055..3ff39fb72 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 04/12] 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 e673a3bc1..ad0e9d931 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 @@ _excluded_urls = get_excluded_urls() def get_default_span_name(): - span_name = None + span_name = "" try: span_name = flask.request.url_rule.rule except AttributeError: From a10e2c68d1a39e1f046fd1e9a70fe847c8795a74 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 10:00:54 -0500 Subject: [PATCH 05/12] requests --- .../CHANGELOG.md | 3 +++ .../instrumentation/requests/__init__.py | 14 ++++++++++-- .../tests/test_requests_integration.py | 22 +++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md index 00d730f4f..491d5a95f 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Add span name callback + ([#157](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/157)) + ## Version 0.15b0 Released 2020-11-02 diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index b4738647d..60e2d8451 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -64,7 +64,7 @@ _SUPPRESS_REQUESTS_INSTRUMENTATION_KEY = "suppress_requests_instrumentation" # pylint: disable=unused-argument # pylint: disable=R0915 -def _instrument(tracer_provider=None, span_callback=None): +def _instrument(tracer_provider=None, span_callback=None, name_callback=get_default_span_name): """Enables tracing of all requests calls that go through :code:`requests.session.Session.request` (this includes :code:`requests.get`, etc.).""" @@ -124,7 +124,7 @@ def _instrument(tracer_provider=None, span_callback=None): # See # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#http-client method = method.upper() - span_name = "HTTP {}".format(method) + span_name = name_callback(method, url) recorder = RequestsInstrumentor().metric_recorder @@ -217,6 +217,12 @@ def _uninstrument_from(instr_root, restore_as_bound_func=False): setattr(instr_root, instr_func_name, original) +# pylint: disable=unused-argument +def get_default_span_name(method_name, url): + """Default implementation for name_callback, returns HTTP {method_name}.""" + return "HTTP {}".format(method_name).strip() + + class RequestsInstrumentor(BaseInstrumentor, MetricMixin): """An instrumentor for requests See `BaseInstrumentor` @@ -229,10 +235,14 @@ class RequestsInstrumentor(BaseInstrumentor, MetricMixin): **kwargs: Optional arguments ``tracer_provider``: a TracerProvider, defaults to global ``span_callback``: An optional callback invoked before returning the http response. Invoked with Span and requests.Response + ``name_callback``: Callback which calculates a generic span name for an + outgoing HTTP request based on the method and url. + Optional: Defaults to get_default_span_name. """ _instrument( tracer_provider=kwargs.get("tracer_provider"), span_callback=kwargs.get("span_callback"), + name_callback=kwargs.get("name_callback"), ) self.init_metrics( __name__, __version__, diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index f5209108e..81f21113e 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -107,6 +107,28 @@ class RequestsIntegrationTestBase(abc.ABC): self.assertEqual(view_data.aggregator.current.count, 1) self.assertGreaterEqual(view_data.aggregator.current.sum, 0) + def test_name_callback(self): + def name_callback(): + return "test_name" + RequestsInstrumentor().uninstrument() + RequestsInstrumentor().instrument(name_callback=name_callback) + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + span = self.assert_span() + + self.assertEqual(span.name, "test_name") + + def test_name_callback_default(self): + def name_callback(): + return 123 + RequestsInstrumentor().uninstrument() + RequestsInstrumentor().instrument(name_callback=name_callback) + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + span = self.assert_span() + + self.assertEqual(span.name, "HTTP GET") + def test_not_foundbasic(self): url_404 = "http://httpbin.org/status/404" httpretty.register_uri( From dad94d434d691581a6d942c56a1ac58ba77ee3c5 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 10:14:44 -0500 Subject: [PATCH 06/12] build --- .../instrumentation/requests/__init__.py | 13 ++++++++----- .../tests/test_requests_integration.py | 2 ++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 60e2d8451..db08e5e30 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -64,7 +64,7 @@ _SUPPRESS_REQUESTS_INSTRUMENTATION_KEY = "suppress_requests_instrumentation" # pylint: disable=unused-argument # pylint: disable=R0915 -def _instrument(tracer_provider=None, span_callback=None, name_callback=get_default_span_name): +def _instrument(tracer_provider=None, span_callback=None, name_callback=None): """Enables tracing of all requests calls that go through :code:`requests.session.Session.request` (this includes :code:`requests.get`, etc.).""" @@ -124,7 +124,11 @@ def _instrument(tracer_provider=None, span_callback=None, name_callback=get_defa # See # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#http-client method = method.upper() - span_name = name_callback(method, url) + span_name = "" + if name_callback: + span_name = name_callback() + if not span_name: + span_name = get_default_span_name(method) recorder = RequestsInstrumentor().metric_recorder @@ -217,10 +221,9 @@ def _uninstrument_from(instr_root, restore_as_bound_func=False): setattr(instr_root, instr_func_name, original) -# pylint: disable=unused-argument -def get_default_span_name(method_name, url): +def get_default_span_name(method): """Default implementation for name_callback, returns HTTP {method_name}.""" - return "HTTP {}".format(method_name).strip() + return "HTTP {}".format(method).strip() class RequestsInstrumentor(BaseInstrumentor, MetricMixin): diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index 81f21113e..2f84f983d 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -110,6 +110,7 @@ class RequestsIntegrationTestBase(abc.ABC): def test_name_callback(self): def name_callback(): return "test_name" + RequestsInstrumentor().uninstrument() RequestsInstrumentor().instrument(name_callback=name_callback) result = self.perform_request(self.URL) @@ -121,6 +122,7 @@ class RequestsIntegrationTestBase(abc.ABC): def test_name_callback_default(self): def name_callback(): return 123 + RequestsInstrumentor().uninstrument() RequestsInstrumentor().instrument(name_callback=name_callback) result = self.perform_request(self.URL) From 4f79f80bc97f4a537a273625f6901855a58ff5b4 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 10:35:30 -0500 Subject: [PATCH 07/12] test --- .../src/opentelemetry/instrumentation/requests/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index db08e5e30..02128f944 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -125,9 +125,9 @@ def _instrument(tracer_provider=None, span_callback=None, name_callback=None): # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#http-client method = method.upper() span_name = "" - if name_callback: + if name_callback is not None: span_name = name_callback() - if not span_name: + if not span_name or not isinstance(span_name, str): span_name = get_default_span_name(method) recorder = RequestsInstrumentor().metric_recorder From e76106623ff29d76b68e60de683ba0131a9e0001 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 9 Nov 2020 10:50:04 -0500 Subject: [PATCH 08/12] changelog --- .../opentelemetry-instrumentation-requests/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md index 491d5a95f..164fd0823 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-requests/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased - Add span name callback - ([#157](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/157)) + ([#158](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/158)) ## Version 0.15b0 From d688bd779b54b65bda1fe6b621d79ddcf317357f Mon Sep 17 00:00:00 2001 From: Jonathan Wong Date: Tue, 17 Nov 2020 19:44:46 +0000 Subject: [PATCH 09/12] Fix broken flask instrumentation doc link --- instrumentation/opentelemetry-instrumentation-flask/README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/README.rst b/instrumentation/opentelemetry-instrumentation-flask/README.rst index f79d8fd60..9d393c688 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/README.rst +++ b/instrumentation/opentelemetry-instrumentation-flask/README.rst @@ -34,5 +34,5 @@ will exclude requests such as ``https://site/client/123/info`` and ``https://sit References ---------- -* `OpenTelemetry Flask Instrumentation `_ +* `OpenTelemetry Flask Instrumentation `_ * `OpenTelemetry Project `_ From cee66ec04fd1aa67d454b3168a80a748b16f6ced Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 19:06:25 +0530 Subject: [PATCH 10/12] 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 ad0e9d931..bfc1b3d79 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 172a1d744..c78feac7d 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -208,3 +208,32 @@ class TestProgrammaticCustomSpanName( 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 11/12] 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 c78feac7d..0bed5d20d 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -220,6 +220,7 @@ class TestProgrammaticCustomSpanNameCallbackWithoutApp( 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__) From fd493f446f47b9a3eca06e8940f4712620b96389 Mon Sep 17 00:00:00 2001 From: "(Eliseo) Nathaniel Ruiz Nowell" Date: Wed, 18 Nov 2020 12:39:15 -0800 Subject: [PATCH 12/12] Fixes and Improvements to botocore instrumentation (#150) --- .../instrumentation/boto/__init__.py | 51 ++- .../CHANGELOG.md | 2 + .../instrumentation/botocore/__init__.py | 171 +++------- .../tests/test_botocore_instrumentation.py | 323 +++++++++++++----- 4 files changed, 339 insertions(+), 208 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-boto/src/opentelemetry/instrumentation/boto/__init__.py b/instrumentation/opentelemetry-instrumentation-boto/src/opentelemetry/instrumentation/boto/__init__.py index 57a6fb16f..58dee0ba7 100644 --- a/instrumentation/opentelemetry-instrumentation-boto/src/opentelemetry/instrumentation/boto/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-boto/src/opentelemetry/instrumentation/boto/__init__.py @@ -48,13 +48,15 @@ from boto.connection import AWSAuthConnection, AWSQueryConnection from wrapt import wrap_function_wrapper from opentelemetry.instrumentation.boto.version import __version__ -from opentelemetry.instrumentation.botocore import add_span_arg_tags, unwrap from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.utils import unwrap from opentelemetry.sdk.trace import Resource from opentelemetry.trace import SpanKind, get_tracer logger = logging.getLogger(__name__) +SERVICE_PARAMS_BLOCK_LIST = {"s3": ["params.Body"]} + def _get_instance_region_name(instance): region = getattr(instance, "region", None) @@ -201,3 +203,50 @@ class BotoInstrumentor(BaseInstrumentor): args, kwargs, ) + + +def flatten_dict(dict_, sep=".", prefix=""): + """ + Returns a normalized dict of depth 1 with keys in order of embedding + """ + # NOTE: This should probably be in `opentelemetry.instrumentation.utils`. + # adapted from https://stackoverflow.com/a/19647596 + return ( + { + prefix + sep + k if prefix else k: v + for kk, vv in dict_.items() + for k, v in flatten_dict(vv, sep, kk).items() + } + if isinstance(dict_, dict) + else {prefix: dict_} + ) + + +def add_span_arg_tags(span, aws_service, args, args_names, args_traced): + def truncate_arg_value(value, max_len=1024): + """Truncate values which are bytes and greater than `max_len`. + Useful for parameters like "Body" in `put_object` operations. + """ + if isinstance(value, bytes) and len(value) > max_len: + return b"..." + + return value + + if not span.is_recording(): + return + + # Do not trace `Key Management Service` or `Secure Token Service` API calls + # over concerns of security leaks. + if aws_service not in {"kms", "sts"}: + tags = dict( + (name, value) + for (name, value) in zip(args_names, args) + if name in args_traced + ) + tags = flatten_dict(tags) + + for param_key, value in tags.items(): + if param_key in SERVICE_PARAMS_BLOCK_LIST.get(aws_service, {}): + continue + + span.set_attribute(param_key, truncate_arg_value(value)) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-botocore/CHANGELOG.md index ea05e4d72..800f8487f 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-botocore/CHANGELOG.md @@ -5,6 +5,8 @@ ([#181](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/181)) - Make botocore instrumentation check if instrumentation has been suppressed ([#182](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/182)) +- Botocore SpanKind as CLIENT and modify existing traced attributes + ([#150])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/150) ## Version 0.13b0 diff --git a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py index 1dd1989c5..713a1e6a6 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py @@ -49,12 +49,14 @@ API import logging from botocore.client import BaseClient +from botocore.exceptions import ClientError, ParamValidationError from wrapt import ObjectProxy, wrap_function_wrapper from opentelemetry import context as context_api from opentelemetry import propagators from opentelemetry.instrumentation.botocore.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.utils import unwrap from opentelemetry.sdk.trace import Resource from opentelemetry.trace import SpanKind, get_tracer @@ -70,15 +72,13 @@ def _patched_endpoint_prepare_request(wrapped, instance, args, kwargs): class BotocoreInstrumentor(BaseInstrumentor): - """A instrumentor for Botocore + """An instrumentor for Botocore. See `BaseInstrumentor` """ def _instrument(self, **kwargs): - # FIXME should the tracer provider be accessed via Configuration - # instead? # pylint: disable=attribute-defined-outside-init self._tracer = get_tracer( __name__, __version__, kwargs.get("tracer_provider") @@ -99,137 +99,66 @@ class BotocoreInstrumentor(BaseInstrumentor): def _uninstrument(self, **kwargs): unwrap(BaseClient, "_make_api_call") + # pylint: disable=too-many-branches def _patched_api_call(self, original_func, instance, args, kwargs): if context_api.get_value("suppress_instrumentation"): return original_func(*args, **kwargs) - endpoint_name = deep_getattr(instance, "_endpoint._endpoint_prefix") + # pylint: disable=protected-access + service_name = instance._service_model.service_name + operation_name, api_params = args + + error = None + result = None with self._tracer.start_as_current_span( - "{}.command".format(endpoint_name), kind=SpanKind.CONSUMER, + "{}".format(service_name), kind=SpanKind.CLIENT, ) as span: + if span.is_recording(): + span.set_attribute("aws.operation", operation_name) + span.set_attribute("aws.region", instance.meta.region_name) + span.set_attribute("aws.service", service_name) + if "QueueUrl" in api_params: + span.set_attribute("aws.queue_url", api_params["QueueUrl"]) + if "TableName" in api_params: + span.set_attribute( + "aws.table_name", api_params["TableName"] + ) - operation = None - if args and span.is_recording(): - operation = args[0] - span.resource = Resource( - attributes={ - "endpoint": endpoint_name, - "operation": operation.lower(), - } - ) + try: + result = original_func(*args, **kwargs) + except ClientError as ex: + error = ex - else: - span.resource = Resource( - attributes={"endpoint": endpoint_name} - ) - - add_span_arg_tags( - span, - endpoint_name, - args, - ("action", "params", "path", "verb"), - {"params", "path", "verb"}, - ) + if error: + result = error.response if span.is_recording(): - region_name = deep_getattr(instance, "meta.region_name") + if "ResponseMetadata" in result: + metadata = result["ResponseMetadata"] + req_id = None + if "RequestId" in metadata: + req_id = metadata["RequestId"] + elif "HTTPHeaders" in metadata: + headers = metadata["HTTPHeaders"] + if "x-amzn-RequestId" in headers: + req_id = headers["x-amzn-RequestId"] + elif "x-amz-request-id" in headers: + req_id = headers["x-amz-request-id"] + elif "x-amz-id-2" in headers: + req_id = headers["x-amz-id-2"] - meta = { - "aws.agent": "botocore", - "aws.operation": operation, - "aws.region": region_name, - } - for key, value in meta.items(): - span.set_attribute(key, value) + if req_id: + span.set_attribute( + "aws.request_id", req_id, + ) - result = original_func(*args, **kwargs) + if "HTTPStatusCode" in metadata: + span.set_attribute( + "http.status_code", metadata["HTTPStatusCode"], + ) - if span.is_recording(): - span.set_attribute( - "http.status_code", - result["ResponseMetadata"]["HTTPStatusCode"], - ) - span.set_attribute( - "retry_attempts", - result["ResponseMetadata"]["RetryAttempts"], - ) + if error: + raise error return result - - -def unwrap(obj, attr): - function = getattr(obj, attr, None) - if ( - function - and isinstance(function, ObjectProxy) - and hasattr(function, "__wrapped__") - ): - setattr(obj, attr, function.__wrapped__) - - -def add_span_arg_tags(span, endpoint_name, args, args_names, args_traced): - def truncate_arg_value(value, max_len=1024): - """Truncate values which are bytes and greater than `max_len`. - Useful for parameters like "Body" in `put_object` operations. - """ - if isinstance(value, bytes) and len(value) > max_len: - return b"..." - - return value - - def flatten_dict(dict_, sep=".", prefix=""): - """ - Returns a normalized dict of depth 1 with keys in order of embedding - """ - # adapted from https://stackoverflow.com/a/19647596 - return ( - { - prefix + sep + k if prefix else k: v - for kk, vv in dict_.items() - for k, v in flatten_dict(vv, sep, kk).items() - } - if isinstance(dict_, dict) - else {prefix: dict_} - ) - - if not span.is_recording(): - return - - if endpoint_name not in {"kms", "sts"}: - tags = dict( - (name, value) - for (name, value) in zip(args_names, args) - if name in args_traced - ) - tags = flatten_dict(tags) - for key, value in { - k: truncate_arg_value(v) - for k, v in tags.items() - if k not in {"s3": ["params.Body"]}.get(endpoint_name, []) - }.items(): - span.set_attribute(key, value) - - -def deep_getattr(obj, attr_string, default=None): - """ - Returns the attribute of ``obj`` at the dotted path given by - ``attr_string``, if no such attribute is reachable, returns ``default``. - - >>> deep_getattr(cass, "cluster") - >> deep_getattr(cass, "cluster.metadata.partitioner") - u"org.apache.cassandra.dht.Murmur3Partitioner" - - >>> deep_getattr(cass, "i.dont.exist", default="default") - "default" - """ - attrs = attr_string.split(".") - for attr in attrs: - try: - obj = getattr(obj, attr) - except AttributeError: - return default - - return obj diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index 8c1cb4c0c..23dc32d2d 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py @@ -17,29 +17,25 @@ from unittest.mock import Mock, patch import botocore.session from botocore.exceptions import ParamValidationError from moto import ( # pylint: disable=import-error + mock_dynamodb2, mock_ec2, mock_kinesis, mock_kms, mock_lambda, mock_s3, mock_sqs, + mock_sts, mock_xray, ) from opentelemetry import propagators +from opentelemetry import trace as trace_api from opentelemetry.context import attach, detach, set_value from opentelemetry.instrumentation.botocore import BotocoreInstrumentor -from opentelemetry.sdk.resources import Resource from opentelemetry.test.mock_textmap import MockTextMapPropagator from opentelemetry.test.test_base import TestBase -def assert_span_http_status_code(span, code): - """Assert on the span"s "http.status_code" tag""" - tag = span.attributes["http.status_code"] - assert tag == code, "%r != %r" % (tag, code) - - class TestBotocoreInstrumentor(TestBase): """Botocore integration testsuite""" @@ -66,20 +62,17 @@ class TestBotocoreInstrumentor(TestBase): assert spans span = spans[0] self.assertEqual(len(spans), 1) - self.assertEqual(span.attributes["aws.agent"], "botocore") - self.assertEqual(span.attributes["aws.region"], "us-west-2") - self.assertEqual(span.attributes["aws.operation"], "DescribeInstances") - assert_span_http_status_code(span, 200) self.assertEqual( - span.resource, - Resource( - attributes={ - "endpoint": "ec2", - "operation": "describeinstances", - } - ), + span.attributes, + { + "aws.operation": "DescribeInstances", + "aws.region": "us-west-2", + "aws.request_id": "fdcdcab1-ae5c-489e-9c33-4637c5dda355", + "aws.service": "ec2", + "http.status_code": 200, + }, ) - self.assertEqual(span.name, "ec2.command") + self.assertEqual(span.name, "ec2") @mock_ec2 def test_not_recording(self): @@ -117,13 +110,14 @@ class TestBotocoreInstrumentor(TestBase): assert spans span = spans[0] self.assertEqual(len(spans), 2) - self.assertEqual(span.attributes["aws.operation"], "ListBuckets") - assert_span_http_status_code(span, 200) self.assertEqual( - span.resource, - Resource( - attributes={"endpoint": "s3", "operation": "listbuckets"} - ), + span.attributes, + { + "aws.operation": "ListBuckets", + "aws.region": "us-west-2", + "aws.service": "s3", + "http.status_code": 200, + }, ) # testing for span error @@ -134,10 +128,15 @@ class TestBotocoreInstrumentor(TestBase): assert spans span = spans[2] self.assertEqual( - span.resource, - Resource( - attributes={"endpoint": "s3", "operation": "listobjects"} - ), + span.attributes, + { + "aws.operation": "ListObjects", + "aws.region": "us-west-2", + "aws.service": "s3", + }, + ) + self.assertIs( + span.status.status_code, trace_api.status.StatusCode.ERROR, ) # Comment test for issue 1088 @@ -148,29 +147,42 @@ class TestBotocoreInstrumentor(TestBase): location = {"LocationConstraint": "us-west-2"} s3.create_bucket(Bucket="mybucket", CreateBucketConfiguration=location) s3.put_object(**params) + s3.get_object(Bucket="mybucket", Key="foo") spans = self.memory_exporter.get_finished_spans() assert spans - span = spans[0] - self.assertEqual(len(spans), 2) - self.assertEqual(span.attributes["aws.operation"], "CreateBucket") - assert_span_http_status_code(span, 200) + self.assertEqual(len(spans), 3) + create_bucket_attributes = spans[0].attributes self.assertEqual( - span.resource, - Resource( - attributes={"endpoint": "s3", "operation": "createbucket"} - ), + create_bucket_attributes, + { + "aws.operation": "CreateBucket", + "aws.region": "us-west-2", + "aws.service": "s3", + "http.status_code": 200, + }, ) - self.assertEqual(spans[1].attributes["aws.operation"], "PutObject") + put_object_attributes = spans[1].attributes self.assertEqual( - spans[1].resource, - Resource(attributes={"endpoint": "s3", "operation": "putobject"}), - ) - self.assertEqual(spans[1].attributes["params.Key"], str(params["Key"])) - self.assertEqual( - spans[1].attributes["params.Bucket"], str(params["Bucket"]) + put_object_attributes, + { + "aws.operation": "PutObject", + "aws.region": "us-west-2", + "aws.service": "s3", + "http.status_code": 200, + }, ) self.assertTrue("params.Body" not in spans[1].attributes.keys()) + get_object_attributes = spans[2].attributes + self.assertEqual( + get_object_attributes, + { + "aws.operation": "GetObject", + "aws.region": "us-west-2", + "aws.service": "s3", + "http.status_code": 200, + }, + ) @mock_sqs def test_sqs_client(self): @@ -182,14 +194,62 @@ class TestBotocoreInstrumentor(TestBase): assert spans span = spans[0] self.assertEqual(len(spans), 1) - self.assertEqual(span.attributes["aws.region"], "us-east-1") - self.assertEqual(span.attributes["aws.operation"], "ListQueues") - assert_span_http_status_code(span, 200) + actual = span.attributes + self.assertRegex(actual["aws.request_id"], r"[A-Z0-9]{52}") + del actual["aws.request_id"] self.assertEqual( - span.resource, - Resource( - attributes={"endpoint": "sqs", "operation": "listqueues"} - ), + actual, + { + "aws.operation": "ListQueues", + "aws.region": "us-east-1", + "aws.service": "sqs", + "http.status_code": 200, + }, + ) + + @mock_sqs + def test_sqs_send_message(self): + sqs = self.session.create_client("sqs", region_name="us-east-1") + + test_queue_name = "test_queue_name" + + response = sqs.create_queue(QueueName=test_queue_name) + + sqs.send_message( + QueueUrl=response["QueueUrl"], MessageBody="Test SQS MESSAGE!" + ) + + spans = self.memory_exporter.get_finished_spans() + assert spans + self.assertEqual(len(spans), 2) + create_queue_attributes = spans[0].attributes + self.assertRegex( + create_queue_attributes["aws.request_id"], r"[A-Z0-9]{52}" + ) + del create_queue_attributes["aws.request_id"] + self.assertEqual( + create_queue_attributes, + { + "aws.operation": "CreateQueue", + "aws.region": "us-east-1", + "aws.service": "sqs", + "http.status_code": 200, + }, + ) + send_msg_attributes = spans[1].attributes + self.assertRegex( + send_msg_attributes["aws.request_id"], r"[A-Z0-9]{52}" + ) + del send_msg_attributes["aws.request_id"] + self.assertEqual( + send_msg_attributes, + { + "aws.operation": "SendMessage", + "aws.queue_url": response["QueueUrl"], + "aws.region": "us-east-1", + "aws.service": "sqs", + "http.status_code": 200, + }, ) @mock_kinesis @@ -204,14 +264,14 @@ class TestBotocoreInstrumentor(TestBase): assert spans span = spans[0] self.assertEqual(len(spans), 1) - self.assertEqual(span.attributes["aws.region"], "us-east-1") - self.assertEqual(span.attributes["aws.operation"], "ListStreams") - assert_span_http_status_code(span, 200) self.assertEqual( - span.resource, - Resource( - attributes={"endpoint": "kinesis", "operation": "liststreams"} - ), + span.attributes, + { + "aws.operation": "ListStreams", + "aws.region": "us-east-1", + "aws.service": "kinesis", + "http.status_code": 200, + }, ) @mock_kinesis @@ -249,14 +309,14 @@ class TestBotocoreInstrumentor(TestBase): assert spans span = spans[0] self.assertEqual(len(spans), 1) - self.assertEqual(span.attributes["aws.region"], "us-east-1") - self.assertEqual(span.attributes["aws.operation"], "ListFunctions") - assert_span_http_status_code(span, 200) self.assertEqual( - span.resource, - Resource( - attributes={"endpoint": "lambda", "operation": "listfunctions"} - ), + span.attributes, + { + "aws.operation": "ListFunctions", + "aws.region": "us-east-1", + "aws.service": "lambda", + "http.status_code": 200, + }, ) @mock_kms @@ -269,12 +329,38 @@ class TestBotocoreInstrumentor(TestBase): assert spans span = spans[0] self.assertEqual(len(spans), 1) - self.assertEqual(span.attributes["aws.region"], "us-east-1") - self.assertEqual(span.attributes["aws.operation"], "ListKeys") - assert_span_http_status_code(span, 200) self.assertEqual( - span.resource, - Resource(attributes={"endpoint": "kms", "operation": "listkeys"}), + span.attributes, + { + "aws.operation": "ListKeys", + "aws.region": "us-east-1", + "aws.service": "kms", + "http.status_code": 200, + }, + ) + + # checking for protection on kms against security leak + self.assertTrue("params" not in span.attributes.keys()) + + @mock_sts + def test_sts_client(self): + sts = self.session.create_client("sts", region_name="us-east-1") + + sts.get_caller_identity() + + spans = self.memory_exporter.get_finished_spans() + assert spans + span = spans[0] + self.assertEqual(len(spans), 1) + self.assertEqual( + span.attributes, + { + "aws.operation": "GetCallerIdentity", + "aws.region": "us-east-1", + "aws.request_id": "c6104cbe-af31-11e0-8154-cbc7ccf896c7", + "aws.service": "sts", + "http.status_code": 200, + }, ) # checking for protection on sts against security leak @@ -299,25 +385,19 @@ class TestBotocoreInstrumentor(TestBase): ec2.describe_instances() spans = self.memory_exporter.get_finished_spans() - assert spans - span = spans[0] self.assertEqual(len(spans), 1) - self.assertEqual(span.attributes["aws.agent"], "botocore") - self.assertEqual(span.attributes["aws.region"], "us-west-2") + span = spans[0] + describe_instances_attributes = spans[0].attributes self.assertEqual( - span.attributes["aws.operation"], "DescribeInstances" + describe_instances_attributes, + { + "aws.operation": "DescribeInstances", + "aws.region": "us-west-2", + "aws.request_id": "fdcdcab1-ae5c-489e-9c33-4637c5dda355", + "aws.service": "ec2", + "http.status_code": 200, + }, ) - assert_span_http_status_code(span, 200) - self.assertEqual( - span.resource, - Resource( - attributes={ - "endpoint": "ec2", - "operation": "describeinstances", - } - ), - ) - self.assertEqual(span.name, "ec2.command") self.assertIn(MockTextMapPropagator.TRACE_ID_KEY, headers) self.assertEqual( @@ -345,3 +425,74 @@ class TestBotocoreInstrumentor(TestBase): spans = self.memory_exporter.get_finished_spans() self.assertEqual(0, len(spans)) + + @mock_dynamodb2 + def test_dynamodb_client(self): + ddb = self.session.create_client("dynamodb", region_name="us-west-2") + + test_table_name = "test_table_name" + + ddb.create_table( + AttributeDefinitions=[ + {"AttributeName": "id", "AttributeType": "S"}, + ], + KeySchema=[{"AttributeName": "id", "KeyType": "HASH"}], + ProvisionedThroughput={ + "ReadCapacityUnits": 5, + "WriteCapacityUnits": 5, + }, + TableName=test_table_name, + ) + + ddb.put_item(TableName=test_table_name, Item={"id": {"S": "test_key"}}) + + ddb.get_item(TableName=test_table_name, Key={"id": {"S": "test_key"}}) + + spans = self.memory_exporter.get_finished_spans() + assert spans + self.assertEqual(len(spans), 3) + create_table_attributes = spans[0].attributes + self.assertRegex( + create_table_attributes["aws.request_id"], r"[A-Z0-9]{52}" + ) + del create_table_attributes["aws.request_id"] + self.assertEqual( + create_table_attributes, + { + "aws.operation": "CreateTable", + "aws.region": "us-west-2", + "aws.service": "dynamodb", + "aws.table_name": "test_table_name", + "http.status_code": 200, + }, + ) + put_item_attributes = spans[1].attributes + self.assertRegex( + put_item_attributes["aws.request_id"], r"[A-Z0-9]{52}" + ) + del put_item_attributes["aws.request_id"] + self.assertEqual( + put_item_attributes, + { + "aws.operation": "PutItem", + "aws.region": "us-west-2", + "aws.service": "dynamodb", + "aws.table_name": "test_table_name", + "http.status_code": 200, + }, + ) + get_item_attributes = spans[2].attributes + self.assertRegex( + get_item_attributes["aws.request_id"], r"[A-Z0-9]{52}" + ) + del get_item_attributes["aws.request_id"] + self.assertEqual( + get_item_attributes, + { + "aws.operation": "GetItem", + "aws.region": "us-west-2", + "aws.service": "dynamodb", + "aws.table_name": "test_table_name", + "http.status_code": 200, + }, + )