From f91ce7c6d9eafdb4b0d7e67697d1674a6a280005 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 17 Apr 2020 09:31:26 -0700 Subject: [PATCH 01/32] porting redis instrumentation --- .../tests/docker-compose.yml | 4 + .../tests/redis/test_redis_functional.py | 150 ++++++++++++++++++ .../README.rst | 23 +++ .../setup.cfg | 58 +++++++ .../setup.py | 26 +++ .../instrumentation/redis/__init__.py | 47 ++++++ .../instrumentation/redis/patch.py | 136 ++++++++++++++++ .../instrumentation/redis/util.py | 55 +++++++ .../instrumentation/redis/version.py | 15 ++ .../tests/__init__.py | 13 ++ .../tests/test_redis.py | 54 +++++++ tox.ini | 15 +- 12 files changed, 595 insertions(+), 1 deletion(-) create mode 100644 ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py create mode 100644 ext/opentelemetry-instrumentation-redis/README.rst create mode 100644 ext/opentelemetry-instrumentation-redis/setup.cfg create mode 100644 ext/opentelemetry-instrumentation-redis/setup.py create mode 100644 ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py create mode 100644 ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py create mode 100644 ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py create mode 100644 ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/version.py create mode 100644 ext/opentelemetry-instrumentation-redis/tests/__init__.py create mode 100644 ext/opentelemetry-instrumentation-redis/tests/test_redis.py diff --git a/ext/opentelemetry-ext-docker-tests/tests/docker-compose.yml b/ext/opentelemetry-ext-docker-tests/tests/docker-compose.yml index c2632dc247b..3e642c584e5 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/docker-compose.yml +++ b/ext/opentelemetry-ext-docker-tests/tests/docker-compose.yml @@ -23,4 +23,8 @@ services: POSTGRES_USER: testuser POSTGRES_PASSWORD: testpassword POSTGRES_DB: opentelemetry-tests + otredis: + image: redis:4.0-alpine + ports: + - "127.0.0.1:6379:6379" diff --git a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py new file mode 100644 index 00000000000..1b00edea98d --- /dev/null +++ b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py @@ -0,0 +1,150 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +import redis + +from opentelemetry import trace +from opentelemetry.instrumentation.redis.patch import patch, unpatch +from opentelemetry.sdk.trace import TracerProvider +from opentelemetry.sdk.trace.export import SimpleExportSpanProcessor +from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, +) +from opentelemetry.test.test_base import TestBase + + +class TestRedisPatch(TestBase): + + TEST_SERVICE = "redis" + TEST_PORT = 6379 + + def setUp(self): + patch() + self.redis_client = redis.Redis(port=self.TEST_PORT) + self.redis_client.flushall() + super().setUp() + + def tearDown(self): + unpatch() + + def test_long_command(self): + self.redis_client.mget(*range(1000)) + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + span = spans[0] + self.assertEqual(span.attributes["service"], self.TEST_SERVICE) + self.assertEqual(span.name, "redis.command") + self.assertIs( + span.status.canonical_code, trace.status.StatusCanonicalCode.OK + ) + + self.assertEqual(span.attributes.get("db.instance"), 0) + self.assertEqual( + span.attributes.get("db.url"), "redis://localhost:6379" + ) + + self.assertTrue( + span.attributes.get("db.statement").startswith("MGET 0 1 2 3") + ) + self.assertTrue(span.attributes.get("db.statement").endswith("...")) + + def test_basics(self): + self.assertIsNone(self.redis_client.get("cheese")) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + span = spans[0] + self.assertEqual(span.attributes["service"], self.TEST_SERVICE) + self.assertEqual(span.name, "redis.command") + self.assertIs( + span.status.canonical_code, trace.status.StatusCanonicalCode.OK + ) + self.assertEqual(span.attributes.get("db.instance"), 0) + self.assertEqual( + span.attributes.get("db.url"), "redis://localhost:6379" + ) + self.assertEqual(span.attributes.get("db.statement"), "GET cheese") + self.assertEqual(span.attributes.get("redis.args_length"), 2) + + def test_pipeline_traced(self): + with self.redis_client.pipeline(transaction=False) as pipeline: + pipeline.set("blah", 32) + pipeline.rpush("foo", "éé") + pipeline.hgetall("xxx") + pipeline.execute() + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + span = spans[0] + self.assertEqual(span.attributes["service"], self.TEST_SERVICE) + self.assertEqual(span.name, "redis.command") + self.assertIs( + span.status.canonical_code, trace.status.StatusCanonicalCode.OK + ) + self.assertEqual(span.attributes.get("db.instance"), 0) + self.assertEqual( + span.attributes.get("db.url"), "redis://localhost:6379" + ) + self.assertEqual( + span.attributes.get("db.statement"), + "SET blah 32\nRPUSH foo éé\nHGETALL xxx", + ) + self.assertEqual(span.attributes.get("redis.pipeline_length"), 3) + + def test_pipeline_immediate(self): + with self.redis_client.pipeline() as pipeline: + pipeline.set("a", 1) + pipeline.immediate_execute_command("SET", "b", 2) + pipeline.execute() + + spans = self.memory_exporter.get_finished_spans() + # expecting two separate spans here, rather than a + # single span for the whole pipeline + self.assertEqual(len(spans), 2) + span = spans[0] + self.assertEqual(span.attributes["service"], self.TEST_SERVICE) + self.assertEqual(span.name, "redis.command") + self.assertEqual(span.attributes.get("db.statement"), "SET b 2") + self.assertIs( + span.status.canonical_code, trace.status.StatusCanonicalCode.OK + ) + self.assertEqual(span.attributes.get("db.instance"), 0) + self.assertEqual( + span.attributes.get("db.url"), "redis://localhost:6379" + ) + + def test_parent(self): + """Ensure OpenTelemetry works with redis.""" + ot_tracer = trace.get_tracer("redis_svc") + + with ot_tracer.start_as_current_span("redis_get"): + self.assertIsNone(self.redis_client.get("cheese")) + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + child_span, parent_span = spans[0], spans[1] + + # confirm the parenting + self.assertIsNone(parent_span.parent) + self.assertIs(child_span.parent, parent_span) + + self.assertEqual(parent_span.name, "redis_get") + self.assertEqual(parent_span.instrumentation_info.name, "redis_svc") + + self.assertEqual( + child_span.attributes.get("service"), self.TEST_SERVICE + ) + self.assertEqual(child_span.name, "redis.command") diff --git a/ext/opentelemetry-instrumentation-redis/README.rst b/ext/opentelemetry-instrumentation-redis/README.rst new file mode 100644 index 00000000000..1a071ad0fee --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/README.rst @@ -0,0 +1,23 @@ +OpenTelemetry Redis Instrumentation +=================================== + +|pypi| + +.. |pypi| image:: https://badge.fury.io/py/opentelemetry-instrumentation-redis.svg + :target: https://pypi.org/project/opentelemetry-instrumentation-redis/ + +This library allows tracing requests made by the Redis library. + +Installation +------------ + +:: + + pip install opentelemetry-instrumentation-redis + + +References +---------- + +* `OpenTelemetry Redis Instrumentation `_ +* `OpenTelemetry Project `_ diff --git a/ext/opentelemetry-instrumentation-redis/setup.cfg b/ext/opentelemetry-instrumentation-redis/setup.cfg new file mode 100644 index 00000000000..43bc3848c0d --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/setup.cfg @@ -0,0 +1,58 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +[metadata] +name = opentelemetry-instrumentation-redis +description = Redis tracing for OpenTelemetry +long_description = file: README.rst +long_description_content_type = text/x-rst +author = OpenTelemetry Authors +author_email = cncf-opentelemetry-contributors@lists.cncf.io +url = https://github.com/open-telemetry/opentelemetry-python-contrib/tree/master/instrumentation/opentelemetry-instrumentation-redis +platforms = any +license = Apache-2.0 +classifiers = + Development Status :: 4 - Beta + Intended Audience :: Developers + License :: OSI Approved :: Apache Software License + Programming Language :: Python + Programming Language :: Python :: 3 + Programming Language :: Python :: 3.4 + Programming Language :: Python :: 3.5 + Programming Language :: Python :: 3.6 + Programming Language :: Python :: 3.7 + Programming Language :: Python :: 3.8 + +[options] +python_requires = >=3.4 +package_dir= + =src +packages=find_namespace: +install_requires = + opentelemetry-api == 0.7dev0 + opentelemetry-auto-instrumentation == 0.7dev0 + redis + wrapt >= 1.12.1 + +[options.extras_require] +test = + opentelemetry-test == 0.7.dev0 + opentelemetry-sdk == 0.7dev0 + +[options.packages.find] +where = src + +[options.entry_points] +opentelemetry_instrumentor = + redis = opentelemetry.instrumentation.redis:RedisInstrumentor diff --git a/ext/opentelemetry-instrumentation-redis/setup.py b/ext/opentelemetry-instrumentation-redis/setup.py new file mode 100644 index 00000000000..df80a8fd1aa --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/setup.py @@ -0,0 +1,26 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os + +import setuptools + +BASE_DIR = os.path.dirname(__file__) +VERSION_FILENAME = os.path.join( + BASE_DIR, "src", "opentelemetry", "instrumentation", "redis", "version.py" +) +PACKAGE_INFO = {} +with open(VERSION_FILENAME) as f: + exec(f.read(), PACKAGE_INFO) + +setuptools.setup(version=PACKAGE_INFO["__version__"]) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py new file mode 100644 index 00000000000..24890e3a043 --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -0,0 +1,47 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Instrument redis to report Redis queries. + +There are two options for instrumenting code. The first option is to use +the `opentelemetry-auto-instrumentation` executable which will automatically +patch your Redis client. The second is to programmatically enable +instrumentation via the following code: + +:: + + from opentelemetry.instrumentation.redis.patch import patch + import redis + + # You can patch redis specifically + patch() + + # This will report a span with the default settings + client = redis.StrictRedis(host="localhost", port=6379) + client.get("my-key") +""" +from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor +from opentelemetry.instrumentation.redis.patch import patch, unpatch + + +class RedisInstrumentor(BaseInstrumentor): + """An instrumentor for Redis + See `BaseInstrumentor` + """ + + def _instrument(self): + patch() + + def _uninstrument(self): + unpatch() diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py new file mode 100644 index 00000000000..631c89a35b5 --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py @@ -0,0 +1,136 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# pylint:disable=relative-beyond-top-level +import redis +from wrapt import ObjectProxy, wrap_function_wrapper + +from opentelemetry import trace + +from .util import _extract_conn_attributes, _format_command_args +from .version import __version__ + +_DEFAULT_SERVICE = "redis" +_RAWCMD = "db.statement" +_CMD = "redis.command" + + +def patch(tracer=None): + """Patch the instrumented methods + + This duplicated doesn't look nice. The nicer alternative is to use an ObjectProxy on top + of Redis and StrictRedis. However, it means that any "import redis.Redis" won't be instrumented. + """ + if getattr(redis, "_opentelemetry_patch", False): + return + setattr(redis, "_opentelemetry_patch", True) + + if tracer: + setattr(redis, "_opentelemetry_tracer", tracer) + + if redis.VERSION < (3, 0, 0): + wrap_function_wrapper( + "redis", "StrictRedis.execute_command", traced_execute_command + ) + wrap_function_wrapper( + "redis.client", "BasePipeline.execute", traced_execute_pipeline + ) + wrap_function_wrapper( + "redis.client", + "BasePipeline.immediate_execute_command", + traced_execute_command, + ) + else: + wrap_function_wrapper( + "redis", "Redis.execute_command", traced_execute_command + ) + wrap_function_wrapper( + "redis.client", "Pipeline.execute", traced_execute_pipeline + ) + wrap_function_wrapper( + "redis.client", + "Pipeline.immediate_execute_command", + traced_execute_command, + ) + + +def unwrap(obj, attr): + func = getattr(obj, attr, None) + if isinstance(func, ObjectProxy) and hasattr(func, "__wrapped__"): + setattr(obj, attr, func.__wrapped__) + + +def unpatch(): + if getattr(redis, "_opentelemetry_patch", False): + setattr(redis, "_opentelemetry_patch", False) + if redis.VERSION < (3, 0, 0): + unwrap(redis.StrictRedis, "execute_command") + unwrap(redis.StrictRedis, "pipeline") + unwrap(redis.Redis, "pipeline") + unwrap( + redis.client.BasePipeline, # pylint:disable=no-member + "execute", + ) + unwrap( + redis.client.BasePipeline, # pylint:disable=no-member + "immediate_execute_command", + ) + else: + unwrap(redis.Redis, "execute_command") + unwrap(redis.Redis, "pipeline") + unwrap(redis.client.Pipeline, "execute") + unwrap(redis.client.Pipeline, "immediate_execute_command") + + +def _get_tracer(): + tracer = getattr(redis, "_opentelemetry_tracer", False) + if tracer: + return tracer + tracer = trace.get_tracer(_DEFAULT_SERVICE, __version__) + setattr(redis, "_opentelemetry_tracer", tracer) + return tracer + + +def traced_execute_command(func, instance, args, kwargs): + tracer = _get_tracer() + query = _format_command_args(args) + with tracer.start_as_current_span(_CMD) as span: + span.set_attribute("service", tracer.instrumentation_info.name) + span.set_attribute(_RAWCMD, query) + _set_connection_attributes(span, instance) + span.set_attribute("redis.args_length", len(args)) + return func(*args, **kwargs) + + +def traced_execute_pipeline(func, instance, args, kwargs): + tracer = _get_tracer() + + cmds = [_format_command_args(c) for c, _ in instance.command_stack] + resource = "\n".join(cmds) + + with tracer.start_as_current_span(_CMD) as span: + span.set_attribute("service", tracer.instrumentation_info.name) + span.set_attribute(_RAWCMD, resource) + _set_connection_attributes(span, instance) + span.set_attribute( + "redis.pipeline_length", len(instance.command_stack) + ) + return func(*args, **kwargs) + + +def _set_connection_attributes(span, conn): + for key, value in _extract_conn_attributes( + conn.connection_pool.connection_kwargs + ).items(): + span.set_attribute(key, value) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py new file mode 100644 index 00000000000..5ce4587fb1b --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py @@ -0,0 +1,55 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Some utils used by the redis integration +""" + + +def _extract_conn_attributes(conn_kwargs): + """ Transform redis conn info into dict """ + try: + return { + "db.type": "redis", + "db.url": "redis://{}:{}".format( + conn_kwargs["host"], conn_kwargs["port"] + ), + "db.instance": conn_kwargs["db"] or 0, + } + except KeyError: + return {} + + +def _format_command_args(args): + """Format command arguments and trims them as needed""" + value_max_len = 100 + value_too_long_mark = "..." + cmd_max_len = 1000 + length = 0 + out = [] + for arg in args: + cmd = str(arg) + + if len(cmd) > value_max_len: + cmd = cmd[:value_max_len] + value_too_long_mark + + if length + len(cmd) > cmd_max_len: + prefix = cmd[: cmd_max_len - length] + out.append("%s%s" % (prefix, value_too_long_mark)) + break + + out.append(cmd) + length += len(cmd) + + return " ".join(out) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/version.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/version.py new file mode 100644 index 00000000000..86c61362ab5 --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/version.py @@ -0,0 +1,15 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +__version__ = "0.7.dev0" diff --git a/ext/opentelemetry-instrumentation-redis/tests/__init__.py b/ext/opentelemetry-instrumentation-redis/tests/__init__.py new file mode 100644 index 00000000000..b0a6f428417 --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/tests/__init__.py @@ -0,0 +1,13 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py new file mode 100644 index 00000000000..d574deaafe2 --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -0,0 +1,54 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from unittest import mock + +import redis + +from opentelemetry.instrumentation.redis.patch import patch, unpatch +from opentelemetry.test.test_base import TestBase + + +class TestRedis(TestBase): + def test_patch_unpatch(self): + # with mock.patch("redis.Redis"): + redis_client = redis.Redis() + # Test patch idempotence + patch() + patch() + + with mock.patch.object(redis_client, "connection"): + redis_client.get("key") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + self.memory_exporter.clear() + + # Test unpatch + unpatch() + + with mock.patch.object(redis_client, "connection"): + redis_client.get("key") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 0) + self.memory_exporter.clear() + + # Test patch again + patch() + + with mock.patch.object(redis_client, "connection"): + redis_client.get("key") + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) diff --git a/tox.ini b/tox.ini index 9d1d1a26cc7..3e4786f37d5 100644 --- a/tox.ini +++ b/tox.ini @@ -83,6 +83,10 @@ envlist = ; opentelemetry-ext-grpc py3{4,5,6,7,8}-test-ext-grpc + ; opentelemetry-instrumentation-redis + py3{4,5,6,7,8}-test-instrumentation-redis + pypy3-test-instrumentation-redis + ; Coverage is temporarily disabled for pypy3 due to the pytest bug. ; pypy3-coverage @@ -127,6 +131,7 @@ changedir = test-example-basic-tracer: docs/examples/basic_tracer/tests test-example-http: docs/examples/http/tests test-opentracing-shim: ext/opentelemetry-ext-opentracing-shim/tests + test-instrumentation-redis: ext/opentelemetry-instrumentation-redis/tests commands_pre = ; Install without -e to test the actual installation @@ -166,6 +171,11 @@ commands_pre = pymongo: pip install {toxinidir}/ext/opentelemetry-ext-pymongo psycopg2: pip install {toxinidir}/ext/opentelemetry-ext-dbapi psycopg2: pip install {toxinidir}/ext/opentelemetry-ext-psycopg2 + redis: pip install {toxinidir}/opentelemetry-auto-instrumentation + redis: pip install {toxinidir}/opentelemetry-sdk + redis: pip install {toxinidir}/tests/util + redis: pip install {toxinidir}/ext/opentelemetry-instrumentation-redis + redis: pip install {toxinidir}/ext/opentelemetry-instrumentation-redis[test] http-requests: pip install {toxinidir}/ext/opentelemetry-ext-http-requests http-requests: pip install {toxinidir}/tests/util http-requests: pip install {toxinidir}/opentelemetry-sdk @@ -229,6 +239,7 @@ deps = Deprecated thrift pymongo + redis flask mysql-connector-python wrapt @@ -267,6 +278,7 @@ deps = mysql-connector-python ~= 8.0 pymongo ~= 3.1 psycopg2-binary ~= 2.8.4 + redis changedir = ext/opentelemetry-ext-docker-tests/tests @@ -277,7 +289,8 @@ commands_pre = -e {toxinidir}/ext/opentelemetry-ext-dbapi \ -e {toxinidir}/ext/opentelemetry-ext-mysql \ -e {toxinidir}/ext/opentelemetry-ext-psycopg2 \ - -e {toxinidir}/ext/opentelemetry-ext-pymongo + -e {toxinidir}/ext/opentelemetry-ext-pymongo \ + -e {toxinidir}/ext/opentelemetry-instrumentation-redis docker-compose up -d python check_availability.py commands = From 88ea6f17669c2491d569d84a71adac89f7ad930b Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 17 Apr 2020 10:25:17 -0700 Subject: [PATCH 02/32] fix lint and docker-tests --- .isort.cfg | 2 +- tox.ini | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.isort.cfg b/.isort.cfg index c5723b06a20..ae2dfc32520 100644 --- a/.isort.cfg +++ b/.isort.cfg @@ -15,4 +15,4 @@ multi_line_output=3 skip=target skip_glob=**/gen/*,.venv*/*,venv*/* known_first_party=opentelemetry,opentelemetry_example_app -known_third_party=psutil,pytest +known_third_party=psutil,pytest,redis,redis_opentracing diff --git a/tox.ini b/tox.ini index 3e4786f37d5..51072f8c86d 100644 --- a/tox.ini +++ b/tox.ini @@ -262,10 +262,10 @@ deps = commands_pre = pip install -e {toxinidir}/opentelemetry-api \ + -e {toxinidir}/opentelemetry-auto-instrumentation \ -e {toxinidir}/opentelemetry-sdk \ -e {toxinidir}/ext/opentelemetry-ext-http-requests \ -e {toxinidir}/ext/opentelemetry-ext-wsgi \ - -e {toxinidir}/opentelemetry-auto-instrumentation \ -e {toxinidir}/ext/opentelemetry-ext-flask commands = @@ -278,7 +278,7 @@ deps = mysql-connector-python ~= 8.0 pymongo ~= 3.1 psycopg2-binary ~= 2.8.4 - redis + redis ~= 3.3.11 changedir = ext/opentelemetry-ext-docker-tests/tests @@ -286,6 +286,8 @@ changedir = commands_pre = pip install -e {toxinidir}/opentelemetry-api \ -e {toxinidir}/opentelemetry-sdk \ + -e {toxinidir}/opentelemetry-auto-instrumentation \ + -e {toxinidir}/tests/util \ -e {toxinidir}/ext/opentelemetry-ext-dbapi \ -e {toxinidir}/ext/opentelemetry-ext-mysql \ -e {toxinidir}/ext/opentelemetry-ext-psycopg2 \ From 4157cbfebbd0db6a07ec8cb1784a7848ec148035 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 20 Apr 2020 13:50:08 -0700 Subject: [PATCH 03/32] small cleanup --- ext/opentelemetry-instrumentation-redis/tests/test_redis.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py index d574deaafe2..736189bdcef 100644 --- a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py +++ b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -21,7 +21,6 @@ class TestRedis(TestBase): def test_patch_unpatch(self): - # with mock.patch("redis.Redis"): redis_client = redis.Redis() # Test patch idempotence patch() From 1e48c7da7743fcfa35a7a0ab4b29c6c8488d5e28 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 20 Apr 2020 14:09:46 -0700 Subject: [PATCH 04/32] improving documentation --- docs/ext/redis/redis.rst | 7 +++++++ .../MANIFEST.in | 9 +++++++++ .../instrumentation/redis/__init__.py | 19 ++++++++++++++++--- 3 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 docs/ext/redis/redis.rst create mode 100644 ext/opentelemetry-instrumentation-redis/MANIFEST.in diff --git a/docs/ext/redis/redis.rst b/docs/ext/redis/redis.rst new file mode 100644 index 00000000000..4e21bce24b9 --- /dev/null +++ b/docs/ext/redis/redis.rst @@ -0,0 +1,7 @@ +OpenTelemetry Redis Instrumentation +=================================== + +.. automodule:: opentelemetry.instrumentation.redis + :members: + :undoc-members: + :show-inheritance: diff --git a/ext/opentelemetry-instrumentation-redis/MANIFEST.in b/ext/opentelemetry-instrumentation-redis/MANIFEST.in new file mode 100644 index 00000000000..aed3e33273b --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/MANIFEST.in @@ -0,0 +1,9 @@ +graft src +graft tests +global-exclude *.pyc +global-exclude *.pyo +global-exclude __pycache__/* +include CHANGELOG.md +include MANIFEST.in +include README.rst +include LICENSE diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index 24890e3a043..e1aefde5803 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -12,24 +12,37 @@ # See the License for the specific language governing permissions and # limitations under the License. # -"""Instrument redis to report Redis queries. +""" +Instrument `redis`_ to report Redis queries. There are two options for instrumenting code. The first option is to use the `opentelemetry-auto-instrumentation` executable which will automatically patch your Redis client. The second is to programmatically enable instrumentation via the following code: -:: +.. _redis: https://pypi.org/project/redis/ + +Usage +----- + +.. code:: python + from opentelemetry import trace from opentelemetry.instrumentation.redis.patch import patch + from opentelemetry.sdk.trace import TracerProvider import redis + trace.set_tracer_provider(TracerProvider()) + # You can patch redis specifically - patch() + patch(trace.get_tracer_provider()) # This will report a span with the default settings client = redis.StrictRedis(host="localhost", port=6379) client.get("my-key") + +API +--- """ from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.redis.patch import patch, unpatch From d0af1ec1a6b0f704ab3f28833e0525e561fd5254 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 20 Apr 2020 14:12:40 -0700 Subject: [PATCH 05/32] expect a tracer_provider instead of a tracer --- .../src/opentelemetry/instrumentation/redis/patch.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py index 631c89a35b5..7050fd32985 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py @@ -26,7 +26,7 @@ _CMD = "redis.command" -def patch(tracer=None): +def patch(tracer_provider=None): """Patch the instrumented methods This duplicated doesn't look nice. The nicer alternative is to use an ObjectProxy on top @@ -36,8 +36,12 @@ def patch(tracer=None): return setattr(redis, "_opentelemetry_patch", True) - if tracer: - setattr(redis, "_opentelemetry_tracer", tracer) + if tracer_provider: + setattr( + redis, + "_opentelemetry_tracer", + tracer_provider.get_tracer(_DEFAULT_SERVICE, __version__), + ) if redis.VERSION < (3, 0, 0): wrap_function_wrapper( From 64e1d722ceca2098ec1571980dc9bc1466e8a761 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 20 Apr 2020 14:46:00 -0700 Subject: [PATCH 06/32] fixing tox file --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 2a4fe50b5ec..0b1b0523cee 100644 --- a/tox.ini +++ b/tox.ini @@ -176,6 +176,7 @@ commands_pre = psycopg2: pip install {toxinidir}/ext/opentelemetry-ext-dbapi psycopg2: pip install {toxinidir}/ext/opentelemetry-ext-psycopg2 + redis: pip install {toxinidir}/opentelemetry-auto-instrumentation redis: pip install {toxinidir}/ext/opentelemetry-instrumentation-redis[test] http-requests: pip install {toxinidir}/ext/opentelemetry-ext-http-requests[test] From 2e71d4fb88d047f986eb02ec3139cc2fd2dd16fe Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 20 Apr 2020 16:07:41 -0700 Subject: [PATCH 07/32] removing reload, it breaks checks for Span types --- tests/util/src/opentelemetry/test/test_base.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/util/src/opentelemetry/test/test_base.py b/tests/util/src/opentelemetry/test/test_base.py index 974cc894081..c2224cb9c67 100644 --- a/tests/util/src/opentelemetry/test/test_base.py +++ b/tests/util/src/opentelemetry/test/test_base.py @@ -31,9 +31,5 @@ def setUpClass(cls): span_processor = export.SimpleExportSpanProcessor(cls.memory_exporter) cls.tracer_provider.add_span_processor(span_processor) - @classmethod - def tearDownClass(cls): - reload(trace_api) - def setUp(self): self.memory_exporter.clear() From a98fb4779e9353827c40e3bac108d548dec18c75 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 21 Apr 2020 09:13:09 -0700 Subject: [PATCH 08/32] remove reload --- tests/util/src/opentelemetry/test/test_base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/util/src/opentelemetry/test/test_base.py b/tests/util/src/opentelemetry/test/test_base.py index c2224cb9c67..6f1d2f3e625 100644 --- a/tests/util/src/opentelemetry/test/test_base.py +++ b/tests/util/src/opentelemetry/test/test_base.py @@ -13,7 +13,6 @@ # limitations under the License. import unittest -from importlib import reload from opentelemetry import trace as trace_api from opentelemetry.sdk.trace import TracerProvider, export From 8933c0bc464ae27c8480aa5c47a68391d4d5c0c7 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 22 Apr 2020 09:35:46 -0700 Subject: [PATCH 09/32] cleaning up tests --- .../tests/redis/test_redis_functional.py | 12 +++--------- .../tests/test_redis.py | 2 ++ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py index 1b00edea98d..899427aa9e5 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py @@ -12,17 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -import unittest - import redis from opentelemetry import trace from opentelemetry.instrumentation.redis.patch import patch, unpatch -from opentelemetry.sdk.trace import TracerProvider -from opentelemetry.sdk.trace.export import SimpleExportSpanProcessor -from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( - InMemorySpanExporter, -) from opentelemetry.test.test_base import TestBase @@ -32,12 +25,13 @@ class TestRedisPatch(TestBase): TEST_PORT = 6379 def setUp(self): - patch() + super().setUp() self.redis_client = redis.Redis(port=self.TEST_PORT) self.redis_client.flushall() - super().setUp() + patch() def tearDown(self): + super().tearDown() unpatch() def test_long_command(self): diff --git a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py index 736189bdcef..ab4e3f084ba 100644 --- a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py +++ b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -35,6 +35,8 @@ def test_patch_unpatch(self): # Test unpatch unpatch() + # ensure double unpatching doesnt break things + unpatch() with mock.patch.object(redis_client, "connection"): redis_client.get("key") From 9b7e311baddc21c6d7d47dd771eab4c797c630b6 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 22 Apr 2020 09:41:19 -0700 Subject: [PATCH 10/32] adding comment to remember to update how the tracer is set --- .../src/opentelemetry/instrumentation/redis/patch.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py index 7050fd32985..312f56ad43c 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py @@ -98,6 +98,10 @@ def unpatch(): def _get_tracer(): + # NOTE: this code will be safe to remove once the instrumentator changes + # to support configuration are merged in. At that point, we can set + # the tracer during the initialization as we'll have a tracer_provider + # to do so. tracer = getattr(redis, "_opentelemetry_tracer", False) if tracer: return tracer From a33287b401dbaa32d0bcc59ab4e5ceba04aa91f0 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 22 Apr 2020 14:23:36 -0700 Subject: [PATCH 11/32] updating code to use instrument/uninstrument interface --- .../tests/redis/test_redis_functional.py | 6 +- .../instrumentation/redis/__init__.py | 15 ++-- .../instrumentation/redis/patch.py | 71 +++++++------------ .../tests/test_redis.py | 12 ++-- 4 files changed, 44 insertions(+), 60 deletions(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py index 899427aa9e5..8bf9dd9a74e 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py @@ -15,7 +15,7 @@ import redis from opentelemetry import trace -from opentelemetry.instrumentation.redis.patch import patch, unpatch +from opentelemetry.instrumentation.redis import RedisInstrumentor from opentelemetry.test.test_base import TestBase @@ -28,11 +28,11 @@ def setUp(self): super().setUp() self.redis_client = redis.Redis(port=self.TEST_PORT) self.redis_client.flushall() - patch() + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) def tearDown(self): super().tearDown() - unpatch() + RedisInstrumentor().uninstrument() def test_long_command(self): self.redis_client.mget(*range(1000)) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index e1aefde5803..f92db3d3135 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -44,8 +44,9 @@ API --- """ +from opentelemetry import trace from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor -from opentelemetry.instrumentation.redis.patch import patch, unpatch +from opentelemetry.instrumentation.redis.patch import _patch, _unpatch class RedisInstrumentor(BaseInstrumentor): @@ -53,8 +54,12 @@ class RedisInstrumentor(BaseInstrumentor): See `BaseInstrumentor` """ - def _instrument(self): - patch() + def _instrument(self, **kwargs): + _patch( + tracer_provider=kwargs.get( + "tracer_provider", trace.get_tracer_provider() + ) + ) - def _uninstrument(self): - unpatch() + def _uninstrument(self, **kwargs): + _unpatch() diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py index 312f56ad43c..8d1215d8864 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py @@ -26,22 +26,17 @@ _CMD = "redis.command" -def patch(tracer_provider=None): +def _patch(tracer_provider=None): """Patch the instrumented methods This duplicated doesn't look nice. The nicer alternative is to use an ObjectProxy on top of Redis and StrictRedis. However, it means that any "import redis.Redis" won't be instrumented. """ - if getattr(redis, "_opentelemetry_patch", False): - return - setattr(redis, "_opentelemetry_patch", True) - - if tracer_provider: - setattr( - redis, - "_opentelemetry_tracer", - tracer_provider.get_tracer(_DEFAULT_SERVICE, __version__), - ) + setattr( + redis, + "_opentelemetry_tracer", + tracer_provider.get_tracer(_DEFAULT_SERVICE, __version__), + ) if redis.VERSION < (3, 0, 0): wrap_function_wrapper( @@ -75,43 +70,27 @@ def unwrap(obj, attr): setattr(obj, attr, func.__wrapped__) -def unpatch(): - if getattr(redis, "_opentelemetry_patch", False): - setattr(redis, "_opentelemetry_patch", False) - if redis.VERSION < (3, 0, 0): - unwrap(redis.StrictRedis, "execute_command") - unwrap(redis.StrictRedis, "pipeline") - unwrap(redis.Redis, "pipeline") - unwrap( - redis.client.BasePipeline, # pylint:disable=no-member - "execute", - ) - unwrap( - redis.client.BasePipeline, # pylint:disable=no-member - "immediate_execute_command", - ) - else: - unwrap(redis.Redis, "execute_command") - unwrap(redis.Redis, "pipeline") - unwrap(redis.client.Pipeline, "execute") - unwrap(redis.client.Pipeline, "immediate_execute_command") - - -def _get_tracer(): - # NOTE: this code will be safe to remove once the instrumentator changes - # to support configuration are merged in. At that point, we can set - # the tracer during the initialization as we'll have a tracer_provider - # to do so. - tracer = getattr(redis, "_opentelemetry_tracer", False) - if tracer: - return tracer - tracer = trace.get_tracer(_DEFAULT_SERVICE, __version__) - setattr(redis, "_opentelemetry_tracer", tracer) - return tracer +def _unpatch(): + if redis.VERSION < (3, 0, 0): + unwrap(redis.StrictRedis, "execute_command") + unwrap(redis.StrictRedis, "pipeline") + unwrap(redis.Redis, "pipeline") + unwrap( + redis.client.BasePipeline, "execute", # pylint:disable=no-member + ) + unwrap( + redis.client.BasePipeline, # pylint:disable=no-member + "immediate_execute_command", + ) + else: + unwrap(redis.Redis, "execute_command") + unwrap(redis.Redis, "pipeline") + unwrap(redis.client.Pipeline, "execute") + unwrap(redis.client.Pipeline, "immediate_execute_command") def traced_execute_command(func, instance, args, kwargs): - tracer = _get_tracer() + tracer = getattr(redis, "_opentelemetry_tracer") query = _format_command_args(args) with tracer.start_as_current_span(_CMD) as span: span.set_attribute("service", tracer.instrumentation_info.name) @@ -122,7 +101,7 @@ def traced_execute_command(func, instance, args, kwargs): def traced_execute_pipeline(func, instance, args, kwargs): - tracer = _get_tracer() + tracer = getattr(redis, "_opentelemetry_tracer") cmds = [_format_command_args(c) for c, _ in instance.command_stack] resource = "\n".join(cmds) diff --git a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py index ab4e3f084ba..bc6db9acf15 100644 --- a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py +++ b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -15,7 +15,7 @@ import redis -from opentelemetry.instrumentation.redis.patch import patch, unpatch +from opentelemetry.instrumentation.redis import RedisInstrumentor from opentelemetry.test.test_base import TestBase @@ -23,8 +23,8 @@ class TestRedis(TestBase): def test_patch_unpatch(self): redis_client = redis.Redis() # Test patch idempotence - patch() - patch() + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) with mock.patch.object(redis_client, "connection"): redis_client.get("key") @@ -34,9 +34,9 @@ def test_patch_unpatch(self): self.memory_exporter.clear() # Test unpatch - unpatch() + RedisInstrumentor().uninstrument() # ensure double unpatching doesnt break things - unpatch() + RedisInstrumentor().uninstrument() with mock.patch.object(redis_client, "connection"): redis_client.get("key") @@ -46,7 +46,7 @@ def test_patch_unpatch(self): self.memory_exporter.clear() # Test patch again - patch() + RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) with mock.patch.object(redis_client, "connection"): redis_client.get("key") From 5f400679b886a13a8a0289335487b8bce5f80462 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 23 Apr 2020 08:41:37 -0700 Subject: [PATCH 12/32] adding check for redis, refactoring and improving error handling --- .../tests/check_availability.py | 107 ++++++++++-------- 1 file changed, 57 insertions(+), 50 deletions(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/check_availability.py b/ext/opentelemetry-ext-docker-tests/tests/check_availability.py index 0b4376030d9..4f98c886291 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/check_availability.py +++ b/ext/opentelemetry-ext-docker-tests/tests/check_availability.py @@ -18,6 +18,7 @@ import mysql.connector import psycopg2 import pymongo +import redis MONGODB_COLLECTION_NAME = "test" MONGODB_DB_NAME = os.getenv("MONGODB_DB_NAME", "opentelemetry-tests") @@ -33,69 +34,74 @@ POSTGRES_PASSWORD = os.getenv("POSTGRESQL_HOST", "testpassword") POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT", "5432")) POSTGRES_USER = os.getenv("POSTGRESQL_HOST", "testuser") +REDIS_HOST = os.getenv("REDIS_HOST", "localhost") +REDIS_PORT = int(os.getenv("REDIS_PORT ", "6379")) RETRY_COUNT = 5 RETRY_INTERVAL = 5 # Seconds logger = logging.getLogger(__name__) +def retryable(func): + def wrapper(): + # Try to connect to DB + for i in range(RETRY_COUNT): + try: + func() + break + except Exception as ex: # pylint: disable=broad-except + if i == RETRY_COUNT - 1: + raise ex + logger.error( + "waiting for %s, retry %d/%d [%s]", + func.__name__, + i + 1, + RETRY_COUNT, + ex, + ) + time.sleep(RETRY_INTERVAL) + + return wrapper + + def check_pymongo_connection(): - # Try to connect to DB - for i in range(RETRY_COUNT): - try: - client = pymongo.MongoClient( - MONGODB_HOST, MONGODB_PORT, serverSelectionTimeoutMS=2000 - ) - db = client[MONGODB_DB_NAME] - collection = db[MONGODB_COLLECTION_NAME] - collection.find_one() - client.close() - break - except Exception as ex: - if i == RETRY_COUNT - 1: - raise (ex) - logger.exception(ex) - time.sleep(RETRY_INTERVAL) + client = pymongo.MongoClient( + MONGODB_HOST, MONGODB_PORT, serverSelectionTimeoutMS=2000 + ) + db = client[MONGODB_DB_NAME] + collection = db[MONGODB_COLLECTION_NAME] + collection.find_one() + client.close() +@retryable def check_mysql_connection(): - # Try to connect to DB - for i in range(RETRY_COUNT): - try: - connection = mysql.connector.connect( - user=MYSQL_USER, - password=MYSQL_PASSWORD, - host=MYSQL_HOST, - port=MYSQL_PORT, - database=MYSQL_DB_NAME, - ) - connection.close() - break - except Exception as ex: - if i == RETRY_COUNT - 1: - raise (ex) - logger.exception(ex) - time.sleep(RETRY_INTERVAL) + connection = mysql.connector.connect( + user=MYSQL_USER, + password=MYSQL_PASSWORD, + host=MYSQL_HOST, + port=MYSQL_PORT, + database=MYSQL_DB_NAME, + ) + connection.close() +@retryable def check_postgres_connection(): - # Try to connect to DB - for i in range(RETRY_COUNT): - try: - connection = psycopg2.connect( - dbname=POSTGRES_DB_NAME, - user=POSTGRES_USER, - password=POSTGRES_PASSWORD, - host=POSTGRES_HOST, - port=POSTGRES_PORT, - ) - connection.close() - break - except Exception as ex: - if i == RETRY_COUNT - 1: - raise (ex) - logger.exception(ex) - time.sleep(RETRY_INTERVAL) + connection = psycopg2.connect( + dbname=POSTGRES_DB_NAME, + user=POSTGRES_USER, + password=POSTGRES_PASSWORD, + host=POSTGRES_HOST, + port=POSTGRES_PORT, + ) + connection.close() + + +@retryable +def check_redis_connection(): + connection = redis.Redis(host=REDIS_HOST, port=REDIS_PORT) + connection.hgetall("*") def check_docker_services_availability(): @@ -103,6 +109,7 @@ def check_docker_services_availability(): check_pymongo_connection() check_mysql_connection() check_postgres_connection() + check_redis_connection() check_docker_services_availability() From 2bcadf52d59789abc8bfd4da3fd00598437a5fc8 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 23 Apr 2020 08:42:31 -0700 Subject: [PATCH 13/32] clean unused import --- .../src/opentelemetry/instrumentation/redis/patch.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py index 8d1215d8864..1b22a60d45f 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py @@ -16,8 +16,6 @@ import redis from wrapt import ObjectProxy, wrap_function_wrapper -from opentelemetry import trace - from .util import _extract_conn_attributes, _format_command_args from .version import __version__ From 65c6cb3f5b76296c8f75334bf5b07b0b6a1b5e55 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 23 Apr 2020 08:49:05 -0700 Subject: [PATCH 14/32] adding changelog --- ext/opentelemetry-instrumentation-redis/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 ext/opentelemetry-instrumentation-redis/CHANGELOG.md diff --git a/ext/opentelemetry-instrumentation-redis/CHANGELOG.md b/ext/opentelemetry-instrumentation-redis/CHANGELOG.md new file mode 100644 index 00000000000..33144da9132 --- /dev/null +++ b/ext/opentelemetry-instrumentation-redis/CHANGELOG.md @@ -0,0 +1,5 @@ +# Changelog + +## Unreleased + +- Initial release \ No newline at end of file From 5fd98fee315ed9a236a7241040c483ab39dfda76 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 23 Apr 2020 12:57:26 -0700 Subject: [PATCH 15/32] update doc --- .../src/opentelemetry/instrumentation/redis/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index f92db3d3135..bd3ef7b26d1 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -28,14 +28,14 @@ .. code:: python from opentelemetry import trace - from opentelemetry.instrumentation.redis.patch import patch + from opentelemetry.instrumentation.redis import RedisInstrumentor from opentelemetry.sdk.trace import TracerProvider import redis trace.set_tracer_provider(TracerProvider()) # You can patch redis specifically - patch(trace.get_tracer_provider()) + RedisInstrumentor().instrument(tracer_provider=trace.get_tracer_provider()) # This will report a span with the default settings client = redis.StrictRedis(host="localhost", port=6379) From ffab5f91c4adabdc0e5c6638c10c2e933b1c0d9c Mon Sep 17 00:00:00 2001 From: alrex Date: Fri, 24 Apr 2020 08:25:11 -0700 Subject: [PATCH 16/32] Update ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Mauricio Vásquez --- .../src/opentelemetry/instrumentation/redis/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index bd3ef7b26d1..6fdab1a9ffa 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -34,7 +34,7 @@ trace.set_tracer_provider(TracerProvider()) - # You can patch redis specifically + # Instrument redis RedisInstrumentor().instrument(tracer_provider=trace.get_tracer_provider()) # This will report a span with the default settings From 039ad132b77e1dbf2af099e9ad4bea124e011336 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 24 Apr 2020 13:18:02 -0700 Subject: [PATCH 17/32] PyMySQL Integration (#504) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integration for PyMySQL. Fixes some documentation as well for other db integrations. Leverages dbapi. Co-authored-by: Mauricio Vásquez --- docs/ext/pymysql/pymysql.rst | 7 ++ .../src/opentelemetry/ext/dbapi/__init__.py | 3 + .../tests/pymysql/test_pymysql_functional.py | 108 ++++++++++++++++++ ext/opentelemetry-ext-mysql/setup.cfg | 1 + ext/opentelemetry-ext-pymysql/CHANGELOG.md | 3 + ext/opentelemetry-ext-pymysql/README.rst | 24 ++++ ext/opentelemetry-ext-pymysql/setup.cfg | 48 ++++++++ ext/opentelemetry-ext-pymysql/setup.py | 26 +++++ .../src/opentelemetry/ext/pymysql/__init__.py | 68 +++++++++++ .../src/opentelemetry/ext/pymysql/version.py | 15 +++ .../tests/__init__.py | 0 .../tests/test_pymysql_integration.py | 38 ++++++ tox.ini | 11 ++ 13 files changed, 352 insertions(+) create mode 100644 docs/ext/pymysql/pymysql.rst create mode 100644 ext/opentelemetry-ext-docker-tests/tests/pymysql/test_pymysql_functional.py create mode 100644 ext/opentelemetry-ext-pymysql/CHANGELOG.md create mode 100644 ext/opentelemetry-ext-pymysql/README.rst create mode 100644 ext/opentelemetry-ext-pymysql/setup.cfg create mode 100644 ext/opentelemetry-ext-pymysql/setup.py create mode 100644 ext/opentelemetry-ext-pymysql/src/opentelemetry/ext/pymysql/__init__.py create mode 100644 ext/opentelemetry-ext-pymysql/src/opentelemetry/ext/pymysql/version.py create mode 100644 ext/opentelemetry-ext-pymysql/tests/__init__.py create mode 100644 ext/opentelemetry-ext-pymysql/tests/test_pymysql_integration.py diff --git a/docs/ext/pymysql/pymysql.rst b/docs/ext/pymysql/pymysql.rst new file mode 100644 index 00000000000..23dca80c4f2 --- /dev/null +++ b/docs/ext/pymysql/pymysql.rst @@ -0,0 +1,7 @@ +OpenTelemetry PyMySQL Integration +================================= + +.. automodule:: opentelemetry.ext.pymysql + :members: + :undoc-members: + :show-inheritance: diff --git a/ext/opentelemetry-ext-dbapi/src/opentelemetry/ext/dbapi/__init__.py b/ext/opentelemetry-ext-dbapi/src/opentelemetry/ext/dbapi/__init__.py index 441434b1897..7a94b58957f 100644 --- a/ext/opentelemetry-ext-dbapi/src/opentelemetry/ext/dbapi/__init__.py +++ b/ext/opentelemetry-ext-dbapi/src/opentelemetry/ext/dbapi/__init__.py @@ -178,6 +178,9 @@ def get_connection_attributes(self, connection): self.name = self.database_component self.database = self.connection_props.get("database", "") if self.database: + # PyMySQL encodes names with utf-8 + if hasattr(self.database, "decode"): + self.database = self.database.decode(errors="ignore") self.name += "." + self.database user = self.connection_props.get("user") if user is not None: diff --git a/ext/opentelemetry-ext-docker-tests/tests/pymysql/test_pymysql_functional.py b/ext/opentelemetry-ext-docker-tests/tests/pymysql/test_pymysql_functional.py new file mode 100644 index 00000000000..e4ee65af30f --- /dev/null +++ b/ext/opentelemetry-ext-docker-tests/tests/pymysql/test_pymysql_functional.py @@ -0,0 +1,108 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import time +import unittest + +import pymysql as pymy + +from opentelemetry import trace as trace_api +from opentelemetry.ext.pymysql import trace_integration +from opentelemetry.sdk.trace import Tracer, TracerProvider +from opentelemetry.sdk.trace.export import SimpleExportSpanProcessor +from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, +) + +MYSQL_USER = os.getenv("MYSQL_USER ", "testuser") +MYSQL_PASSWORD = os.getenv("MYSQL_PASSWORD ", "testpassword") +MYSQL_HOST = os.getenv("MYSQL_HOST ", "localhost") +MYSQL_PORT = int(os.getenv("MYSQL_PORT ", "3306")) +MYSQL_DB_NAME = os.getenv("MYSQL_DB_NAME ", "opentelemetry-tests") + + +class TestFunctionalPyMysql(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls._connection = None + cls._cursor = None + cls._tracer_provider = TracerProvider() + cls._tracer = Tracer(cls._tracer_provider, None) + cls._span_exporter = InMemorySpanExporter() + cls._span_processor = SimpleExportSpanProcessor(cls._span_exporter) + cls._tracer_provider.add_span_processor(cls._span_processor) + trace_integration(cls._tracer_provider) + cls._connection = pymy.connect( + user=MYSQL_USER, + password=MYSQL_PASSWORD, + host=MYSQL_HOST, + port=MYSQL_PORT, + database=MYSQL_DB_NAME, + ) + cls._cursor = cls._connection.cursor() + + @classmethod + def tearDownClass(cls): + if cls._connection: + cls._connection.close() + + def setUp(self): + self._span_exporter.clear() + + def validate_spans(self): + spans = self._span_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + for span in spans: + if span.name == "rootSpan": + root_span = span + else: + db_span = span + self.assertIsInstance(span.start_time, int) + self.assertIsInstance(span.end_time, int) + self.assertIsNotNone(root_span) + self.assertIsNotNone(db_span) + self.assertEqual(root_span.name, "rootSpan") + self.assertEqual(db_span.name, "mysql.opentelemetry-tests") + self.assertIsNotNone(db_span.parent) + self.assertEqual(db_span.parent.name, root_span.name) + self.assertIs(db_span.kind, trace_api.SpanKind.CLIENT) + self.assertEqual(db_span.attributes["db.instance"], MYSQL_DB_NAME) + self.assertEqual(db_span.attributes["net.peer.name"], MYSQL_HOST) + self.assertEqual(db_span.attributes["net.peer.port"], MYSQL_PORT) + + def test_execute(self): + """Should create a child span for execute + """ + with self._tracer.start_as_current_span("rootSpan"): + self._cursor.execute("CREATE TABLE IF NOT EXISTS test (id INT)") + self.validate_spans() + + def test_executemany(self): + """Should create a child span for executemany + """ + with self._tracer.start_as_current_span("rootSpan"): + data = ["1", "2", "3"] + stmt = "INSERT INTO test (id) VALUES (%s)" + self._cursor.executemany(stmt, data) + self.validate_spans() + + def test_callproc(self): + """Should create a child span for callproc + """ + with self._tracer.start_as_current_span("rootSpan"), self.assertRaises( + Exception + ): + self._cursor.callproc("test", ()) + self.validate_spans() diff --git a/ext/opentelemetry-ext-mysql/setup.cfg b/ext/opentelemetry-ext-mysql/setup.cfg index 3da2aad0f01..0b592641ce3 100644 --- a/ext/opentelemetry-ext-mysql/setup.cfg +++ b/ext/opentelemetry-ext-mysql/setup.cfg @@ -41,6 +41,7 @@ package_dir= packages=find_namespace: install_requires = opentelemetry-api == 0.7.dev0 + opentelemetry-ext-dbapi == 0.7.dev0 mysql-connector-python ~= 8.0 wrapt >= 1.0.0, < 2.0.0 diff --git a/ext/opentelemetry-ext-pymysql/CHANGELOG.md b/ext/opentelemetry-ext-pymysql/CHANGELOG.md new file mode 100644 index 00000000000..1512c421622 --- /dev/null +++ b/ext/opentelemetry-ext-pymysql/CHANGELOG.md @@ -0,0 +1,3 @@ +# Changelog + +## Unreleased diff --git a/ext/opentelemetry-ext-pymysql/README.rst b/ext/opentelemetry-ext-pymysql/README.rst new file mode 100644 index 00000000000..3cf845366b9 --- /dev/null +++ b/ext/opentelemetry-ext-pymysql/README.rst @@ -0,0 +1,24 @@ +OpenTelemetry PyMySQL integration +================================= + +|pypi| + +.. |pypi| image:: https://badge.fury.io/py/opentelemetry-ext-pymysql.svg + :target: https://pypi.org/project/opentelemetry-ext-pymysql/ + +Integration with PyMySQL that supports the PyMySQL library and is +specified to trace_integration using 'PyMySQL'. + + +Installation +------------ + +:: + + pip install opentelemetry-ext-pymysql + + +References +---------- +* `OpenTelemetry PyMySQL Integration `_ +* `OpenTelemetry Project `_ diff --git a/ext/opentelemetry-ext-pymysql/setup.cfg b/ext/opentelemetry-ext-pymysql/setup.cfg new file mode 100644 index 00000000000..9f44953a402 --- /dev/null +++ b/ext/opentelemetry-ext-pymysql/setup.cfg @@ -0,0 +1,48 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +[metadata] +name = opentelemetry-ext-pymysql +description = OpenTelemetry PyMySQL integration +long_description = file: README.rst +long_description_content_type = text/x-rst +author = OpenTelemetry Authors +author_email = cncf-opentelemetry-contributors@lists.cncf.io +url = https://github.com/open-telemetry/opentelemetry-python/tree/master/ext/opentelemetry-ext-pymysql +platforms = any +license = Apache-2.0 +classifiers = + Development Status :: 4 - Beta + Intended Audience :: Developers + License :: OSI Approved :: Apache Software License + Programming Language :: Python + Programming Language :: Python :: 3 + Programming Language :: Python :: 3.4 + Programming Language :: Python :: 3.5 + Programming Language :: Python :: 3.6 + Programming Language :: Python :: 3.7 + Programming Language :: Python :: 3.8 + +[options] +python_requires = >=3.4 +package_dir= + =src +packages=find_namespace: +install_requires = + opentelemetry-api == 0.7.dev0 + opentelemetry-ext-dbapi == 0.7.dev0 + PyMySQL ~= 0.9.3 + +[options.packages.find] +where = src diff --git a/ext/opentelemetry-ext-pymysql/setup.py b/ext/opentelemetry-ext-pymysql/setup.py new file mode 100644 index 00000000000..a3f057b310b --- /dev/null +++ b/ext/opentelemetry-ext-pymysql/setup.py @@ -0,0 +1,26 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os + +import setuptools + +BASE_DIR = os.path.dirname(__file__) +VERSION_FILENAME = os.path.join( + BASE_DIR, "src", "opentelemetry", "ext", "pymysql", "version.py" +) +PACKAGE_INFO = {} +with open(VERSION_FILENAME) as f: + exec(f.read(), PACKAGE_INFO) + +setuptools.setup(version=PACKAGE_INFO["__version__"]) diff --git a/ext/opentelemetry-ext-pymysql/src/opentelemetry/ext/pymysql/__init__.py b/ext/opentelemetry-ext-pymysql/src/opentelemetry/ext/pymysql/__init__.py new file mode 100644 index 00000000000..0f06578e0bc --- /dev/null +++ b/ext/opentelemetry-ext-pymysql/src/opentelemetry/ext/pymysql/__init__.py @@ -0,0 +1,68 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +The integration with PyMySQL supports the `PyMySQL`_ library and is specified +to ``trace_integration`` using ``'PyMySQL'``. + +.. _PyMySQL: https://pypi.org/project/PyMySQL/ + +Usage +----- + +.. code:: python + + import pymysql + from opentelemetry import trace + from opentelemetry.ext.pymysql import trace_integration + from opentelemetry.sdk.trace import TracerProvider + + trace.set_tracer_provider(TracerProvider()) + trace_integration() + cnx = pymysql.connect(database="MySQL_Database") + cursor = cnx.cursor() + cursor.execute("INSERT INTO test (testField) VALUES (123)" + cnx.commit() + cursor.close() + cnx.close() + +API +--- +""" + +import typing + +import pymysql + +from opentelemetry.ext.dbapi import wrap_connect +from opentelemetry.ext.pymysql.version import __version__ +from opentelemetry.trace import TracerProvider, get_tracer + + +def trace_integration(tracer_provider: typing.Optional[TracerProvider] = None): + """Integrate with the PyMySQL library. + https://github.com/PyMySQL/PyMySQL/ + """ + + tracer = get_tracer(__name__, __version__, tracer_provider) + + connection_attributes = { + "database": "db", + "port": "port", + "host": "host", + "user": "user", + } + wrap_connect( + tracer, pymysql, "connect", "mysql", "sql", connection_attributes + ) diff --git a/ext/opentelemetry-ext-pymysql/src/opentelemetry/ext/pymysql/version.py b/ext/opentelemetry-ext-pymysql/src/opentelemetry/ext/pymysql/version.py new file mode 100644 index 00000000000..86c61362ab5 --- /dev/null +++ b/ext/opentelemetry-ext-pymysql/src/opentelemetry/ext/pymysql/version.py @@ -0,0 +1,15 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +__version__ = "0.7.dev0" diff --git a/ext/opentelemetry-ext-pymysql/tests/__init__.py b/ext/opentelemetry-ext-pymysql/tests/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ext/opentelemetry-ext-pymysql/tests/test_pymysql_integration.py b/ext/opentelemetry-ext-pymysql/tests/test_pymysql_integration.py new file mode 100644 index 00000000000..c452c31ec6d --- /dev/null +++ b/ext/opentelemetry-ext-pymysql/tests/test_pymysql_integration.py @@ -0,0 +1,38 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from unittest import mock + +import pymysql + +import opentelemetry.ext.pymysql +from opentelemetry.ext.pymysql import trace_integration +from opentelemetry.test.test_base import TestBase + + +class TestPyMysqlIntegration(TestBase): + def test_trace_integration(self): + with mock.patch("pymysql.connect"): + trace_integration() + cnx = pymysql.connect(database="test") + cursor = cnx.cursor() + query = "SELECT * FROM test" + cursor.execute(query) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + + # Check version and name in span's instrumentation info + self.check_span_instrumentation_info(span, opentelemetry.ext.pymysql) diff --git a/tox.ini b/tox.ini index 0b1b0523cee..c11df2ea68f 100644 --- a/tox.ini +++ b/tox.ini @@ -64,6 +64,10 @@ envlist = py3{4,5,6,7,8}-test-ext-pymongo pypy3-test-ext-pymongo + ; opentelemetry-ext-pymysql + py3{4,5,6,7,8}-test-ext-pymysql + pypy3-test-ext-pymysql + ; opentelemetry-ext-wsgi py3{4,5,6,7,8}-test-ext-wsgi pypy3-test-ext-wsgi @@ -124,6 +128,7 @@ changedir = test-ext-prometheus: ext/opentelemetry-ext-prometheus/tests test-ext-pymongo: ext/opentelemetry-ext-pymongo/tests test-ext-psycopg2: ext/opentelemetry-ext-psycopg2/tests + test-ext-pymysql: ext/opentelemetry-ext-pymysql/tests test-ext-wsgi: ext/opentelemetry-ext-wsgi/tests test-ext-zipkin: ext/opentelemetry-ext-zipkin/tests test-ext-flask: ext/opentelemetry-ext-flask/tests @@ -176,6 +181,9 @@ commands_pre = psycopg2: pip install {toxinidir}/ext/opentelemetry-ext-dbapi psycopg2: pip install {toxinidir}/ext/opentelemetry-ext-psycopg2 + pymysql: pip install {toxinidir}/ext/opentelemetry-ext-dbapi + pymysql: pip install {toxinidir}/ext/opentelemetry-ext-pymysql + redis: pip install {toxinidir}/opentelemetry-auto-instrumentation redis: pip install {toxinidir}/ext/opentelemetry-instrumentation-redis[test] @@ -241,6 +249,7 @@ deps = pymongo redis flask + pymysql mysql-connector-python wrapt psycopg2-binary @@ -277,6 +286,7 @@ deps = docker-compose >= 1.25.2 mysql-connector-python ~= 8.0 pymongo ~= 3.1 + pymysql ~= 0.9.3 psycopg2-binary ~= 2.8.4 redis ~= 3.3.11 @@ -292,6 +302,7 @@ commands_pre = -e {toxinidir}/ext/opentelemetry-ext-mysql \ -e {toxinidir}/ext/opentelemetry-ext-psycopg2 \ -e {toxinidir}/ext/opentelemetry-ext-pymongo \ + -e {toxinidir}/ext/opentelemetry-ext-pymysql \ -e {toxinidir}/ext/opentelemetry-instrumentation-redis docker-compose up -d python check_availability.py From 75a9bf46e618eceb80f087b55e58e2ece22f3a98 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 24 Apr 2020 14:19:50 -0700 Subject: [PATCH 18/32] review feedback --- .../tests/check_availability.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/check_availability.py b/ext/opentelemetry-ext-docker-tests/tests/check_availability.py index 4f98c886291..0545e1efe1a 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/check_availability.py +++ b/ext/opentelemetry-ext-docker-tests/tests/check_availability.py @@ -48,10 +48,8 @@ def wrapper(): for i in range(RETRY_COUNT): try: func() - break + return except Exception as ex: # pylint: disable=broad-except - if i == RETRY_COUNT - 1: - raise ex logger.error( "waiting for %s, retry %d/%d [%s]", func.__name__, @@ -60,10 +58,12 @@ def wrapper(): ex, ) time.sleep(RETRY_INTERVAL) + raise Exception("waiting for %s failed") return wrapper +@retryable def check_pymongo_connection(): client = pymongo.MongoClient( MONGODB_HOST, MONGODB_PORT, serverSelectionTimeoutMS=2000 From 28e6d7cc0385f0f8aaeb3b4d76b54f914dc6833e Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 24 Apr 2020 14:28:24 -0700 Subject: [PATCH 19/32] review feedback --- .../tests/redis/test_redis_functional.py | 15 +++++++------- .../instrumentation/redis/patch.py | 20 +++++++++---------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py index 8bf9dd9a74e..546bc88d839 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py @@ -21,12 +21,11 @@ class TestRedisPatch(TestBase): - TEST_SERVICE = "redis" - TEST_PORT = 6379 + test_service = "redis" def setUp(self): super().setUp() - self.redis_client = redis.Redis(port=self.TEST_PORT) + self.redis_client = redis.Redis(port=6379) self.redis_client.flushall() RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) @@ -40,7 +39,7 @@ def test_long_command(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.attributes["service"], self.TEST_SERVICE) + self.assertEqual(span.attributes["service"], self.test_service) self.assertEqual(span.name, "redis.command") self.assertIs( span.status.canonical_code, trace.status.StatusCanonicalCode.OK @@ -61,7 +60,7 @@ def test_basics(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.attributes["service"], self.TEST_SERVICE) + self.assertEqual(span.attributes["service"], self.test_service) self.assertEqual(span.name, "redis.command") self.assertIs( span.status.canonical_code, trace.status.StatusCanonicalCode.OK @@ -83,7 +82,7 @@ def test_pipeline_traced(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.attributes["service"], self.TEST_SERVICE) + self.assertEqual(span.attributes["service"], self.test_service) self.assertEqual(span.name, "redis.command") self.assertIs( span.status.canonical_code, trace.status.StatusCanonicalCode.OK @@ -109,7 +108,7 @@ def test_pipeline_immediate(self): # single span for the whole pipeline self.assertEqual(len(spans), 2) span = spans[0] - self.assertEqual(span.attributes["service"], self.TEST_SERVICE) + self.assertEqual(span.attributes["service"], self.test_service) self.assertEqual(span.name, "redis.command") self.assertEqual(span.attributes.get("db.statement"), "SET b 2") self.assertIs( @@ -139,6 +138,6 @@ def test_parent(self): self.assertEqual(parent_span.instrumentation_info.name, "redis_svc") self.assertEqual( - child_span.attributes.get("service"), self.TEST_SERVICE + child_span.attributes.get("service"), self.test_service ) self.assertEqual(child_span.name, "redis.command") diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py index 1b22a60d45f..368229c65b7 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py @@ -62,7 +62,7 @@ def _patch(tracer_provider=None): ) -def unwrap(obj, attr): +def _unwrap(obj, attr): func = getattr(obj, attr, None) if isinstance(func, ObjectProxy) and hasattr(func, "__wrapped__"): setattr(obj, attr, func.__wrapped__) @@ -70,21 +70,21 @@ def unwrap(obj, attr): def _unpatch(): if redis.VERSION < (3, 0, 0): - unwrap(redis.StrictRedis, "execute_command") - unwrap(redis.StrictRedis, "pipeline") - unwrap(redis.Redis, "pipeline") - unwrap( + _unwrap(redis.StrictRedis, "execute_command") + _unwrap(redis.StrictRedis, "pipeline") + _unwrap(redis.Redis, "pipeline") + _unwrap( redis.client.BasePipeline, "execute", # pylint:disable=no-member ) - unwrap( + _unwrap( redis.client.BasePipeline, # pylint:disable=no-member "immediate_execute_command", ) else: - unwrap(redis.Redis, "execute_command") - unwrap(redis.Redis, "pipeline") - unwrap(redis.client.Pipeline, "execute") - unwrap(redis.client.Pipeline, "immediate_execute_command") + _unwrap(redis.Redis, "execute_command") + _unwrap(redis.Redis, "pipeline") + _unwrap(redis.client.Pipeline, "execute") + _unwrap(redis.client.Pipeline, "immediate_execute_command") def traced_execute_command(func, instance, args, kwargs): From 1b978a5647b7f6c527f6747a9dbd3705a1d7e763 Mon Sep 17 00:00:00 2001 From: alrex Date: Fri, 24 Apr 2020 14:33:40 -0700 Subject: [PATCH 20/32] Apply suggestions from code review Co-Authored-By: Diego Hurtado --- .../src/opentelemetry/instrumentation/redis/__init__.py | 2 +- .../src/opentelemetry/instrumentation/redis/util.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index 6fdab1a9ffa..7c6d07f6272 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -17,7 +17,7 @@ There are two options for instrumenting code. The first option is to use the `opentelemetry-auto-instrumentation` executable which will automatically -patch your Redis client. The second is to programmatically enable +instrument your Redis client. The second is to programmatically enable instrumentation via the following code: .. _redis: https://pypi.org/project/redis/ diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py index 5ce4587fb1b..4dd83c8888b 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py @@ -32,7 +32,7 @@ def _extract_conn_attributes(conn_kwargs): def _format_command_args(args): - """Format command arguments and trims them as needed""" + """Format command arguments and trim them as needed""" value_max_len = 100 value_too_long_mark = "..." cmd_max_len = 1000 From 5d743175ce40bf145cb9507f898e4f7df7a40746 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 24 Apr 2020 14:38:19 -0700 Subject: [PATCH 21/32] adding version for redis --- ext/opentelemetry-instrumentation-redis/setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-instrumentation-redis/setup.cfg b/ext/opentelemetry-instrumentation-redis/setup.cfg index 43bc3848c0d..139bbbe3299 100644 --- a/ext/opentelemetry-instrumentation-redis/setup.cfg +++ b/ext/opentelemetry-instrumentation-redis/setup.cfg @@ -42,7 +42,7 @@ packages=find_namespace: install_requires = opentelemetry-api == 0.7dev0 opentelemetry-auto-instrumentation == 0.7dev0 - redis + redis >= 2.6 wrapt >= 1.12.1 [options.extras_require] From d843cad9a31951b9a56a2a8104326805be81f940 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 24 Apr 2020 14:44:33 -0700 Subject: [PATCH 22/32] more review feedback changes --- .../src/opentelemetry/instrumentation/redis/__init__.py | 6 +----- .../src/opentelemetry/instrumentation/redis/patch.py | 2 +- ext/opentelemetry-instrumentation-redis/tests/test_redis.py | 6 +----- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index 7c6d07f6272..f2277361abc 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -55,11 +55,7 @@ class RedisInstrumentor(BaseInstrumentor): """ def _instrument(self, **kwargs): - _patch( - tracer_provider=kwargs.get( - "tracer_provider", trace.get_tracer_provider() - ) - ) + _patch(kwargs.get("tracer_provider", trace.get_tracer_provider())) def _uninstrument(self, **kwargs): _unpatch() diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py index 368229c65b7..42da4a501ea 100644 --- a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py +++ b/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py @@ -24,7 +24,7 @@ _CMD = "redis.command" -def _patch(tracer_provider=None): +def _patch(tracer_provider): """Patch the instrumented methods This duplicated doesn't look nice. The nicer alternative is to use an ObjectProxy on top diff --git a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py index bc6db9acf15..072e4bf6a8e 100644 --- a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py +++ b/ext/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -22,8 +22,6 @@ class TestRedis(TestBase): def test_patch_unpatch(self): redis_client = redis.Redis() - # Test patch idempotence - RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) with mock.patch.object(redis_client, "connection"): @@ -35,8 +33,6 @@ def test_patch_unpatch(self): # Test unpatch RedisInstrumentor().uninstrument() - # ensure double unpatching doesnt break things - RedisInstrumentor().uninstrument() with mock.patch.object(redis_client, "connection"): redis_client.get("key") @@ -46,7 +42,7 @@ def test_patch_unpatch(self): self.memory_exporter.clear() # Test patch again - RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) + RedisInstrumentor().instrument() with mock.patch.object(redis_client, "connection"): redis_client.get("key") From e064dc14e7f09c6fecb22de0fbc062471ad9c7cb Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 24 Apr 2020 14:59:23 -0700 Subject: [PATCH 23/32] rename from instrumentation to ext --- .../CHANGELOG.md | 0 .../MANIFEST.in | 0 ext/opentelemetry-ext-redis/README.rst | 23 +++++++++++++++++++ .../setup.cfg | 4 ++-- .../setup.py | 0 .../instrumentation/redis/__init__.py | 0 .../instrumentation/redis/patch.py | 0 .../instrumentation/redis/util.py | 0 .../instrumentation/redis/version.py | 0 .../tests/__init__.py | 0 .../tests/test_redis.py | 0 .../README.rst | 23 ------------------- tox.ini | 12 +++++----- 13 files changed, 31 insertions(+), 31 deletions(-) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/CHANGELOG.md (100%) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/MANIFEST.in (100%) create mode 100644 ext/opentelemetry-ext-redis/README.rst rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/setup.cfg (90%) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/setup.py (100%) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/src/opentelemetry/instrumentation/redis/__init__.py (100%) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/src/opentelemetry/instrumentation/redis/patch.py (100%) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/src/opentelemetry/instrumentation/redis/util.py (100%) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/src/opentelemetry/instrumentation/redis/version.py (100%) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/tests/__init__.py (100%) rename ext/{opentelemetry-instrumentation-redis => opentelemetry-ext-redis}/tests/test_redis.py (100%) delete mode 100644 ext/opentelemetry-instrumentation-redis/README.rst diff --git a/ext/opentelemetry-instrumentation-redis/CHANGELOG.md b/ext/opentelemetry-ext-redis/CHANGELOG.md similarity index 100% rename from ext/opentelemetry-instrumentation-redis/CHANGELOG.md rename to ext/opentelemetry-ext-redis/CHANGELOG.md diff --git a/ext/opentelemetry-instrumentation-redis/MANIFEST.in b/ext/opentelemetry-ext-redis/MANIFEST.in similarity index 100% rename from ext/opentelemetry-instrumentation-redis/MANIFEST.in rename to ext/opentelemetry-ext-redis/MANIFEST.in diff --git a/ext/opentelemetry-ext-redis/README.rst b/ext/opentelemetry-ext-redis/README.rst new file mode 100644 index 00000000000..49cc95f6b1e --- /dev/null +++ b/ext/opentelemetry-ext-redis/README.rst @@ -0,0 +1,23 @@ +OpenTelemetry Redis Instrumentation +=================================== + +|pypi| + +.. |pypi| image:: https://badge.fury.io/py/opentelemetry-ext-redis.svg + :target: https://pypi.org/project/opentelemetry-ext-redis/ + +This library allows tracing requests made by the Redis library. + +Installation +------------ + +:: + + pip install opentelemetry-ext-redis + + +References +---------- + +* `OpenTelemetry Redis Instrumentation `_ +* `OpenTelemetry Project `_ diff --git a/ext/opentelemetry-instrumentation-redis/setup.cfg b/ext/opentelemetry-ext-redis/setup.cfg similarity index 90% rename from ext/opentelemetry-instrumentation-redis/setup.cfg rename to ext/opentelemetry-ext-redis/setup.cfg index 139bbbe3299..5a8e3374156 100644 --- a/ext/opentelemetry-instrumentation-redis/setup.cfg +++ b/ext/opentelemetry-ext-redis/setup.cfg @@ -13,13 +13,13 @@ # limitations under the License. # [metadata] -name = opentelemetry-instrumentation-redis +name = opentelemetry-ext-redis description = Redis tracing for OpenTelemetry long_description = file: README.rst long_description_content_type = text/x-rst author = OpenTelemetry Authors author_email = cncf-opentelemetry-contributors@lists.cncf.io -url = https://github.com/open-telemetry/opentelemetry-python-contrib/tree/master/instrumentation/opentelemetry-instrumentation-redis +url = https://github.com/open-telemetry/opentelemetry-python/tree/master/ext/opentelemetry-ext-redis platforms = any license = Apache-2.0 classifiers = diff --git a/ext/opentelemetry-instrumentation-redis/setup.py b/ext/opentelemetry-ext-redis/setup.py similarity index 100% rename from ext/opentelemetry-instrumentation-redis/setup.py rename to ext/opentelemetry-ext-redis/setup.py diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py similarity index 100% rename from ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py rename to ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/patch.py similarity index 100% rename from ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/patch.py rename to ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/patch.py diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py b/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/util.py similarity index 100% rename from ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py rename to ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/util.py diff --git a/ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/version.py b/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/version.py similarity index 100% rename from ext/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/version.py rename to ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/version.py diff --git a/ext/opentelemetry-instrumentation-redis/tests/__init__.py b/ext/opentelemetry-ext-redis/tests/__init__.py similarity index 100% rename from ext/opentelemetry-instrumentation-redis/tests/__init__.py rename to ext/opentelemetry-ext-redis/tests/__init__.py diff --git a/ext/opentelemetry-instrumentation-redis/tests/test_redis.py b/ext/opentelemetry-ext-redis/tests/test_redis.py similarity index 100% rename from ext/opentelemetry-instrumentation-redis/tests/test_redis.py rename to ext/opentelemetry-ext-redis/tests/test_redis.py diff --git a/ext/opentelemetry-instrumentation-redis/README.rst b/ext/opentelemetry-instrumentation-redis/README.rst deleted file mode 100644 index 1a071ad0fee..00000000000 --- a/ext/opentelemetry-instrumentation-redis/README.rst +++ /dev/null @@ -1,23 +0,0 @@ -OpenTelemetry Redis Instrumentation -=================================== - -|pypi| - -.. |pypi| image:: https://badge.fury.io/py/opentelemetry-instrumentation-redis.svg - :target: https://pypi.org/project/opentelemetry-instrumentation-redis/ - -This library allows tracing requests made by the Redis library. - -Installation ------------- - -:: - - pip install opentelemetry-instrumentation-redis - - -References ----------- - -* `OpenTelemetry Redis Instrumentation `_ -* `OpenTelemetry Project `_ diff --git a/tox.ini b/tox.ini index c11df2ea68f..0e35428deff 100644 --- a/tox.ini +++ b/tox.ini @@ -87,9 +87,9 @@ envlist = ; opentelemetry-ext-grpc py3{4,5,6,7,8}-test-ext-grpc - ; opentelemetry-instrumentation-redis - py3{4,5,6,7,8}-test-instrumentation-redis - pypy3-test-instrumentation-redis + ; opentelemetry-ext-redis + py3{4,5,6,7,8}-test-ext-redis + pypy3-test-ext-redis ; Coverage is temporarily disabled for pypy3 due to the pytest bug. ; pypy3-coverage @@ -136,7 +136,7 @@ changedir = test-example-basic-tracer: docs/examples/basic_tracer/tests test-example-http: docs/examples/http/tests test-opentracing-shim: ext/opentelemetry-ext-opentracing-shim/tests - test-instrumentation-redis: ext/opentelemetry-instrumentation-redis/tests + test-ext-redis: ext/opentelemetry-ext-redis/tests commands_pre = ; Install without -e to test the actual installation @@ -185,7 +185,7 @@ commands_pre = pymysql: pip install {toxinidir}/ext/opentelemetry-ext-pymysql redis: pip install {toxinidir}/opentelemetry-auto-instrumentation - redis: pip install {toxinidir}/ext/opentelemetry-instrumentation-redis[test] + redis: pip install {toxinidir}/ext/opentelemetry-ext-redis[test] http-requests: pip install {toxinidir}/ext/opentelemetry-ext-http-requests[test] @@ -303,7 +303,7 @@ commands_pre = -e {toxinidir}/ext/opentelemetry-ext-psycopg2 \ -e {toxinidir}/ext/opentelemetry-ext-pymongo \ -e {toxinidir}/ext/opentelemetry-ext-pymysql \ - -e {toxinidir}/ext/opentelemetry-instrumentation-redis + -e {toxinidir}/ext/opentelemetry-ext-redis docker-compose up -d python check_availability.py commands = From 27127328ff05fd8e72a35b6c6c29ed1d450cca54 Mon Sep 17 00:00:00 2001 From: alrex Date: Fri, 24 Apr 2020 15:43:36 -0700 Subject: [PATCH 24/32] Update ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py Co-Authored-By: Chris Kleinknecht --- .../src/opentelemetry/instrumentation/redis/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py index f2277361abc..2b52037c178 100644 --- a/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -15,8 +15,8 @@ """ Instrument `redis`_ to report Redis queries. -There are two options for instrumenting code. The first option is to use -the `opentelemetry-auto-instrumentation` executable which will automatically +There are two options for instrumenting code. The first option is to use the +``opentelemetry-auto-instrumentation`` executable which will automatically instrument your Redis client. The second is to programmatically enable instrumentation via the following code: From 890703242e26daf628f0e9952e38edaafce91cb1 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 24 Apr 2020 16:04:35 -0700 Subject: [PATCH 25/32] fixing exception in check_availability --- ext/opentelemetry-ext-docker-tests/tests/check_availability.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/check_availability.py b/ext/opentelemetry-ext-docker-tests/tests/check_availability.py index 0545e1efe1a..91b8e5539dc 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/check_availability.py +++ b/ext/opentelemetry-ext-docker-tests/tests/check_availability.py @@ -58,7 +58,7 @@ def wrapper(): ex, ) time.sleep(RETRY_INTERVAL) - raise Exception("waiting for %s failed") + raise Exception("waiting for {} failed".format(func.__name__)) return wrapper From 5ee71de90f13137d3c326f3ae6add9aaaf8541d8 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 24 Apr 2020 16:23:24 -0700 Subject: [PATCH 26/32] finish renaming, remove patch.py --- docs/ext/redis/redis.rst | 2 +- .../tests/redis/test_redis_functional.py | 2 +- ext/opentelemetry-ext-redis/setup.cfg | 2 +- ext/opentelemetry-ext-redis/setup.py | 2 +- .../src/opentelemetry/ext/redis/__init__.py | 164 ++++++++++++++++++ .../{instrumentation => ext}/redis/util.py | 0 .../{instrumentation => ext}/redis/version.py | 0 .../instrumentation/redis/__init__.py | 61 ------- .../instrumentation/redis/patch.py | 121 ------------- .../tests/test_redis.py | 2 +- 10 files changed, 169 insertions(+), 187 deletions(-) create mode 100644 ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py rename ext/opentelemetry-ext-redis/src/opentelemetry/{instrumentation => ext}/redis/util.py (100%) rename ext/opentelemetry-ext-redis/src/opentelemetry/{instrumentation => ext}/redis/version.py (100%) delete mode 100644 ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py delete mode 100644 ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/patch.py diff --git a/docs/ext/redis/redis.rst b/docs/ext/redis/redis.rst index 4e21bce24b9..38a72ad52fc 100644 --- a/docs/ext/redis/redis.rst +++ b/docs/ext/redis/redis.rst @@ -1,7 +1,7 @@ OpenTelemetry Redis Instrumentation =================================== -.. automodule:: opentelemetry.instrumentation.redis +.. automodule:: opentelemetry.ext.redis :members: :undoc-members: :show-inheritance: diff --git a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py index 546bc88d839..5857f044e4b 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py @@ -15,7 +15,7 @@ import redis from opentelemetry import trace -from opentelemetry.instrumentation.redis import RedisInstrumentor +from opentelemetry.ext.redis import RedisInstrumentor from opentelemetry.test.test_base import TestBase diff --git a/ext/opentelemetry-ext-redis/setup.cfg b/ext/opentelemetry-ext-redis/setup.cfg index 5a8e3374156..e24605c8e35 100644 --- a/ext/opentelemetry-ext-redis/setup.cfg +++ b/ext/opentelemetry-ext-redis/setup.cfg @@ -55,4 +55,4 @@ where = src [options.entry_points] opentelemetry_instrumentor = - redis = opentelemetry.instrumentation.redis:RedisInstrumentor + redis = opentelemetry.ext.redis:RedisInstrumentor diff --git a/ext/opentelemetry-ext-redis/setup.py b/ext/opentelemetry-ext-redis/setup.py index df80a8fd1aa..d09847efd81 100644 --- a/ext/opentelemetry-ext-redis/setup.py +++ b/ext/opentelemetry-ext-redis/setup.py @@ -17,7 +17,7 @@ BASE_DIR = os.path.dirname(__file__) VERSION_FILENAME = os.path.join( - BASE_DIR, "src", "opentelemetry", "instrumentation", "redis", "version.py" + BASE_DIR, "src", "opentelemetry", "ext", "redis", "version.py" ) PACKAGE_INFO = {} with open(VERSION_FILENAME) as f: diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py new file mode 100644 index 00000000000..f1945db07ec --- /dev/null +++ b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py @@ -0,0 +1,164 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +""" +Instrument `redis`_ to report Redis queries. + +There are two options for instrumenting code. The first option is to use the +``opentelemetry-auto-instrumentation`` executable which will automatically +instrument your Redis client. The second is to programmatically enable +instrumentation via the following code: + +.. _redis: https://pypi.org/project/redis/ + +Usage +----- + +.. code:: python + + from opentelemetry import trace + from opentelemetry.ext.redis import RedisInstrumentor + from opentelemetry.sdk.trace import TracerProvider + import redis + + trace.set_tracer_provider(TracerProvider()) + + # Instrument redis + RedisInstrumentor().instrument(tracer_provider=trace.get_tracer_provider()) + + # This will report a span with the default settings + client = redis.StrictRedis(host="localhost", port=6379) + client.get("my-key") + +API +--- +""" + +import redis +from wrapt import ObjectProxy, wrap_function_wrapper + +from opentelemetry import trace +from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor + +from opentelemetry.ext.redis.util import ( + _extract_conn_attributes, + _format_command_args, +) +from opentelemetry.ext.redis.version import __version__ + +_DEFAULT_SERVICE = "redis" +_RAWCMD = "db.statement" +_CMD = "redis.command" + + +def _set_connection_attributes(span, conn): + for key, value in _extract_conn_attributes( + conn.connection_pool.connection_kwargs + ).items(): + span.set_attribute(key, value) + + +def _unwrap(obj, attr): + func = getattr(obj, attr, None) + if isinstance(func, ObjectProxy) and hasattr(func, "__wrapped__"): + setattr(obj, attr, func.__wrapped__) + + +def _traced_execute_command(func, instance, args, kwargs): + tracer = getattr(redis, "_opentelemetry_tracer") + query = _format_command_args(args) + with tracer.start_as_current_span(_CMD) as span: + span.set_attribute("service", tracer.instrumentation_info.name) + span.set_attribute(_RAWCMD, query) + _set_connection_attributes(span, instance) + span.set_attribute("redis.args_length", len(args)) + return func(*args, **kwargs) + + +def _traced_execute_pipeline(func, instance, args, kwargs): + tracer = getattr(redis, "_opentelemetry_tracer") + + cmds = [_format_command_args(c) for c, _ in instance.command_stack] + resource = "\n".join(cmds) + + with tracer.start_as_current_span(_CMD) as span: + span.set_attribute("service", tracer.instrumentation_info.name) + span.set_attribute(_RAWCMD, resource) + _set_connection_attributes(span, instance) + span.set_attribute( + "redis.pipeline_length", len(instance.command_stack) + ) + return func(*args, **kwargs) + + +class RedisInstrumentor(BaseInstrumentor): + """An instrumentor for Redis + See `BaseInstrumentor` + """ + + def _instrument(self, **kwargs): + tracer_provider = kwargs.get( + "tracer_provider", trace.get_tracer_provider() + ) + setattr( + redis, + "_opentelemetry_tracer", + tracer_provider.get_tracer(_DEFAULT_SERVICE, __version__), + ) + + if redis.VERSION < (3, 0, 0): + wrap_function_wrapper( + "redis", "StrictRedis.execute_command", _traced_execute_command + ) + wrap_function_wrapper( + "redis.client", + "BasePipeline.execute", + _traced_execute_pipeline, + ) + wrap_function_wrapper( + "redis.client", + "BasePipeline.immediate_execute_command", + _traced_execute_command, + ) + else: + wrap_function_wrapper( + "redis", "Redis.execute_command", _traced_execute_command + ) + wrap_function_wrapper( + "redis.client", "Pipeline.execute", _traced_execute_pipeline + ) + wrap_function_wrapper( + "redis.client", + "Pipeline.immediate_execute_command", + _traced_execute_command, + ) + + def _uninstrument(self, **kwargs): + if redis.VERSION < (3, 0, 0): + _unwrap(redis.StrictRedis, "execute_command") + _unwrap(redis.StrictRedis, "pipeline") + _unwrap(redis.Redis, "pipeline") + _unwrap( + redis.client.BasePipeline, + "execute", # pylint:disable=no-member + ) + _unwrap( + redis.client.BasePipeline, # pylint:disable=no-member + "immediate_execute_command", + ) + else: + _unwrap(redis.Redis, "execute_command") + _unwrap(redis.Redis, "pipeline") + _unwrap(redis.client.Pipeline, "execute") + _unwrap(redis.client.Pipeline, "immediate_execute_command") diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/util.py b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/util.py similarity index 100% rename from ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/util.py rename to ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/util.py diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/version.py b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/version.py similarity index 100% rename from ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/version.py rename to ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/version.py diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py b/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py deleted file mode 100644 index 2b52037c178..00000000000 --- a/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ /dev/null @@ -1,61 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -""" -Instrument `redis`_ to report Redis queries. - -There are two options for instrumenting code. The first option is to use the -``opentelemetry-auto-instrumentation`` executable which will automatically -instrument your Redis client. The second is to programmatically enable -instrumentation via the following code: - -.. _redis: https://pypi.org/project/redis/ - -Usage ------ - -.. code:: python - - from opentelemetry import trace - from opentelemetry.instrumentation.redis import RedisInstrumentor - from opentelemetry.sdk.trace import TracerProvider - import redis - - trace.set_tracer_provider(TracerProvider()) - - # Instrument redis - RedisInstrumentor().instrument(tracer_provider=trace.get_tracer_provider()) - - # This will report a span with the default settings - client = redis.StrictRedis(host="localhost", port=6379) - client.get("my-key") - -API ---- -""" -from opentelemetry import trace -from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor -from opentelemetry.instrumentation.redis.patch import _patch, _unpatch - - -class RedisInstrumentor(BaseInstrumentor): - """An instrumentor for Redis - See `BaseInstrumentor` - """ - - def _instrument(self, **kwargs): - _patch(kwargs.get("tracer_provider", trace.get_tracer_provider())) - - def _uninstrument(self, **kwargs): - _unpatch() diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/patch.py b/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/patch.py deleted file mode 100644 index 42da4a501ea..00000000000 --- a/ext/opentelemetry-ext-redis/src/opentelemetry/instrumentation/redis/patch.py +++ /dev/null @@ -1,121 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -# pylint:disable=relative-beyond-top-level -import redis -from wrapt import ObjectProxy, wrap_function_wrapper - -from .util import _extract_conn_attributes, _format_command_args -from .version import __version__ - -_DEFAULT_SERVICE = "redis" -_RAWCMD = "db.statement" -_CMD = "redis.command" - - -def _patch(tracer_provider): - """Patch the instrumented methods - - This duplicated doesn't look nice. The nicer alternative is to use an ObjectProxy on top - of Redis and StrictRedis. However, it means that any "import redis.Redis" won't be instrumented. - """ - setattr( - redis, - "_opentelemetry_tracer", - tracer_provider.get_tracer(_DEFAULT_SERVICE, __version__), - ) - - if redis.VERSION < (3, 0, 0): - wrap_function_wrapper( - "redis", "StrictRedis.execute_command", traced_execute_command - ) - wrap_function_wrapper( - "redis.client", "BasePipeline.execute", traced_execute_pipeline - ) - wrap_function_wrapper( - "redis.client", - "BasePipeline.immediate_execute_command", - traced_execute_command, - ) - else: - wrap_function_wrapper( - "redis", "Redis.execute_command", traced_execute_command - ) - wrap_function_wrapper( - "redis.client", "Pipeline.execute", traced_execute_pipeline - ) - wrap_function_wrapper( - "redis.client", - "Pipeline.immediate_execute_command", - traced_execute_command, - ) - - -def _unwrap(obj, attr): - func = getattr(obj, attr, None) - if isinstance(func, ObjectProxy) and hasattr(func, "__wrapped__"): - setattr(obj, attr, func.__wrapped__) - - -def _unpatch(): - if redis.VERSION < (3, 0, 0): - _unwrap(redis.StrictRedis, "execute_command") - _unwrap(redis.StrictRedis, "pipeline") - _unwrap(redis.Redis, "pipeline") - _unwrap( - redis.client.BasePipeline, "execute", # pylint:disable=no-member - ) - _unwrap( - redis.client.BasePipeline, # pylint:disable=no-member - "immediate_execute_command", - ) - else: - _unwrap(redis.Redis, "execute_command") - _unwrap(redis.Redis, "pipeline") - _unwrap(redis.client.Pipeline, "execute") - _unwrap(redis.client.Pipeline, "immediate_execute_command") - - -def traced_execute_command(func, instance, args, kwargs): - tracer = getattr(redis, "_opentelemetry_tracer") - query = _format_command_args(args) - with tracer.start_as_current_span(_CMD) as span: - span.set_attribute("service", tracer.instrumentation_info.name) - span.set_attribute(_RAWCMD, query) - _set_connection_attributes(span, instance) - span.set_attribute("redis.args_length", len(args)) - return func(*args, **kwargs) - - -def traced_execute_pipeline(func, instance, args, kwargs): - tracer = getattr(redis, "_opentelemetry_tracer") - - cmds = [_format_command_args(c) for c, _ in instance.command_stack] - resource = "\n".join(cmds) - - with tracer.start_as_current_span(_CMD) as span: - span.set_attribute("service", tracer.instrumentation_info.name) - span.set_attribute(_RAWCMD, resource) - _set_connection_attributes(span, instance) - span.set_attribute( - "redis.pipeline_length", len(instance.command_stack) - ) - return func(*args, **kwargs) - - -def _set_connection_attributes(span, conn): - for key, value in _extract_conn_attributes( - conn.connection_pool.connection_kwargs - ).items(): - span.set_attribute(key, value) diff --git a/ext/opentelemetry-ext-redis/tests/test_redis.py b/ext/opentelemetry-ext-redis/tests/test_redis.py index 072e4bf6a8e..1354d1faa02 100644 --- a/ext/opentelemetry-ext-redis/tests/test_redis.py +++ b/ext/opentelemetry-ext-redis/tests/test_redis.py @@ -15,7 +15,7 @@ import redis -from opentelemetry.instrumentation.redis import RedisInstrumentor +from opentelemetry.ext.redis import RedisInstrumentor from opentelemetry.test.test_base import TestBase From 2faca4815c1ee700b4d81c9273af5fef5d86700b Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 24 Apr 2020 16:48:11 -0700 Subject: [PATCH 27/32] fix lint --- .../src/opentelemetry/ext/redis/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py index f1945db07ec..26e94b275a4 100644 --- a/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py +++ b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py @@ -50,7 +50,6 @@ from opentelemetry import trace from opentelemetry.auto_instrumentation.instrumentor import BaseInstrumentor - from opentelemetry.ext.redis.util import ( _extract_conn_attributes, _format_command_args, From f372d1e54c9ac8751a6d1965bb6d7f1bf0c12443 Mon Sep 17 00:00:00 2001 From: alrex Date: Fri, 24 Apr 2020 16:48:59 -0700 Subject: [PATCH 28/32] Apply suggestions from code review Co-Authored-By: Diego Hurtado --- .../tests/redis/test_redis_functional.py | 2 +- ext/opentelemetry-ext-redis/tests/test_redis.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py index 5857f044e4b..71fccda7197 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py @@ -19,7 +19,7 @@ from opentelemetry.test.test_base import TestBase -class TestRedisPatch(TestBase): +class TestRedisInstrument(TestBase): test_service = "redis" diff --git a/ext/opentelemetry-ext-redis/tests/test_redis.py b/ext/opentelemetry-ext-redis/tests/test_redis.py index 1354d1faa02..b36e96a6278 100644 --- a/ext/opentelemetry-ext-redis/tests/test_redis.py +++ b/ext/opentelemetry-ext-redis/tests/test_redis.py @@ -20,7 +20,7 @@ class TestRedis(TestBase): - def test_patch_unpatch(self): + def test_instrument_uninstrument(self): redis_client = redis.Redis() RedisInstrumentor().instrument(tracer_provider=self.tracer_provider) @@ -31,7 +31,7 @@ def test_patch_unpatch(self): self.assertEqual(len(spans), 1) self.memory_exporter.clear() - # Test unpatch + # Test uninstrument RedisInstrumentor().uninstrument() with mock.patch.object(redis_client, "connection"): @@ -41,7 +41,7 @@ def test_patch_unpatch(self): self.assertEqual(len(spans), 0) self.memory_exporter.clear() - # Test patch again + # Test instrument again RedisInstrumentor().instrument() with mock.patch.object(redis_client, "connection"): From 87eceb6abc44882bcacd55c872db525e39ca64aa Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Fri, 24 Apr 2020 21:26:52 -0700 Subject: [PATCH 29/32] sdk: span parents are now always spancontext (#548) Exporter and span handling complexity was increasing due to the Span.parent attribute being either a Span or a SpanContext. As there is no requirement in the specification to have the parent attribute be a Span, removing this complexity and handling. Co-authored-by: Chris Kleinknecht Co-authored-by: Alex Boten --- .../tests/mysql/test_mysql_functional.py | 2 +- .../tests/postgres/test_psycopg_functional.py | 2 +- .../tests/pymongo/test_pymongo_functional.py | 2 +- .../tests/pymysql/test_pymysql_functional.py | 2 +- .../src/opentelemetry/ext/jaeger/__init__.py | 6 +----- .../tests/test_shim.py | 11 ++++++++--- .../ext/otcollector/trace_exporter/__init__.py | 17 ++--------------- .../tests/test_otcollector_trace_exporter.py | 5 ++++- .../src/opentelemetry/sdk/trace/__init__.py | 10 +++++----- opentelemetry-sdk/tests/context/test_asyncio.py | 2 +- opentelemetry-sdk/tests/trace/test_trace.py | 4 ++-- 11 files changed, 27 insertions(+), 36 deletions(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/mysql/test_mysql_functional.py b/ext/opentelemetry-ext-docker-tests/tests/mysql/test_mysql_functional.py index 36fd7bf3d3c..d0261d2f63f 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/mysql/test_mysql_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/mysql/test_mysql_functional.py @@ -65,7 +65,7 @@ def validate_spans(self): self.assertEqual(root_span.name, "rootSpan") self.assertEqual(db_span.name, "mysql.opentelemetry-tests") self.assertIsNotNone(db_span.parent) - self.assertEqual(db_span.parent.name, root_span.name) + self.assertIs(db_span.parent, root_span.get_context()) self.assertIs(db_span.kind, trace_api.SpanKind.CLIENT) self.assertEqual(db_span.attributes["db.instance"], MYSQL_DB_NAME) self.assertEqual(db_span.attributes["net.peer.name"], MYSQL_HOST) diff --git a/ext/opentelemetry-ext-docker-tests/tests/postgres/test_psycopg_functional.py b/ext/opentelemetry-ext-docker-tests/tests/postgres/test_psycopg_functional.py index c53baf81a62..0a43926145d 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/postgres/test_psycopg_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/postgres/test_psycopg_functional.py @@ -68,7 +68,7 @@ def validate_spans(self): self.assertEqual(root_span.name, "rootSpan") self.assertEqual(child_span.name, "postgresql.opentelemetry-tests") self.assertIsNotNone(child_span.parent) - self.assertEqual(child_span.parent.name, root_span.name) + self.assertIs(child_span.parent, root_span.get_context()) self.assertIs(child_span.kind, trace_api.SpanKind.CLIENT) self.assertEqual( child_span.attributes["db.instance"], POSTGRES_DB_NAME diff --git a/ext/opentelemetry-ext-docker-tests/tests/pymongo/test_pymongo_functional.py b/ext/opentelemetry-ext-docker-tests/tests/pymongo/test_pymongo_functional.py index 567e0d86a0e..1b2e2694829 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/pymongo/test_pymongo_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/pymongo/test_pymongo_functional.py @@ -51,7 +51,7 @@ def validate_spans(self): self.assertIsNot(root_span, None) self.assertIsNot(pymongo_span, None) self.assertIsNotNone(pymongo_span.parent) - self.assertEqual(pymongo_span.parent.name, root_span.name) + self.assertIs(pymongo_span.parent, root_span.get_context()) self.assertIs(pymongo_span.kind, trace_api.SpanKind.CLIENT) self.assertEqual( pymongo_span.attributes["db.instance"], MONGODB_DB_NAME diff --git a/ext/opentelemetry-ext-docker-tests/tests/pymysql/test_pymysql_functional.py b/ext/opentelemetry-ext-docker-tests/tests/pymysql/test_pymysql_functional.py index e4ee65af30f..a7dc2136558 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/pymysql/test_pymysql_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/pymysql/test_pymysql_functional.py @@ -76,7 +76,7 @@ def validate_spans(self): self.assertEqual(root_span.name, "rootSpan") self.assertEqual(db_span.name, "mysql.opentelemetry-tests") self.assertIsNotNone(db_span.parent) - self.assertEqual(db_span.parent.name, root_span.name) + self.assertIs(db_span.parent, root_span.get_context()) self.assertIs(db_span.kind, trace_api.SpanKind.CLIENT) self.assertEqual(db_span.attributes["db.instance"], MYSQL_DB_NAME) self.assertEqual(db_span.attributes["net.peer.name"], MYSQL_HOST) diff --git a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py index 8a8d40b260c..23ed6e725d8 100644 --- a/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py +++ b/ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/__init__.py @@ -194,11 +194,7 @@ def _translate_to_jaeger(spans: Span): status = span.status - parent_id = 0 - if isinstance(span.parent, trace_api.Span): - parent_id = span.parent.get_context().span_id - elif isinstance(span.parent, trace_api.SpanContext): - parent_id = span.parent.span_id + parent_id = span.parent.span_id if span.parent else 0 tags = _extract_tags(span.attributes) diff --git a/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py b/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py index 75e98860798..7a0913f973b 100644 --- a/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py +++ b/ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py @@ -281,7 +281,8 @@ def test_parent_child_implicit(self): self.assertEqual(parent_trace_id, child_trace_id) self.assertEqual( - child.span.unwrap().parent, parent.span.unwrap() + child.span.unwrap().parent, + parent.span.unwrap().get_context(), ) # Verify parent span becomes the active span again. @@ -309,7 +310,9 @@ def test_parent_child_explicit_span(self): child_trace_id = child.span.unwrap().get_context().trace_id self.assertEqual(child_trace_id, parent_trace_id) - self.assertEqual(child.span.unwrap().parent, parent.unwrap()) + self.assertEqual( + child.span.unwrap().parent, parent.unwrap().get_context() + ) with self.shim.start_span("ParentSpan") as parent: child = self.shim.start_span("ChildSpan", child_of=parent) @@ -318,7 +321,9 @@ def test_parent_child_explicit_span(self): child_trace_id = child.unwrap().get_context().trace_id self.assertEqual(child_trace_id, parent_trace_id) - self.assertEqual(child.unwrap().parent, parent.unwrap()) + self.assertEqual( + child.unwrap().parent, parent.unwrap().get_context() + ) child.finish() diff --git a/ext/opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/trace_exporter/__init__.py b/ext/opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/trace_exporter/__init__.py index d287bcfb4a8..fb6237e86d8 100644 --- a/ext/opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/trace_exporter/__init__.py +++ b/ext/opentelemetry-ext-otcollector/src/opentelemetry/ext/otcollector/trace_exporter/__init__.py @@ -109,9 +109,7 @@ def translate_to_collector(spans: Sequence[Span]): ) parent_id = 0 - if isinstance(span.parent, trace_api.Span): - parent_id = span.parent.get_context().span_id - elif isinstance(span.parent, trace_api.SpanContext): + if span.parent is not None: parent_id = span.parent.span_id collector_span.parent_span_id = parent_id.to_bytes(8, "big") @@ -157,18 +155,7 @@ def translate_to_collector(spans: Sequence[Span]): collector_span_link.type = ( trace_pb2.Span.Link.Type.TYPE_UNSPECIFIED ) - - if isinstance(span.parent, trace_api.Span): - if ( - link.context.span_id - == span.parent.get_context().span_id - and link.context.trace_id - == span.parent.get_context().trace_id - ): - collector_span_link.type = ( - trace_pb2.Span.Link.Type.PARENT_LINKED_SPAN - ) - elif isinstance(span.parent, trace_api.SpanContext): + if span.parent is not None: if ( link.context.span_id == span.parent.span_id and link.context.trace_id == span.parent.trace_id diff --git a/ext/opentelemetry-ext-otcollector/tests/test_otcollector_trace_exporter.py b/ext/opentelemetry-ext-otcollector/tests/test_otcollector_trace_exporter.py index 74c1d088720..4a0a556137d 100644 --- a/ext/opentelemetry-ext-otcollector/tests/test_otcollector_trace_exporter.py +++ b/ext/opentelemetry-ext-otcollector/tests/test_otcollector_trace_exporter.py @@ -135,7 +135,10 @@ def test_translate_to_collector(self): kind=trace_api.SpanKind.SERVER, ) span_3 = trace.Span( - name="test3", context=other_context, links=(link_2,), parent=span_2 + name="test3", + context=other_context, + links=(link_2,), + parent=span_2.get_context(), ) otel_spans = [span_1, span_2, span_3] otel_spans[0].start(start_time=start_times[0]) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 3d2fc96c1b8..5eff5e61307 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -198,8 +198,8 @@ class Span(trace_api.Span): Args: name: The name of the operation this span represents context: The immutable span context - parent: This span's parent, may be a `SpanContext` if the parent is - remote, null if this is a root span + parent: This span's parent's `SpanContext`, or + null if this is a root span sampler: The sampler used to create this span trace_config: TODO resource: Entity producing telemetry @@ -219,7 +219,7 @@ def __init__( self, name: str, context: trace_api.SpanContext, - parent: trace_api.ParentSpan = None, + parent: Optional[trace_api.SpanContext] = None, sampler: Optional[sampling.Sampler] = None, trace_config: None = None, # TODO resource: None = None, @@ -594,7 +594,7 @@ def start_span( # pylint: disable=too-many-locals if parent_context is not None and not isinstance( parent_context, trace_api.SpanContext ): - raise TypeError + raise TypeError("parent must be a Span, SpanContext or None.") if parent_context is None or not parent_context.is_valid(): parent = parent_context = None @@ -640,7 +640,7 @@ def start_span( # pylint: disable=too-many-locals span = Span( name=name, context=context, - parent=parent, + parent=parent_context, sampler=self.source.sampler, resource=self.source.resource, attributes=span_attributes, diff --git a/opentelemetry-sdk/tests/context/test_asyncio.py b/opentelemetry-sdk/tests/context/test_asyncio.py index f213753c59f..4fa653205b0 100644 --- a/opentelemetry-sdk/tests/context/test_asyncio.py +++ b/opentelemetry-sdk/tests/context/test_asyncio.py @@ -108,4 +108,4 @@ def test_with_asyncio(self): for span in span_list: if span is expected_parent: continue - self.assertEqual(span.parent, expected_parent) + self.assertEqual(span.parent, expected_parent.get_context()) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index f9db925f11c..1094f1afb98 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -243,7 +243,7 @@ def test_start_span_implicit(self): with tracer.start_span( "child", kind=trace_api.SpanKind.CLIENT ) as child: - self.assertIs(child.parent, root) + self.assertIs(child.parent, root.get_context()) self.assertEqual(child.kind, trace_api.SpanKind.CLIENT) self.assertIsNotNone(child.start_time) @@ -332,7 +332,7 @@ def test_start_as_current_span_implicit(self): with tracer.start_as_current_span("child") as child: self.assertIs(tracer.get_current_span(), child) - self.assertIs(child.parent, root) + self.assertIs(child.parent, root.get_context()) # After exiting the child's scope the parent should become the # current span again. From a10916bfd71429f3beed7cef23112360892408b7 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Sun, 26 Apr 2020 14:50:26 -0700 Subject: [PATCH 30/32] fix lint --- .../src/opentelemetry/ext/redis/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py index 26e94b275a4..e653936ca54 100644 --- a/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py +++ b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/__init__.py @@ -149,8 +149,8 @@ def _uninstrument(self, **kwargs): _unwrap(redis.StrictRedis, "pipeline") _unwrap(redis.Redis, "pipeline") _unwrap( - redis.client.BasePipeline, - "execute", # pylint:disable=no-member + redis.client.BasePipeline, # pylint:disable=no-member + "execute", ) _unwrap( redis.client.BasePipeline, # pylint:disable=no-member From 0757069d111b694893b988c23a0e0f3b06d84100 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Sun, 26 Apr 2020 14:50:37 -0700 Subject: [PATCH 31/32] refactor tests --- .../tests/redis/test_redis_functional.py | 47 +++++-------------- 1 file changed, 12 insertions(+), 35 deletions(-) diff --git a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py index 71fccda7197..45283c442cd 100644 --- a/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py +++ b/ext/opentelemetry-ext-docker-tests/tests/redis/test_redis_functional.py @@ -33,23 +33,24 @@ def tearDown(self): super().tearDown() RedisInstrumentor().uninstrument() - def test_long_command(self): - self.redis_client.mget(*range(1000)) - - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 1) - span = spans[0] + def _check_span(self, span): self.assertEqual(span.attributes["service"], self.test_service) self.assertEqual(span.name, "redis.command") self.assertIs( span.status.canonical_code, trace.status.StatusCanonicalCode.OK ) - self.assertEqual(span.attributes.get("db.instance"), 0) self.assertEqual( span.attributes.get("db.url"), "redis://localhost:6379" ) + def test_long_command(self): + self.redis_client.mget(*range(1000)) + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + span = spans[0] + self._check_span(span) self.assertTrue( span.attributes.get("db.statement").startswith("MGET 0 1 2 3") ) @@ -60,15 +61,7 @@ def test_basics(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.attributes["service"], self.test_service) - self.assertEqual(span.name, "redis.command") - self.assertIs( - span.status.canonical_code, trace.status.StatusCanonicalCode.OK - ) - self.assertEqual(span.attributes.get("db.instance"), 0) - self.assertEqual( - span.attributes.get("db.url"), "redis://localhost:6379" - ) + self._check_span(span) self.assertEqual(span.attributes.get("db.statement"), "GET cheese") self.assertEqual(span.attributes.get("redis.args_length"), 2) @@ -82,15 +75,7 @@ def test_pipeline_traced(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.attributes["service"], self.test_service) - self.assertEqual(span.name, "redis.command") - self.assertIs( - span.status.canonical_code, trace.status.StatusCanonicalCode.OK - ) - self.assertEqual(span.attributes.get("db.instance"), 0) - self.assertEqual( - span.attributes.get("db.url"), "redis://localhost:6379" - ) + self._check_span(span) self.assertEqual( span.attributes.get("db.statement"), "SET blah 32\nRPUSH foo éé\nHGETALL xxx", @@ -108,16 +93,8 @@ def test_pipeline_immediate(self): # single span for the whole pipeline self.assertEqual(len(spans), 2) span = spans[0] - self.assertEqual(span.attributes["service"], self.test_service) - self.assertEqual(span.name, "redis.command") + self._check_span(span) self.assertEqual(span.attributes.get("db.statement"), "SET b 2") - self.assertIs( - span.status.canonical_code, trace.status.StatusCanonicalCode.OK - ) - self.assertEqual(span.attributes.get("db.instance"), 0) - self.assertEqual( - span.attributes.get("db.url"), "redis://localhost:6379" - ) def test_parent(self): """Ensure OpenTelemetry works with redis.""" @@ -132,7 +109,7 @@ def test_parent(self): # confirm the parenting self.assertIsNone(parent_span.parent) - self.assertIs(child_span.parent, parent_span) + self.assertIs(child_span.parent, parent_span.get_context()) self.assertEqual(parent_span.name, "redis_get") self.assertEqual(parent_span.instrumentation_info.name, "redis_svc") From e4d908404d5ee3fd1881f918409c4254bbe34197 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Sun, 26 Apr 2020 14:59:07 -0700 Subject: [PATCH 32/32] set attributes that are available --- .../src/opentelemetry/ext/redis/util.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/util.py b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/util.py index 4dd83c8888b..28951340893 100644 --- a/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/util.py +++ b/ext/opentelemetry-ext-redis/src/opentelemetry/ext/redis/util.py @@ -19,16 +19,18 @@ def _extract_conn_attributes(conn_kwargs): """ Transform redis conn info into dict """ + attributes = { + "db.type": "redis", + "db.instance": conn_kwargs.get("db", 0), + } try: - return { - "db.type": "redis", - "db.url": "redis://{}:{}".format( - conn_kwargs["host"], conn_kwargs["port"] - ), - "db.instance": conn_kwargs["db"] or 0, - } + attributes["db.url"] = "redis://{}:{}".format( + conn_kwargs["host"], conn_kwargs["port"] + ) except KeyError: - return {} + pass # don't include url attribute + + return attributes def _format_command_args(args):