diff --git a/CHANGELOG.md b/CHANGELOG.md index aa54d05abb..ec096b1be4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ inject a `requests.Session` or `grpc.ChannelCredentials` object into OTLP export ([#4634](https://github.com/open-telemetry/opentelemetry-python/pull/4634)) - semantic-conventions: Bump to 1.37.0 ([#4731](https://github.com/open-telemetry/opentelemetry-python/pull/4731)) +- opentelemetry-sdk: fix handling of OTEL_ATTRIBUTE_COUNT_LIMIT in logs + ([#4677](https://github.com/open-telemetry/opentelemetry-python/pull/4677)) - Performance: Cache `importlib_metadata.entry_points` ([#4735](https://github.com/open-telemetry/opentelemetry-python/pull/4735)) - opentelemetry-sdk: fix calling Logger.emit with an API LogRecord instance diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index 8b17de1c34..aca3d50c13 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: - return None - 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, diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index 55526dc2b6..9da5ea4988 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 ( @@ -34,7 +35,7 @@ set_span_in_context, ) - +# pylint: disable=too-many-public-methods class TestLoggingHandler(unittest.TestCase): def test_handler_default_log_level(self): processor, logger = set_up_test_logging(logging.NOTSET) @@ -367,6 +368,111 @@ 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.""" + # 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)} + + 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.""" + # 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)} + + 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}", + ) + + 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}", + ) + def set_up_test_logging(level, formatter=None, root_logger=False): logger_provider = LoggerProvider()