Attributes make logger unique

Author: sfc-gh-jopel

opentelemetry-sdk/benchmarks/logs/test_benchmark_logs.py

@@ -32,7 +32,7 @@ def test_simple_get_logger_different_names(benchmark, num_loggers):
     ]
 
     def benchmark_get_logger():
-        for i in range(1000):
-            loggers[i % num_loggers].warning("test message")
+        for index in range(1000):
+            loggers[index % num_loggers].warning("test message")
 
     benchmark(benchmark_get_logger)

opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py

@@ -20,8 +20,8 @@
 import threading
 import traceback
 import warnings
+from functools import lru_cache
 from os import environ
-from threading import Lock
 from time import time_ns
 from typing import Any, Callable, Optional, Tuple, Union  # noqa
 
@@ -644,13 +644,47 @@ def __init__(
         self._at_exit_handler = None
         if shutdown_on_exit:
             self._at_exit_handler = atexit.register(self.shutdown)
-        self._logger_cache = {}
-        self._logger_cache_lock = Lock()
 
     @property
     def resource(self):
         return self._resource
 
+    def _get_logger_no_cache(
+        self,
+        name: str,
+        version: Optional[str] = None,
+        schema_url: Optional[str] = None,
+        attributes: Optional[Attributes] = None,
+    ) -> Logger:
+        return Logger(
+            self._resource,
+            self._multi_log_record_processor,
+            InstrumentationScope(
+                name,
+                version,
+                schema_url,
+                attributes,
+            ),
+        )
+
+    @lru_cache(maxsize=None)
+    def _get_logger_cached(
+        self,
+        name: str,
+        version: Optional[str] = None,
+        schema_url: Optional[str] = None,
+    ) -> Logger:
+        return Logger(
+            self._resource,
+            self._multi_log_record_processor,
+            InstrumentationScope(
+                name,
+                version,
+                schema_url,
+                None,
+            ),
+        )
+
     def get_logger(
         self,
         name: str,
@@ -666,28 +700,12 @@ def get_logger(
                 schema_url=schema_url,
                 attributes=attributes,
             )
-        key = (name, version, schema_url)
-        # Fast path if the logger is already in the cache, return it
-        if key in self._logger_cache:
-            return self._logger_cache[key]
-
-        # Lock to prevent race conditions when registering loggers with the same key
-        with self._logger_cache_lock:
-            # Check again in case another thread added the logger while waiting
-            if key in self._logger_cache:
-                return self._logger_cache[key]
-
-            self._logger_cache[key] = Logger(
-                self._resource,
-                self._multi_log_record_processor,
-                InstrumentationScope(
-                    name,
-                    version,
-                    schema_url,
-                    attributes,
-                ),
+        if attributes is None:
+            return self._get_logger_cached(name, version, schema_url)
+        else:
+            return self._get_logger_no_cache(
+                name, version, schema_url, attributes
             )
-            return self._logger_cache[key]
 
     def add_log_record_processor(
         self, log_record_processor: LogRecordProcessor

opentelemetry-sdk/tests/logs/test_log_provider_cache.py

@@ -28,47 +28,53 @@ class TestLogProviderCache(unittest.TestCase):
     def test_get_logger_single_handler(self):
         handler, logger_provider = set_up_logging_handler(level=logging.DEBUG)
 
+        cache_info = logger_provider._get_logger_cached.cache_clear()
+
         logger = create_logger(handler, "test_logger")
 
+        # Ensure logger is lazily cached
+        cache_info = logger_provider._get_logger_cached.cache_info()
+        self.assertEqual(0, cache_info.currsize)
+
         logger.warning("test message")
 
-        self.assertEqual(1, len(logger_provider._logger_cache))
-        self.assertTrue(
-            ("test_logger", "", None) in logger_provider._logger_cache
-        )
+        cache_info = logger_provider._get_logger_cached.cache_info()
+        self.assertEqual(1, cache_info.currsize)
+        self.assertEqual(1, cache_info.misses)
 
+        # Ensure only one logger is cached
         rounds = 100
         for _ in range(rounds):
             logger.warning("test message")
 
-        self.assertEqual(1, len(logger_provider._logger_cache))
-        self.assertTrue(
-            ("test_logger", "", None) in logger_provider._logger_cache
-        )
+        cache_info = logger_provider._get_logger_cached.cache_info()
+        self.assertEqual(1, cache_info.currsize)
+        self.assertEqual(1, cache_info.misses)
 
     def test_get_logger_multiple_loggers(self):
         handler, logger_provider = set_up_logging_handler(level=logging.DEBUG)
 
+        cache_info = logger_provider._get_logger_cached.cache_clear()
+
         num_loggers = 10
         loggers = [create_logger(handler, str(i)) for i in range(num_loggers)]
 
+        # Ensure loggers are lazily cached
+        cache_info = logger_provider._get_logger_cached.cache_info()
+        self.assertEqual(0, cache_info.currsize)
+
         for logger in loggers:
             logger.warning("test message")
 
-        self.assertEqual(num_loggers, len(logger_provider._logger_cache))
-        print(logger_provider._logger_cache)
-        for logger in loggers:
-            self.assertTrue(
-                (logger.name, "", None) in logger_provider._logger_cache
-            )
+        cache_info = logger_provider._get_logger_cached.cache_info()
+        self.assertEqual(num_loggers, cache_info.currsize)
+        self.assertEqual(num_loggers, cache_info.misses)
 
         rounds = 100
         for _ in range(rounds):
             for logger in loggers:
                 logger.warning("test message")
 
-        self.assertEqual(num_loggers, len(logger_provider._logger_cache))
-        for logger in loggers:
-            self.assertTrue(
-                (logger.name, "", None) in logger_provider._logger_cache
-            )
+        cache_info = logger_provider._get_logger_cached.cache_info()
+        self.assertEqual(num_loggers, cache_info.currsize)
+        self.assertEqual(num_loggers, cache_info.misses)