Skip to content

Commit 712ed5d

Browse files
giongto35emdnetoxrmx
authored
OTEL attribute count limit not respected, causing columns dropped (#4677)
* Allow Passing Limit * Add loglimit to log provider * Add Test from log provider * Fix unused var * Remove UNSET value, only use None as Default value * Update rollback loglimits as arg to loggerprovider * Update test_handler.py * Update CHANGELOG.md --------- Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
1 parent dd93fa7 commit 712ed5d

File tree

3 files changed

+117
-16
lines changed

3 files changed

+117
-16
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ inject a `requests.Session` or `grpc.ChannelCredentials` object into OTLP export
2424
([#4634](https://github.com/open-telemetry/opentelemetry-python/pull/4634))
2525
- semantic-conventions: Bump to 1.37.0
2626
([#4731](https://github.com/open-telemetry/opentelemetry-python/pull/4731))
27+
- opentelemetry-sdk: fix handling of OTEL_ATTRIBUTE_COUNT_LIMIT in logs
28+
([#4677](https://github.com/open-telemetry/opentelemetry-python/pull/4677))
2729
- Performance: Cache `importlib_metadata.entry_points`
2830
([#4735](https://github.com/open-telemetry/opentelemetry-python/pull/4735))
2931
- opentelemetry-sdk: fix calling Logger.emit with an API LogRecord instance

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ class LogLimits:
104104
This class does not enforce any limits itself. It only provides a way to read limits from env,
105105
default values and from user provided arguments.
106106
107-
All limit arguments must be either a non-negative integer, ``None`` or ``LogLimits.UNSET``.
107+
All limit arguments must be either a non-negative integer or ``None``.
108108
109109
- All limit arguments are optional.
110110
- If a limit argument is not set, the class will try to read its value from the corresponding
@@ -126,8 +126,6 @@ class LogLimits:
126126
the specified length will be truncated.
127127
"""
128128

129-
UNSET = -1
130-
131129
def __init__(
132130
self,
133131
max_attributes: int | None = None,
@@ -156,9 +154,6 @@ def __repr__(self):
156154
def _from_env_if_absent(
157155
cls, value: int | None, env_var: str, default: int | None = None
158156
) -> int | None:
159-
if value == cls.UNSET:
160-
return None
161-
162157
err_msg = "{} must be a non-negative integer but got {}"
163158

164159
# if no value is provided for the limit, try to load it from env
@@ -181,12 +176,6 @@ def _from_env_if_absent(
181176
return value
182177

183178

184-
_UnsetLogLimits = LogLimits(
185-
max_attributes=LogLimits.UNSET,
186-
max_attribute_length=LogLimits.UNSET,
187-
)
188-
189-
190179
class LogRecord(APILogRecord):
191180
"""A LogRecord instance represents an event being logged.
192181
@@ -206,7 +195,7 @@ def __init__(
206195
body: AnyValue | None = None,
207196
resource: Resource | None = None,
208197
attributes: _ExtendedAttributes | None = None,
209-
limits: LogLimits | None = _UnsetLogLimits,
198+
limits: LogLimits | None = None,
210199
event_name: str | None = None,
211200
): ...
212201

@@ -226,7 +215,7 @@ def __init__(
226215
body: AnyValue | None = None,
227216
resource: Resource | None = None,
228217
attributes: _ExtendedAttributes | None = None,
229-
limits: LogLimits | None = _UnsetLogLimits,
218+
limits: LogLimits | None = None,
230219
): ...
231220

232221
def __init__( # pylint:disable=too-many-locals
@@ -242,7 +231,7 @@ def __init__( # pylint:disable=too-many-locals
242231
body: AnyValue | None = None,
243232
resource: Resource | None = None,
244233
attributes: _ExtendedAttributes | None = None,
245-
limits: LogLimits | None = _UnsetLogLimits,
234+
limits: LogLimits | None = None,
246235
event_name: str | None = None,
247236
):
248237
if trace_id or span_id or trace_flags:
@@ -258,6 +247,10 @@ def __init__( # pylint:disable=too-many-locals
258247
span = get_current_span(context)
259248
span_context = span.get_span_context()
260249

250+
# Use default LogLimits if none provided
251+
if limits is None:
252+
limits = LogLimits()
253+
261254
super().__init__(
262255
**{
263256
"timestamp": timestamp,

opentelemetry-sdk/tests/logs/test_handler.py

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,15 @@
2727
LoggingHandler,
2828
LogRecordProcessor,
2929
)
30+
from opentelemetry.sdk.environment_variables import OTEL_ATTRIBUTE_COUNT_LIMIT
3031
from opentelemetry.semconv._incubating.attributes import code_attributes
3132
from opentelemetry.semconv.attributes import exception_attributes
3233
from opentelemetry.trace import (
3334
INVALID_SPAN_CONTEXT,
3435
set_span_in_context,
3536
)
3637

37-
38+
# pylint: disable=too-many-public-methods
3839
class TestLoggingHandler(unittest.TestCase):
3940
def test_handler_default_log_level(self):
4041
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(
367368

368369
self.assertEqual(processor.emit_count(), 0)
369370

371+
@patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "3"})
372+
def test_otel_attribute_count_limit_respected_in_logging_handler(self):
373+
"""Test that OTEL_ATTRIBUTE_COUNT_LIMIT is properly respected by LoggingHandler."""
374+
# Create a new LoggerProvider within the patched environment
375+
# This will create LogLimits() that reads from the environment variable
376+
logger_provider = LoggerProvider()
377+
processor = FakeProcessor()
378+
logger_provider.add_log_record_processor(processor)
379+
logger = logging.getLogger("env_test")
380+
handler = LoggingHandler(
381+
level=logging.WARNING, logger_provider=logger_provider
382+
)
383+
logger.addHandler(handler)
384+
385+
# Create a log record with many extra attributes
386+
extra_attrs = {f"custom_attr_{i}": f"value_{i}" for i in range(10)}
387+
388+
with self.assertLogs(level=logging.WARNING):
389+
logger.warning(
390+
"Test message with many attributes", extra=extra_attrs
391+
)
392+
393+
log_record = processor.get_log_record(0)
394+
395+
# With OTEL_ATTRIBUTE_COUNT_LIMIT=3, should have exactly 3 attributes
396+
total_attrs = len(log_record.attributes)
397+
self.assertEqual(
398+
total_attrs,
399+
3,
400+
f"Should have exactly 3 attributes due to limit, got {total_attrs}",
401+
)
402+
403+
# Should have 10 dropped attributes (10 custom + 3 code - 3 kept = 10 dropped)
404+
self.assertEqual(
405+
log_record.dropped_attributes,
406+
10,
407+
f"Should have 10 dropped attributes, got {log_record.dropped_attributes}",
408+
)
409+
410+
@patch.dict(os.environ, {OTEL_ATTRIBUTE_COUNT_LIMIT: "5"})
411+
def test_otel_attribute_count_limit_includes_code_attributes(self):
412+
"""Test that OTEL_ATTRIBUTE_COUNT_LIMIT applies to all attributes including code attributes."""
413+
# Create a new LoggerProvider within the patched environment
414+
# This will create LogLimits() that reads from the environment variable
415+
logger_provider = LoggerProvider()
416+
processor = FakeProcessor()
417+
logger_provider.add_log_record_processor(processor)
418+
logger = logging.getLogger("env_test_2")
419+
handler = LoggingHandler(
420+
level=logging.WARNING, logger_provider=logger_provider
421+
)
422+
logger.addHandler(handler)
423+
424+
# Create a log record with some extra attributes
425+
extra_attrs = {f"user_attr_{i}": f"value_{i}" for i in range(8)}
426+
427+
with self.assertLogs(level=logging.WARNING):
428+
logger.warning("Test message", extra=extra_attrs)
429+
430+
log_record = processor.get_log_record(0)
431+
432+
# With OTEL_ATTRIBUTE_COUNT_LIMIT=5, should have exactly 5 attributes
433+
total_attrs = len(log_record.attributes)
434+
self.assertEqual(
435+
total_attrs,
436+
5,
437+
f"Should have exactly 5 attributes due to limit, got {total_attrs}",
438+
)
439+
440+
# Should have 6 dropped attributes (8 user + 3 code - 5 kept = 6 dropped)
441+
self.assertEqual(
442+
log_record.dropped_attributes,
443+
6,
444+
f"Should have 6 dropped attributes, got {log_record.dropped_attributes}",
445+
)
446+
447+
def test_logging_handler_without_env_var_uses_default_limit(self):
448+
"""Test that without OTEL_ATTRIBUTE_COUNT_LIMIT, default limit (128) should apply."""
449+
processor, logger = set_up_test_logging(logging.WARNING)
450+
451+
# Create a log record with many attributes (more than default limit of 128)
452+
extra_attrs = {f"attr_{i}": f"value_{i}" for i in range(150)}
453+
454+
with self.assertLogs(level=logging.WARNING):
455+
logger.warning(
456+
"Test message with many attributes", extra=extra_attrs
457+
)
458+
459+
log_record = processor.get_log_record(0)
460+
461+
# Should be limited to default limit (128) total attributes
462+
total_attrs = len(log_record.attributes)
463+
self.assertEqual(
464+
total_attrs,
465+
128,
466+
f"Should have exactly 128 attributes (default limit), got {total_attrs}",
467+
)
468+
469+
# Should have 25 dropped attributes (150 user + 3 code - 128 kept = 25 dropped)
470+
self.assertEqual(
471+
log_record.dropped_attributes,
472+
25,
473+
f"Should have 25 dropped attributes, got {log_record.dropped_attributes}",
474+
)
475+
370476

371477
def set_up_test_logging(level, formatter=None, root_logger=False):
372478
logger_provider = LoggerProvider()

0 commit comments

Comments
 (0)