Change name to _set_span_attribute_if_value, use constant for _FIELD_…

…TYPES, return when zero returned docs
open-telemetry · Aug 30, 2024 · 8c6c75d · 8c6c75d
1 parent 2ad9768
commit 8c6c75d
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 27 deletions.
diff --git a/...n/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py b/...n/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/__init__.py
@@ -102,7 +102,7 @@ def response_hook(span, instance, response):
 from opentelemetry.instrumentation.redis.util import (
     _extract_conn_attributes,
     _format_command_args,
-    _set_span_attribute,
+    _set_span_attribute_if_value,
     _value_or_none,
 )
 from opentelemetry.instrumentation.redis.version import __version__
@@ -128,6 +128,8 @@ def response_hook(span, instance, response):
 _REDIS_CLUSTER_VERSION = (4, 1, 0)
 _REDIS_ASYNCIO_CLUSTER_VERSION = (4, 3, 2)
 
+_FIELD_TYPES = ["NUMERIC", "TEXT", "GEO", "TAG", "VECTOR"]
+
 
 def _set_connection_attributes(span, conn):
     if not span.is_recording() or not hasattr(conn, "connection_pool"):
@@ -239,7 +241,7 @@ def _traced_execute_pipeline(func, instance, args, kwargs):
         return response
 
     def _add_create_attributes(span, args):
-        _set_span_attribute(
+        _set_span_attribute_if_value(
             span, "redis.create_index.index", _value_or_none(args, 1)
         )
         # According to: https://github.com/redis/redis-py/blob/master/redis/commands/search/commands.py#L155 schema is last argument for execute command
@@ -251,23 +253,22 @@ def _add_create_attributes(span, args):
         field_attribute = ""
         # Schema in format:
         # [first_field_name, first_field_type, first_field_some_attribute1, first_field_some_attribute2, second_field_name, ...]
-        field_types = ["NUMERIC", "TEXT", "GEO", "TAG", "VECTOR"]
-        for index in range(len(schema)):
-            if schema[index] in field_types:
-                field_attribute += (
-                    f"Field(name: {schema[index - 1]}, type: {schema[index]});"
-                )
-        _set_span_attribute(
+        field_attribute = "".join(
+            f"Field(name: {schema[index - 1]}, type: {schema[index]});"
+            for index in range(1, len(schema))
+            if schema[index] in _FIELD_TYPES
+        )
+        _set_span_attribute_if_value(
             span,
             "redis.create_index.fields",
             field_attribute,
         )
 
     def _add_search_attributes(span, response, args):
-        _set_span_attribute(
+        _set_span_attribute_if_value(
             span, "redis.search.index", _value_or_none(args, 1)
         )
-        _set_span_attribute(
+        _set_span_attribute_if_value(
             span, "redis.search.query", _value_or_none(args, 2)
         )
         # Parse response from search
@@ -277,24 +278,21 @@ def _add_search_attributes(span, response, args):
         # Returned documents in array format:
         # [first_field_name, first_field_value, second_field_name, second_field_value ...]
         number_of_returned_documents = _value_or_none(response, 0)
-        _set_span_attribute(
+        _set_span_attribute_if_value(
             span, "redis.search.total", number_of_returned_documents
         )
-        if "NOCONTENT" in args:
+        if "NOCONTENT" in args or not number_of_returned_documents:
             return
-        if number_of_returned_documents:
-            for document_number in range(number_of_returned_documents):
-                document_index = _value_or_none(
-                    response, 1 + 2 * document_number
-                )
-                if document_index:
-                    document = response[2 + 2 * document_number]
-                    for attribute_name_index in range(0, len(document), 2):
-                        _set_span_attribute(
-                            span,
-                            f"redis.search.xdoc_{document_index}.{document[attribute_name_index]}",
-                            document[attribute_name_index + 1],
-                        )
+        for document_number in range(number_of_returned_documents):
+            document_index = _value_or_none(response, 1 + 2 * document_number)
+            if document_index:
+                document = response[2 + 2 * document_number]
+                for attribute_name_index in range(0, len(document), 2):
+                    _set_span_attribute_if_value(
+                        span,
+                        f"redis.search.xdoc_{document_index}.{document[attribute_name_index]}",
+                        document[attribute_name_index + 1],
+                    )
 
     pipeline_class = (
         "BasePipeline" if redis.VERSION < (3, 0, 0) else "Pipeline"

diff --git a/...ation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py b/...ation/opentelemetry-instrumentation-redis/src/opentelemetry/instrumentation/redis/util.py
@@ -70,7 +70,7 @@ def _format_command_args(args):
     return out_str
 
 
-def _set_span_attribute(span, name, value):
+def _set_span_attribute_if_value(span, name, value):
     if value is not None and value != "":
         span.set_attribute(name, value)
     return