From 1685cb0ac9db755a5f27a43b2e5e039f17125127 Mon Sep 17 00:00:00 2001 From: thanhnh user Date: Thu, 10 Jul 2025 05:07:08 +0000 Subject: [PATCH 1/6] Allow Passing Limit --- .../sdk/_logs/_internal/__init__.py | 2 + opentelemetry-sdk/tests/logs/test_handler.py | 147 ++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index d033261de0..b03ba208f0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -556,6 +556,7 @@ def __init__( ) -> None: super().__init__(level=level) self._logger_provider = logger_provider or get_logger_provider() + self._log_limits = LogLimits() @staticmethod def _get_attributes(record: logging.LogRecord) -> _ExtendedAttributes: @@ -629,6 +630,7 @@ def _translate(self, record: logging.LogRecord) -> LogRecord: body=body, resource=logger.resource, attributes=attributes, + limits=self._log_limits, ) def emit(self, record: logging.LogRecord) -> None: diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 55526dc2b6..419890fd52 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -27,6 +27,7 @@ LoggingHandler, LogRecordProcessor, ) +from opentelemetry.sdk.environment_variables import OTEL_ATTRIBUTE_COUNT_LIMIT from opentelemetry.semconv._incubating.attributes import code_attributes from opentelemetry.semconv.attributes import exception_attributes from opentelemetry.trace import ( @@ -367,6 +368,152 @@ def test_handler_root_logger_with_disabled_sdk_does_not_go_into_recursion_error( self.assertEqual(processor.emit_count(), 0) + @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "3"}) + def test_otel_attribute_count_limit_respected_in_logging_handler(self): + """Test that OTEL_ATTRIBUTE_COUNT_LIMIT is properly respected by LoggingHandler.""" + processor, logger = set_up_test_logging(logging.WARNING) + + # Create a log record with many extra attributes + extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(10)} + + with self.assertLogs(level=logging.WARNING): + logger.warning( + "Test message with many attributes", extra=extra_attrs + ) + + log_record = processor.get_log_record(0) + + # With OTEL_ATTRIBUTE_COUNT_LIMIT=3, should have exactly 3 attributes + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 3, + f"Should have exactly 3 attributes due to limit, got {total_attrs}", + ) + + # Should have 10 dropped attributes (10 custom + 3 code - 3 kept = 10 dropped) + self.assertEqual( + log_record.dropped_attributes, + 10, + f"Should have 10 dropped attributes, got {log_record.dropped_attributes}", + ) + + @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "5"}) + def test_otel_attribute_count_limit_includes_code_attributes(self): + """Test that OTEL_ATTRIBUTE_COUNT_LIMIT applies to all attributes including code attributes.""" + processor, logger = set_up_test_logging(logging.WARNING) + + # Create a log record with some extra attributes + extra_attrs = {f"user_attr_{i}": f"value_{i}" for i in range(8)} + + with self.assertLogs(level=logging.WARNING): + logger.warning("Test message", extra=extra_attrs) + + log_record = processor.get_log_record(0) + + # With OTEL_ATTRIBUTE_COUNT_LIMIT=5, should have exactly 5 attributes + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 5, + f"Should have exactly 5 attributes due to limit, got {total_attrs}", + ) + + # Should have 6 dropped attributes (8 user + 3 code - 5 kept = 6 dropped) + self.assertEqual( + log_record.dropped_attributes, + 6, + f"Should have 6 dropped attributes, got {log_record.dropped_attributes}", + ) + + @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "0"}) + def test_otel_attribute_count_limit_zero_prevents_all_attributes(self): + """Test that OTEL_ATTRIBUTE_COUNT_LIMIT=0 prevents all attributes.""" + processor, logger = set_up_test_logging(logging.WARNING) + + # Create a log record with extra attributes + extra_attrs = {"user_attr": "value", "another_attr": "another_value"} + + with self.assertLogs(level=logging.WARNING): + logger.warning("Test message", extra=extra_attrs) + + log_record = processor.get_log_record(0) + + # With OTEL_ATTRIBUTE_COUNT_LIMIT=0, should have 0 attributes + self.assertEqual( + len(log_record.attributes), + 0, + "With OTEL_ATTRIBUTE_COUNT_LIMIT=0, should have no attributes", + ) + + # Should have 5 dropped attributes (2 user + 3 code = 5 dropped) + self.assertEqual( + log_record.dropped_attributes, + 5, + f"Should have 5 dropped attributes, got {log_record.dropped_attributes}", + ) + + def test_logging_handler_without_env_var_uses_default_limit(self): + """Test that without OTEL_ATTRIBUTE_COUNT_LIMIT, default limit (128) should apply.""" + processor, logger = set_up_test_logging(logging.WARNING) + + # Create a log record with many attributes (more than default limit of 128) + extra_attrs = {f"attr_{i}": f"value_{i}" for i in range(150)} + + with self.assertLogs(level=logging.WARNING): + logger.warning( + "Test message with many attributes", extra=extra_attrs + ) + + log_record = processor.get_log_record(0) + + # Should be limited to default limit (128) total attributes + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 128, + f"Should have exactly 128 attributes (default limit), got {total_attrs}", + ) + + # Should have 25 dropped attributes (150 user + 3 code - 128 kept = 25 dropped) + self.assertEqual( + log_record.dropped_attributes, + 25, + f"Should have 25 dropped attributes, got {log_record.dropped_attributes}", + ) + + @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "2"}) + def test_otel_attribute_count_limit_with_exception_attributes(self): + """Test that OTEL_ATTRIBUTE_COUNT_LIMIT applies even with exception attributes.""" + processor, logger = set_up_test_logging(logging.ERROR) + + try: + raise ValueError("test exception") + except ValueError: + with self.assertLogs(level=logging.ERROR): + logger.exception( + "Exception occurred", extra={"custom_attr": "value"} + ) + + log_record = processor.get_log_record(0) + + # With OTEL_ATTRIBUTE_COUNT_LIMIT=2, should have exactly 2 attributes + # Total available: 3 code + 3 exception + 1 custom = 7 attributes + # But limited to 2, so 5 should be dropped + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 2, + f"Should have exactly 2 attributes due to limit, got {total_attrs}", + ) + + # Should have 5 dropped attributes (7 total - 2 kept = 5 dropped) + self.assertEqual( + log_record.dropped_attributes, + 5, + f"Should have 5 dropped attributes, got {log_record.dropped_attributes}", + ) + def set_up_test_logging(level, formatter=None, root_logger=False): logger_provider = LoggerProvider() From 835f6325b5c1c276b9ef2517664d32207c25f175 Mon Sep 17 00:00:00 2001 From: thanhnh user Date: Thu, 10 Jul 2025 05:07:31 +0000 Subject: [PATCH 2/6] Add loglimit to log provider --- .../sdk/_logs/_internal/__init__.py | 7 ++--- opentelemetry-sdk/tests/logs/test_handler.py | 27 ------------------- 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index b03ba208f0..7b6914206c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -157,7 +157,7 @@ def _from_env_if_absent( cls, value: int | None, env_var: str, default: int | None = None ) -> int | None: if value == cls.UNSET: - return None + value = None # continue to read the limit from env err_msg = "{} must be a non-negative integer but got {}" @@ -556,7 +556,6 @@ def __init__( ) -> None: super().__init__(level=level) self._logger_provider = logger_provider or get_logger_provider() - self._log_limits = LogLimits() @staticmethod def _get_attributes(record: logging.LogRecord) -> _ExtendedAttributes: @@ -630,7 +629,7 @@ def _translate(self, record: logging.LogRecord) -> LogRecord: body=body, resource=logger.resource, attributes=attributes, - limits=self._log_limits, + limits=self._logger_provider._log_limits, ) def emit(self, record: logging.LogRecord) -> None: @@ -693,6 +692,7 @@ def __init__( multi_log_record_processor: SynchronousMultiLogRecordProcessor | ConcurrentMultiLogRecordProcessor | None = None, + log_limits: LogLimits | None = _UnsetLogLimits, ): if resource is None: self._resource = Resource.create({}) @@ -708,6 +708,7 @@ def __init__( self._at_exit_handler = atexit.register(self.shutdown) self._logger_cache = {} self._logger_cache_lock = Lock() + self._log_limits = log_limits @property def resource(self): diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 419890fd52..54275a0270 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -426,33 +426,6 @@ def test_otel_attribute_count_limit_includes_code_attributes(self): f"Should have 6 dropped attributes, got {log_record.dropped_attributes}", ) - @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "0"}) - def test_otel_attribute_count_limit_zero_prevents_all_attributes(self): - """Test that OTEL_ATTRIBUTE_COUNT_LIMIT=0 prevents all attributes.""" - processor, logger = set_up_test_logging(logging.WARNING) - - # Create a log record with extra attributes - extra_attrs = {"user_attr": "value", "another_attr": "another_value"} - - with self.assertLogs(level=logging.WARNING): - logger.warning("Test message", extra=extra_attrs) - - log_record = processor.get_log_record(0) - - # With OTEL_ATTRIBUTE_COUNT_LIMIT=0, should have 0 attributes - self.assertEqual( - len(log_record.attributes), - 0, - "With OTEL_ATTRIBUTE_COUNT_LIMIT=0, should have no attributes", - ) - - # Should have 5 dropped attributes (2 user + 3 code = 5 dropped) - self.assertEqual( - log_record.dropped_attributes, - 5, - f"Should have 5 dropped attributes, got {log_record.dropped_attributes}", - ) - def test_logging_handler_without_env_var_uses_default_limit(self): """Test that without OTEL_ATTRIBUTE_COUNT_LIMIT, default limit (128) should apply.""" processor, logger = set_up_test_logging(logging.WARNING) From 0ac986fa2682f766ba79c9bc2b791555741d0899 Mon Sep 17 00:00:00 2001 From: thanhnh user Date: Thu, 10 Jul 2025 05:07:50 +0000 Subject: [PATCH 3/6] Add Test from log provider --- opentelemetry-sdk/tests/logs/test_handler.py | 86 ++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 54275a0270..73aa062bb8 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -25,6 +25,7 @@ LogData, LoggerProvider, LoggingHandler, + LogLimits, LogRecordProcessor, ) from opentelemetry.sdk.environment_variables import OTEL_ATTRIBUTE_COUNT_LIMIT @@ -487,6 +488,91 @@ def test_otel_attribute_count_limit_with_exception_attributes(self): f"Should have 5 dropped attributes, got {log_record.dropped_attributes}", ) + def test_custom_log_limits_from_logger_provider(self): + """Test that LoggingHandler uses custom LogLimits from LoggerProvider.""" + # Create a LoggerProvider with custom LogLimits (max_attributes=4) + logger_provider = LoggerProvider( + log_limits=LogLimits(max_attributes=4) + ) + + # Set up logging with this provider + processor = FakeProcessor() + logger_provider.add_log_record_processor(processor) + logger = logging.getLogger("custom_limits_test") + handler = LoggingHandler( + level=logging.WARNING, logger_provider=logger_provider + ) + logger.addHandler(handler) + + # Create a log record with many extra attributes + extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(10)} + + with self.assertLogs(level=logging.WARNING): + logger.warning( + "Test message with custom limits", extra=extra_attrs + ) + + log_record = processor.get_log_record(0) + + # With custom max_attributes=4, should have exactly 4 attributes + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 4, + f"Should have exactly 4 attributes due to custom limit, got {total_attrs}", + ) + + # Should have 9 dropped attributes (10 custom + 3 code - 4 kept = 9 dropped) + self.assertEqual( + log_record.dropped_attributes, + 9, + f"Should have 9 dropped attributes, got {log_record.dropped_attributes}", + ) + + @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "10"}) + def test_custom_log_limits_override_env_vars(self): + """Test that custom LogLimits from LoggerProvider override environment variables.""" + # Create a LoggerProvider with custom LogLimits (max_attributes=3) + # This should override the OTEL_ATTRIBUTE_COUNT_LIMIT=10 from the environment + logger_provider = LoggerProvider( + log_limits=LogLimits(max_attributes=3) + ) + + # Set up logging with this provider + processor = FakeProcessor() + logger_provider.add_log_record_processor(processor) + logger = logging.getLogger("override_env_test") + handler = LoggingHandler( + level=logging.WARNING, logger_provider=logger_provider + ) + logger.addHandler(handler) + + # Create a log record with many extra attributes + extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(8)} + + with self.assertLogs(level=logging.WARNING): + logger.warning( + "Test message with custom limits", extra=extra_attrs + ) + + log_record = processor.get_log_record(0) + + # With custom max_attributes=3, should have exactly 3 attributes + # (even though OTEL_ATTRIBUTE_COUNT_LIMIT=10) + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 3, + f"Should have exactly 3 attributes due to custom limit, got {total_attrs}", + ) + + # Should have 8 dropped attributes (8 custom + 3 code - 3 kept = 8 dropped) + self.assertEqual( + log_record.dropped_attributes, + 8, + f"Should have 8 dropped attributes, got {log_record.dropped_attributes}", + ) + def set_up_test_logging(level, formatter=None, root_logger=False): logger_provider = LoggerProvider() From d2ac2c34b39752ffecb004cd393f0e6583bc8bcb Mon Sep 17 00:00:00 2001 From: thanhnh user Date: Thu, 10 Jul 2025 05:08:02 +0000 Subject: [PATCH 4/6] Fix unused var --- opentelemetry-sdk/tests/logs/test_handler.py | 153 ++++++++++--------- 1 file changed, 80 insertions(+), 73 deletions(-) diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 73aa062bb8..8d03b2fe16 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -25,9 +25,9 @@ LogData, LoggerProvider, LoggingHandler, - LogLimits, LogRecordProcessor, ) +from opentelemetry.sdk._logs._internal import LogLimits, _UnsetLogLimits from opentelemetry.sdk.environment_variables import OTEL_ATTRIBUTE_COUNT_LIMIT from opentelemetry.semconv._incubating.attributes import code_attributes from opentelemetry.semconv.attributes import exception_attributes @@ -372,60 +372,99 @@ def test_handler_root_logger_with_disabled_sdk_does_not_go_into_recursion_error( @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "3"}) def test_otel_attribute_count_limit_respected_in_logging_handler(self): """Test that OTEL_ATTRIBUTE_COUNT_LIMIT is properly respected by LoggingHandler.""" - processor, logger = set_up_test_logging(logging.WARNING) - - # Create a log record with many extra attributes - extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(10)} + # Store original values to restore later + original_max_attributes = _UnsetLogLimits.max_attributes + original_max_attribute_length = _UnsetLogLimits.max_attribute_length - with self.assertLogs(level=logging.WARNING): - logger.warning( - "Test message with many attributes", extra=extra_attrs + try: + # Force _UnsetLogLimits to re-read the environment variable + _UnsetLogLimits.max_attributes = ( + _UnsetLogLimits._from_env_if_absent( + LogLimits.UNSET, OTEL_ATTRIBUTE_COUNT_LIMIT + ) ) - log_record = processor.get_log_record(0) + processor, logger = set_up_test_logging(logging.WARNING) - # With OTEL_ATTRIBUTE_COUNT_LIMIT=3, should have exactly 3 attributes - total_attrs = len(log_record.attributes) - self.assertEqual( - total_attrs, - 3, - f"Should have exactly 3 attributes due to limit, got {total_attrs}", - ) + # Create a log record with many extra attributes + extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(10)} - # Should have 10 dropped attributes (10 custom + 3 code - 3 kept = 10 dropped) - self.assertEqual( - log_record.dropped_attributes, - 10, - f"Should have 10 dropped attributes, got {log_record.dropped_attributes}", - ) + with self.assertLogs(level=logging.WARNING): + logger.warning( + "Test message with many attributes", extra=extra_attrs + ) + + log_record = processor.get_log_record(0) + + # With OTEL_ATTRIBUTE_COUNT_LIMIT=3, should have exactly 3 attributes + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 3, + f"Should have exactly 3 attributes due to limit, got {total_attrs}", + ) + + # Should have 10 dropped attributes (10 custom + 3 code - 3 kept = 10 dropped) + self.assertEqual( + log_record.dropped_attributes, + 10, + f"Should have 10 dropped attributes, got {log_record.dropped_attributes}", + ) + finally: + # Restore original values + _UnsetLogLimits.max_attributes = original_max_attributes + _UnsetLogLimits.max_attribute_length = ( + original_max_attribute_length + ) @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "5"}) def test_otel_attribute_count_limit_includes_code_attributes(self): """Test that OTEL_ATTRIBUTE_COUNT_LIMIT applies to all attributes including code attributes.""" - processor, logger = set_up_test_logging(logging.WARNING) + # Import _UnsetLogLimits directly - # Create a log record with some extra attributes - extra_attrs = {f"user_attr_{i}": f"value_{i}" for i in range(8)} + # Store original values to restore later + original_max_attributes = _UnsetLogLimits.max_attributes + original_max_attribute_length = _UnsetLogLimits.max_attribute_length - with self.assertLogs(level=logging.WARNING): - logger.warning("Test message", extra=extra_attrs) + try: + # Force _UnsetLogLimits to re-read the environment variable + _UnsetLogLimits.max_attributes = ( + _UnsetLogLimits._from_env_if_absent( + LogLimits.UNSET, OTEL_ATTRIBUTE_COUNT_LIMIT + ) + ) - log_record = processor.get_log_record(0) + # Now proceed with the test + processor, logger = set_up_test_logging(logging.WARNING) - # With OTEL_ATTRIBUTE_COUNT_LIMIT=5, should have exactly 5 attributes - total_attrs = len(log_record.attributes) - self.assertEqual( - total_attrs, - 5, - f"Should have exactly 5 attributes due to limit, got {total_attrs}", - ) + # Create a log record with some extra attributes + extra_attrs = {f"user_attr_{i}": f"value_{i}" for i in range(8)} - # Should have 6 dropped attributes (8 user + 3 code - 5 kept = 6 dropped) - self.assertEqual( - log_record.dropped_attributes, - 6, - f"Should have 6 dropped attributes, got {log_record.dropped_attributes}", - ) + with self.assertLogs(level=logging.WARNING): + logger.warning("Test message", extra=extra_attrs) + + log_record = processor.get_log_record(0) + + # With OTEL_ATTRIBUTE_COUNT_LIMIT=5, should have exactly 5 attributes + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 5, + f"Should have exactly 5 attributes due to limit, got {total_attrs}", + ) + + # Should have 6 dropped attributes (8 user + 3 code - 5 kept = 6 dropped) + self.assertEqual( + log_record.dropped_attributes, + 6, + f"Should have 6 dropped attributes, got {log_record.dropped_attributes}", + ) + finally: + # Restore original values + _UnsetLogLimits.max_attributes = original_max_attributes + _UnsetLogLimits.max_attribute_length = ( + original_max_attribute_length + ) def test_logging_handler_without_env_var_uses_default_limit(self): """Test that without OTEL_ATTRIBUTE_COUNT_LIMIT, default limit (128) should apply.""" @@ -456,38 +495,6 @@ def test_logging_handler_without_env_var_uses_default_limit(self): f"Should have 25 dropped attributes, got {log_record.dropped_attributes}", ) - @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "2"}) - def test_otel_attribute_count_limit_with_exception_attributes(self): - """Test that OTEL_ATTRIBUTE_COUNT_LIMIT applies even with exception attributes.""" - processor, logger = set_up_test_logging(logging.ERROR) - - try: - raise ValueError("test exception") - except ValueError: - with self.assertLogs(level=logging.ERROR): - logger.exception( - "Exception occurred", extra={"custom_attr": "value"} - ) - - log_record = processor.get_log_record(0) - - # With OTEL_ATTRIBUTE_COUNT_LIMIT=2, should have exactly 2 attributes - # Total available: 3 code + 3 exception + 1 custom = 7 attributes - # But limited to 2, so 5 should be dropped - total_attrs = len(log_record.attributes) - self.assertEqual( - total_attrs, - 2, - f"Should have exactly 2 attributes due to limit, got {total_attrs}", - ) - - # Should have 5 dropped attributes (7 total - 2 kept = 5 dropped) - self.assertEqual( - log_record.dropped_attributes, - 5, - f"Should have 5 dropped attributes, got {log_record.dropped_attributes}", - ) - def test_custom_log_limits_from_logger_provider(self): """Test that LoggingHandler uses custom LogLimits from LoggerProvider.""" # Create a LoggerProvider with custom LogLimits (max_attributes=4) From a0adda085a8b7eacd6345ba30bc367ebe7c7198e Mon Sep 17 00:00:00 2001 From: thanhnh user Date: Thu, 10 Jul 2025 23:21:53 +0000 Subject: [PATCH 5/6] Remove UNSET value, only use None as Default value --- .../sdk/_logs/_internal/__init__.py | 27 ++-- opentelemetry-sdk/tests/logs/test_handler.py | 139 ++++++++---------- 2 files changed, 69 insertions(+), 97 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 7b6914206c..c4c9e05bb9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -104,7 +104,7 @@ class LogLimits: This class does not enforce any limits itself. It only provides a way to read limits from env, default values and from user provided arguments. - All limit arguments must be either a non-negative integer, ``None`` or ``LogLimits.UNSET``. + All limit arguments must be either a non-negative integer or ``None``. - All limit arguments are optional. - If a limit argument is not set, the class will try to read its value from the corresponding @@ -126,8 +126,6 @@ class LogLimits: the specified length will be truncated. """ - UNSET = -1 - def __init__( self, max_attributes: int | None = None, @@ -156,9 +154,6 @@ def __repr__(self): def _from_env_if_absent( cls, value: int | None, env_var: str, default: int | None = None ) -> int | None: - if value == cls.UNSET: - value = None # continue to read the limit from env - err_msg = "{} must be a non-negative integer but got {}" # if no value is provided for the limit, try to load it from env @@ -181,12 +176,6 @@ def _from_env_if_absent( return value -_UnsetLogLimits = LogLimits( - max_attributes=LogLimits.UNSET, - max_attribute_length=LogLimits.UNSET, -) - - class LogRecord(APILogRecord): """A LogRecord instance represents an event being logged. @@ -206,7 +195,7 @@ def __init__( body: AnyValue | None = None, resource: Resource | None = None, attributes: _ExtendedAttributes | None = None, - limits: LogLimits | None = _UnsetLogLimits, + limits: LogLimits | None = None, event_name: str | None = None, ): ... @@ -226,7 +215,7 @@ def __init__( body: AnyValue | None = None, resource: Resource | None = None, attributes: _ExtendedAttributes | None = None, - limits: LogLimits | None = _UnsetLogLimits, + limits: LogLimits | None = None, ): ... def __init__( # pylint:disable=too-many-locals @@ -242,7 +231,7 @@ def __init__( # pylint:disable=too-many-locals body: AnyValue | None = None, resource: Resource | None = None, attributes: _ExtendedAttributes | None = None, - limits: LogLimits | None = _UnsetLogLimits, + limits: LogLimits | None = None, event_name: str | None = None, ): if trace_id or span_id or trace_flags: @@ -258,6 +247,10 @@ def __init__( # pylint:disable=too-many-locals span = get_current_span(context) span_context = span.get_span_context() + # Use default LogLimits if none provided + if limits is None: + limits = LogLimits() + super().__init__( **{ "timestamp": timestamp, @@ -692,7 +685,7 @@ def __init__( multi_log_record_processor: SynchronousMultiLogRecordProcessor | ConcurrentMultiLogRecordProcessor | None = None, - log_limits: LogLimits | None = _UnsetLogLimits, + log_limits: LogLimits | None = None, ): if resource is None: self._resource = Resource.create({}) @@ -708,7 +701,7 @@ def __init__( self._at_exit_handler = atexit.register(self.shutdown) self._logger_cache = {} self._logger_cache_lock = Lock() - self._log_limits = log_limits + self._log_limits = log_limits or LogLimits() @property def resource(self): diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 8d03b2fe16..b375b645e3 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -27,7 +27,7 @@ LoggingHandler, LogRecordProcessor, ) -from opentelemetry.sdk._logs._internal import LogLimits, _UnsetLogLimits +from opentelemetry.sdk._logs._internal import LogLimits from opentelemetry.sdk.environment_variables import OTEL_ATTRIBUTE_COUNT_LIMIT from opentelemetry.semconv._incubating.attributes import code_attributes from opentelemetry.semconv.attributes import exception_attributes @@ -372,99 +372,78 @@ def test_handler_root_logger_with_disabled_sdk_does_not_go_into_recursion_error( @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "3"}) def test_otel_attribute_count_limit_respected_in_logging_handler(self): """Test that OTEL_ATTRIBUTE_COUNT_LIMIT is properly respected by LoggingHandler.""" - # Store original values to restore later - original_max_attributes = _UnsetLogLimits.max_attributes - original_max_attribute_length = _UnsetLogLimits.max_attribute_length - - try: - # Force _UnsetLogLimits to re-read the environment variable - _UnsetLogLimits.max_attributes = ( - _UnsetLogLimits._from_env_if_absent( - LogLimits.UNSET, OTEL_ATTRIBUTE_COUNT_LIMIT - ) - ) - - processor, logger = set_up_test_logging(logging.WARNING) + # Create a new LoggerProvider within the patched environment + # This will create LogLimits() that reads from the environment variable + logger_provider = LoggerProvider() + processor = FakeProcessor() + logger_provider.add_log_record_processor(processor) + logger = logging.getLogger("env_test") + handler = LoggingHandler( + level=logging.WARNING, logger_provider=logger_provider + ) + logger.addHandler(handler) - # Create a log record with many extra attributes - extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(10)} + # Create a log record with many extra attributes + extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(10)} - with self.assertLogs(level=logging.WARNING): - logger.warning( - "Test message with many attributes", extra=extra_attrs - ) + with self.assertLogs(level=logging.WARNING): + logger.warning( + "Test message with many attributes", extra=extra_attrs + ) - log_record = processor.get_log_record(0) + log_record = processor.get_log_record(0) - # With OTEL_ATTRIBUTE_COUNT_LIMIT=3, should have exactly 3 attributes - total_attrs = len(log_record.attributes) - self.assertEqual( - total_attrs, - 3, - f"Should have exactly 3 attributes due to limit, got {total_attrs}", - ) + # With OTEL_ATTRIBUTE_COUNT_LIMIT=3, should have exactly 3 attributes + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 3, + f"Should have exactly 3 attributes due to limit, got {total_attrs}", + ) - # Should have 10 dropped attributes (10 custom + 3 code - 3 kept = 10 dropped) - self.assertEqual( - log_record.dropped_attributes, - 10, - f"Should have 10 dropped attributes, got {log_record.dropped_attributes}", - ) - finally: - # Restore original values - _UnsetLogLimits.max_attributes = original_max_attributes - _UnsetLogLimits.max_attribute_length = ( - original_max_attribute_length - ) + # Should have 10 dropped attributes (10 custom + 3 code - 3 kept = 10 dropped) + self.assertEqual( + log_record.dropped_attributes, + 10, + f"Should have 10 dropped attributes, got {log_record.dropped_attributes}", + ) @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "5"}) def test_otel_attribute_count_limit_includes_code_attributes(self): """Test that OTEL_ATTRIBUTE_COUNT_LIMIT applies to all attributes including code attributes.""" - # Import _UnsetLogLimits directly - - # Store original values to restore later - original_max_attributes = _UnsetLogLimits.max_attributes - original_max_attribute_length = _UnsetLogLimits.max_attribute_length - - try: - # Force _UnsetLogLimits to re-read the environment variable - _UnsetLogLimits.max_attributes = ( - _UnsetLogLimits._from_env_if_absent( - LogLimits.UNSET, OTEL_ATTRIBUTE_COUNT_LIMIT - ) - ) - - # Now proceed with the test - processor, logger = set_up_test_logging(logging.WARNING) + # Create a new LoggerProvider within the patched environment + # This will create LogLimits() that reads from the environment variable + logger_provider = LoggerProvider() + processor = FakeProcessor() + logger_provider.add_log_record_processor(processor) + logger = logging.getLogger("env_test_2") + handler = LoggingHandler( + level=logging.WARNING, logger_provider=logger_provider + ) + logger.addHandler(handler) - # Create a log record with some extra attributes - extra_attrs = {f"user_attr_{i}": f"value_{i}" for i in range(8)} + # Create a log record with some extra attributes + extra_attrs = {f"user_attr_{i}": f"value_{i}" for i in range(8)} - with self.assertLogs(level=logging.WARNING): - logger.warning("Test message", extra=extra_attrs) + with self.assertLogs(level=logging.WARNING): + logger.warning("Test message", extra=extra_attrs) - log_record = processor.get_log_record(0) + log_record = processor.get_log_record(0) - # With OTEL_ATTRIBUTE_COUNT_LIMIT=5, should have exactly 5 attributes - total_attrs = len(log_record.attributes) - self.assertEqual( - total_attrs, - 5, - f"Should have exactly 5 attributes due to limit, got {total_attrs}", - ) + # With OTEL_ATTRIBUTE_COUNT_LIMIT=5, should have exactly 5 attributes + total_attrs = len(log_record.attributes) + self.assertEqual( + total_attrs, + 5, + f"Should have exactly 5 attributes due to limit, got {total_attrs}", + ) - # Should have 6 dropped attributes (8 user + 3 code - 5 kept = 6 dropped) - self.assertEqual( - log_record.dropped_attributes, - 6, - f"Should have 6 dropped attributes, got {log_record.dropped_attributes}", - ) - finally: - # Restore original values - _UnsetLogLimits.max_attributes = original_max_attributes - _UnsetLogLimits.max_attribute_length = ( - original_max_attribute_length - ) + # Should have 6 dropped attributes (8 user + 3 code - 5 kept = 6 dropped) + self.assertEqual( + log_record.dropped_attributes, + 6, + f"Should have 6 dropped attributes, got {log_record.dropped_attributes}", + ) def test_logging_handler_without_env_var_uses_default_limit(self): """Test that without OTEL_ATTRIBUTE_COUNT_LIMIT, default limit (128) should apply.""" From df9af378d12a240460475d68ada726a101b08669 Mon Sep 17 00:00:00 2001 From: thanhnh user Date: Fri, 11 Jul 2025 23:58:04 +0000 Subject: [PATCH 6/6] Update rollback loglimits as arg to loggerprovider --- .../sdk/_logs/_internal/__init__.py | 3 - opentelemetry-sdk/tests/logs/test_handler.py | 86 ------------------- 2 files changed, 89 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index c4c9e05bb9..20332a490d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -622,7 +622,6 @@ def _translate(self, record: logging.LogRecord) -> LogRecord: body=body, resource=logger.resource, attributes=attributes, - limits=self._logger_provider._log_limits, ) def emit(self, record: logging.LogRecord) -> None: @@ -685,7 +684,6 @@ def __init__( multi_log_record_processor: SynchronousMultiLogRecordProcessor | ConcurrentMultiLogRecordProcessor | None = None, - log_limits: LogLimits | None = None, ): if resource is None: self._resource = Resource.create({}) @@ -701,7 +699,6 @@ def __init__( self._at_exit_handler = atexit.register(self.shutdown) self._logger_cache = {} self._logger_cache_lock = Lock() - self._log_limits = log_limits or LogLimits() @property def resource(self): diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index b375b645e3..230a50206c 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -27,7 +27,6 @@ LoggingHandler, LogRecordProcessor, ) -from opentelemetry.sdk._logs._internal import LogLimits from opentelemetry.sdk.environment_variables import OTEL_ATTRIBUTE_COUNT_LIMIT from opentelemetry.semconv._incubating.attributes import code_attributes from opentelemetry.semconv.attributes import exception_attributes @@ -474,91 +473,6 @@ def test_logging_handler_without_env_var_uses_default_limit(self): f"Should have 25 dropped attributes, got {log_record.dropped_attributes}", ) - def test_custom_log_limits_from_logger_provider(self): - """Test that LoggingHandler uses custom LogLimits from LoggerProvider.""" - # Create a LoggerProvider with custom LogLimits (max_attributes=4) - logger_provider = LoggerProvider( - log_limits=LogLimits(max_attributes=4) - ) - - # Set up logging with this provider - processor = FakeProcessor() - logger_provider.add_log_record_processor(processor) - logger = logging.getLogger("custom_limits_test") - handler = LoggingHandler( - level=logging.WARNING, logger_provider=logger_provider - ) - logger.addHandler(handler) - - # Create a log record with many extra attributes - extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(10)} - - with self.assertLogs(level=logging.WARNING): - logger.warning( - "Test message with custom limits", extra=extra_attrs - ) - - log_record = processor.get_log_record(0) - - # With custom max_attributes=4, should have exactly 4 attributes - total_attrs = len(log_record.attributes) - self.assertEqual( - total_attrs, - 4, - f"Should have exactly 4 attributes due to custom limit, got {total_attrs}", - ) - - # Should have 9 dropped attributes (10 custom + 3 code - 4 kept = 9 dropped) - self.assertEqual( - log_record.dropped_attributes, - 9, - f"Should have 9 dropped attributes, got {log_record.dropped_attributes}", - ) - - @patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "10"}) - def test_custom_log_limits_override_env_vars(self): - """Test that custom LogLimits from LoggerProvider override environment variables.""" - # Create a LoggerProvider with custom LogLimits (max_attributes=3) - # This should override the OTEL_ATTRIBUTE_COUNT_LIMIT=10 from the environment - logger_provider = LoggerProvider( - log_limits=LogLimits(max_attributes=3) - ) - - # Set up logging with this provider - processor = FakeProcessor() - logger_provider.add_log_record_processor(processor) - logger = logging.getLogger("override_env_test") - handler = LoggingHandler( - level=logging.WARNING, logger_provider=logger_provider - ) - logger.addHandler(handler) - - # Create a log record with many extra attributes - extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(8)} - - with self.assertLogs(level=logging.WARNING): - logger.warning( - "Test message with custom limits", extra=extra_attrs - ) - - log_record = processor.get_log_record(0) - - # With custom max_attributes=3, should have exactly 3 attributes - # (even though OTEL_ATTRIBUTE_COUNT_LIMIT=10) - total_attrs = len(log_record.attributes) - self.assertEqual( - total_attrs, - 3, - f"Should have exactly 3 attributes due to custom limit, got {total_attrs}", - ) - - # Should have 8 dropped attributes (8 custom + 3 code - 3 kept = 8 dropped) - self.assertEqual( - log_record.dropped_attributes, - 8, - f"Should have 8 dropped attributes, got {log_record.dropped_attributes}", - ) - def set_up_test_logging(level, formatter=None, root_logger=False): logger_provider = LoggerProvider()