Skip to content

OTEL attribute count limit not respected, causing columns dropped #4677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -126,8 +126,6 @@ class LogLimits:
the specified length will be truncated.
"""

UNSET = -1

def __init__(
self,
max_attributes: int | None = None,
Expand Down Expand Up @@ -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
Expand All @@ -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.

Expand All @@ -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,
): ...

Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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,
Expand Down
106 changes: 106 additions & 0 deletions opentelemetry-sdk/tests/logs/test_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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()
Expand Down
Loading