From ee90c7e50c7dd16d335d56253114f485b6536a08 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sun, 15 Nov 2020 02:10:34 +0530 Subject: [PATCH 1/6] Update redis instrumentation to follow semantic conventions --- .../opentelemetry/instrumentation/redis/__init__.py | 8 +++++--- .../src/opentelemetry/instrumentation/redis/util.py | 13 +++++++------ .../tests/test_redis.py | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index e1c5db1e94..f8ec42c0cc 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -59,7 +59,6 @@ _DEFAULT_SERVICE = "redis" _RAWCMD = "db.statement" -_CMD = "redis.command" def _set_connection_attributes(span, conn): @@ -75,7 +74,7 @@ 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, kind=trace.SpanKind.CLIENT + args[0], kind=trace.SpanKind.CLIENT ) as span: if span.is_recording(): span.set_attribute("service", tracer.instrumentation_info.name) @@ -91,8 +90,11 @@ def _traced_execute_pipeline(func, instance, args, kwargs): cmds = [_format_command_args(c) for c, _ in instance.command_stack] resource = "\n".join(cmds) + pipeline_cmnds = " ".join([args[0] for args, _ in instance.command_stack]) + span_name = "PIPELINE" + " " + pipeline_cmnds + with tracer.start_as_current_span( - _CMD, kind=trace.SpanKind.CLIENT + span_name, kind=trace.SpanKind.CLIENT ) as span: if span.is_recording(): span.set_attribute("service", tracer.instrumentation_info.name) diff --git a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py index 2895134089..f2ac71a360 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py +++ b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py @@ -20,15 +20,16 @@ def _extract_conn_attributes(conn_kwargs): """ Transform redis conn info into dict """ attributes = { - "db.type": "redis", - "db.instance": conn_kwargs.get("db", 0), + "db.system": "redis", + "db.name": conn_kwargs.get("db", 0), } try: - attributes["db.url"] = "redis://{}:{}".format( - conn_kwargs["host"], conn_kwargs["port"] - ) + attributes["net.peer.name"] = conn_kwargs["host"] + attributes["net.peer.ip"] = conn_kwargs["port"] + attributes["net.transport"] = "IP.TCP" except KeyError: - pass # don't include url attribute + attributes["net.peer.name"] = conn_kwargs["path"] + attributes["net.transport"] = "Unix" return attributes diff --git a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py index 3e07ac725e..0ca99beadb 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py +++ b/instrumentation/opentelemetry-instrumentation-redis/tests/test_redis.py @@ -31,7 +31,7 @@ def test_span_properties(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self.assertEqual(span.name, "redis.command") + self.assertEqual(span.name, "GET") self.assertEqual(span.kind, SpanKind.CLIENT) def test_not_recording(self): From 6f2d60cb12db3b693c9b21505d1f0cf5563c5258 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sun, 15 Nov 2020 02:10:57 +0530 Subject: [PATCH 2/6] Fix docker tests --- .../tests/redis/test_redis_functional.py | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py b/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py index 8bdc120105..749ee10446 100644 --- a/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py +++ b/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py @@ -33,14 +33,13 @@ def tearDown(self): super().tearDown() RedisInstrumentor().uninstrument() - def _check_span(self, span): + def _check_span(self, span, name): self.assertEqual(span.attributes["service"], self.test_service) - self.assertEqual(span.name, "redis.command") + self.assertEqual(span.name, name) self.assertIs(span.status.status_code, trace.status.StatusCode.UNSET) - self.assertEqual(span.attributes.get("db.instance"), 0) - self.assertEqual( - span.attributes.get("db.url"), "redis://localhost:6379" - ) + self.assertEqual(span.attributes.get("db.name"), 0) + self.assertEqual(span.attributes["net.peer.name"], "localhost") + self.assertEqual(span.attributes["net.peer.ip"], 6379) def test_long_command(self): self.redis_client.mget(*range(1000)) @@ -48,7 +47,7 @@ def test_long_command(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self._check_span(span) + self._check_span(span, "MGET") self.assertTrue( span.attributes.get("db.statement").startswith("MGET 0 1 2 3") ) @@ -59,7 +58,7 @@ def test_basics(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self._check_span(span) + self._check_span(span, "GET") self.assertEqual(span.attributes.get("db.statement"), "GET cheese") self.assertEqual(span.attributes.get("redis.args_length"), 2) @@ -73,7 +72,7 @@ def test_pipeline_traced(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self._check_span(span) + self._check_span(span, "PIPELINE SET RPUSH HGETALL") self.assertEqual( span.attributes.get("db.statement"), "SET blah 32\nRPUSH foo éé\nHGETALL xxx", @@ -91,7 +90,7 @@ def test_pipeline_immediate(self): # single span for the whole pipeline self.assertEqual(len(spans), 2) span = spans[0] - self._check_span(span) + self._check_span(span, "SET") self.assertEqual(span.attributes.get("db.statement"), "SET b 2") def test_parent(self): @@ -115,4 +114,4 @@ def test_parent(self): self.assertEqual( child_span.attributes.get("service"), self.test_service ) - self.assertEqual(child_span.name, "redis.command") + self.assertEqual(child_span.name, "GET") From edaf0b1611645539add7818189252d6a36d421a1 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 16 Nov 2020 15:02:37 +0530 Subject: [PATCH 3/6] Add CHANGELOG entry --- .../opentelemetry-instrumentation-redis/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md index 8f2d5f7e84..da4dee8c57 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Update redis instrumentation to follow semantic conventions ([#184](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/184)) + ## Version 0.13b0 Released 2020-09-17 From 857581ca51fc2e9e497d196fa018312c0e13dcac Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 16 Nov 2020 20:53:50 +0530 Subject: [PATCH 4/6] Remove PIPELINE from span name --- .../src/opentelemetry/instrumentation/redis/__init__.py | 3 +-- .../tests/redis/test_redis_functional.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index f8ec42c0cc..66a3c8468b 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -90,8 +90,7 @@ def _traced_execute_pipeline(func, instance, args, kwargs): cmds = [_format_command_args(c) for c, _ in instance.command_stack] resource = "\n".join(cmds) - pipeline_cmnds = " ".join([args[0] for args, _ in instance.command_stack]) - span_name = "PIPELINE" + " " + pipeline_cmnds + span_name = " ".join([args[0] for args, _ in instance.command_stack]) with tracer.start_as_current_span( span_name, kind=trace.SpanKind.CLIENT diff --git a/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py b/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py index 749ee10446..b893a33b61 100644 --- a/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py +++ b/tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py @@ -72,7 +72,7 @@ def test_pipeline_traced(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] - self._check_span(span, "PIPELINE SET RPUSH HGETALL") + self._check_span(span, "SET RPUSH HGETALL") self.assertEqual( span.attributes.get("db.statement"), "SET blah 32\nRPUSH foo éé\nHGETALL xxx", From ea6ed5c9f0e4a8f26b67b582e3239727c6416758 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Tue, 17 Nov 2020 22:08:49 +0530 Subject: [PATCH 5/6] Use db as fallback for span name --- .../src/opentelemetry/instrumentation/redis/__init__.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py index 948675029a..5bf83242ce 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py @@ -70,8 +70,13 @@ def _set_connection_attributes(span, conn): def _traced_execute_command(func, instance, args, kwargs): tracer = getattr(redis, "_opentelemetry_tracer") query = _format_command_args(args) + name = "" + if len(args) > 0 and args[0]: + name = args[0] + else: + name = instance.connection_pool.connection_kwargs.get("db", 0) with tracer.start_as_current_span( - args[0], kind=trace.SpanKind.CLIENT + name, kind=trace.SpanKind.CLIENT ) as span: if span.is_recording(): span.set_attribute("service", tracer.instrumentation_info.name) From 5224a966e1ddebc1ed589c54861c9d4dce0e8206 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 19 Nov 2020 12:49:13 +0530 Subject: [PATCH 6/6] Update CHANGELOG entry --- .../opentelemetry-instrumentation-redis/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md index da4dee8c57..5924a131e2 100644 --- a/instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md @@ -2,7 +2,8 @@ ## Unreleased -- Update redis instrumentation to follow semantic conventions ([#184](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/184)) +- Update redis instrumentation to follow semantic conventions + ([#184](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/184)) ## Version 0.13b0