Merge pull request #184 from lonewolf3739/redis-semantic-conv

Update redis instrumentation to follow semantic conventions

instrumentation/opentelemetry-instrumentation-redis/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## 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

instrumentation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py

@@ -56,7 +56,6 @@
 
 _DEFAULT_SERVICE = "redis"
 _RAWCMD = "db.statement"
-_CMD = "redis.command"
 
 
 def _set_connection_attributes(span, conn):
@@ -71,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(
-        _CMD, kind=trace.SpanKind.CLIENT
+        name, kind=trace.SpanKind.CLIENT
     ) as span:
         if span.is_recording():
             span.set_attribute("service", tracer.instrumentation_info.name)
@@ -88,8 +92,10 @@ def _traced_execute_pipeline(func, instance, args, kwargs):
     cmds = [_format_command_args(c) for c, _ in instance.command_stack]
     resource = "\n".join(cmds)
 
+    span_name = " ".join([args[0] for args, _ in instance.command_stack])
+
     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)

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
 

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):

tests/opentelemetry-docker-tests/tests/redis/test_redis_functional.py

@@ -33,22 +33,21 @@ 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))
 
         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, "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")