Add logger exception support for logs API/SDK by iblancasa · Pull Request #4908 · open-telemetry/opentelemetry-python

iblancasa · 2026-02-11T17:28:29Z

Description

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Unit tests

Does This PR Require a Contrib Repo Change?

Yes. - Link to PR:
No.

Checklist:

Followed the style guidelines of this project
Changelogs have been updated
Unit tests have been added

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

xrmx · 2026-02-12T09:46:42Z

Haven't looked in detail at the PR but we are usually cautious on adding in development stuff to the api / sdk.

trask · 2026-02-13T17:28:44Z

hey @xrmx! check out open-telemetry/opentelemetry-specification#4886

if you can give this PR a basic review and "lgtm" without approving/merging it, then we can treat it as a "qualifying prototype" for marking it stable in the spec. we already have 3 qualifying prototypes which is the min bar for stabilizing the spec, but the more the better!

… into 4907

MikeGoldsmith

LGTM

Fixes #4858 Implementations / Prototypes: - Java: [`ExtendedLogRecordBuilder.setException(Throwable)`](https://github.com/open-telemetry/opentelemetry-java/blob/main/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java#L137) - Builder method called before `emit()` - Merged and available to users - .NET: [`LogRecordAttributeList.RecordException(Exception)`](https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry.Api/Logs/LogRecordAttributeList.cs#L191-L198) - Attribute list method called before `EmitLog()` - JavaScript: [opentelemetry-js#6385](open-telemetry/opentelemetry-js#6385) - Field on LogRecord passed to `emit()` - Python: [opentelemetry-python#4908](open-telemetry/opentelemetry-python#4908) - Parameter on LogRecord passed to `emit()` (only the Python prototype doesn't yet meet the definition of [qualifying prototype](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CONTRIBUTING.md#proposing-a-change) yet) Co-authored-by: Robert Pająk <pellared@hotmail.com>

xrmx · 2026-02-18T16:15:55Z

hey @xrmx! check out open-telemetry/opentelemetry-specification#4886

if you can give this PR a basic review and "lgtm" without approving/merging it, then we can treat it as a "qualifying prototype" for marking it stable in the spec. we already have 3 qualifying prototypes which is the min bar for stabilizing the spec, but the more the better!

Missed this comment but already approved the other PR 😅

… into 4907

… 4907

… into 4907

… 4907

nagkumar91

Nice implementation — spec compliance looks solid. A couple of nits below.

nagkumar91 · 2026-03-10T16:38:03Z

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

        body: AnyValue | None = None,
        attributes: _ExtendedAttributes | None = None,
        event_name: str | None = None,
+        exception: BaseException | None = None,


nit: When record is provided alongside an exception kwarg, the exception is silently dropped here (only forwarded in the else branch). The SDK's Logger.emit() explicitly handles both (exception or getattr(record, "exception", None)), so there's a subtle inconsistency between the proxy and SDK paths.

Not a contract violation (the overloads define them as separate call forms), but it might be worth either forwarding exception here too (self._logger.emit(record, exception=exception)) or adding a comment explaining why it's intentionally omitted.

nagkumar91 · 2026-03-10T16:38:03Z

opentelemetry-sdk/tests/logs/test_logs.py

+        )
+        self.assertIn(
+            "ValueError: boom",
+            attributes[exception_attributes.EXCEPTION_STACKTRACE],


nit: All tests create exceptions without raising them, so __traceback__ is None and traceback.format_exception() produces minimal output with no actual stack frames. Consider adding at least one test that raises and catches an exception to verify real stacktrace formatting:

def test_emit_with_raised_exception_has_stacktrace(self): logger, log_record_processor_mock = self._get_logger() try: raise ValueError("boom") except ValueError as exc: logger.emit(body="error", exception=exc) log_data = log_record_processor_mock.on_emit.call_args.args[0] stacktrace = dict(log_data.log_record.attributes)[exception_attributes.EXCEPTION_STACKTRACE] self.assertIn("Traceback (most recent call last)", stacktrace) self.assertIn("raise ValueError", stacktrace)

herin049 · 2026-03-10T18:02:26Z

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

        """
        # If a record is provided, use it directly
        if record is not None:
+            record_exception = exception or getattr(record, "exception", None)


Personally, I'm not a huge fan of modifying attributes of an already created log record. For example, if the attributes in the created log record are immutable, the _apply_exception_attributes function will raise an exception at runtime.

herin049 · 2026-03-10T18:21:43Z

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

+    attributes = log_record.attributes
+    if attributes:
+        if isinstance(attributes, BoundedAttributes):
+            for key, value in exception_attributes_map.items():
+                if key not in attributes:
+                    attributes[key] = value
+            return
+        merged = dict(attributes)
+        for key, value in exception_attributes_map.items():
+            merged.setdefault(key, value)
+        log_record.attributes = merged
+        return
+
+    log_record.attributes = exception_attributes_map


nit:

Suggested change

attributes = log_record.attributes

if attributes:

if isinstance(attributes, BoundedAttributes):

for key, value in exception_attributes_map.items():

if key not in attributes:

attributes[key] = value

return

merged = dict(attributes)

for key, value in exception_attributes_map.items():

merged.setdefault(key, value)

log_record.attributes = merged

return

log_record.attributes = exception_attributes_map

attributes = log_record.attributes or {}

if isinstance(attributes, BoundedAttributes):

for key, value in exception_attributes_map.items():

if key not in attributes:

attributes[key] = value

return

log_record.attributes = {

**exception_attributes_map,

**attributes,

}

Since we're 3.9 plus, I think a | b is recommended. at least we've been doing that in GenAI. I believe it calls into native code so shoudl be much quicker

I tried this shape, but it ended up surfacing type-check issues on this branch.

herin049 · 2026-03-10T18:23:18Z

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

                event_name=event_name,
+                exception=exception,
            )
+            _apply_exception_attributes(log_record, exception)


nit: If we're creating a log record from scratch (as we are here), we should be able to directly modify the attributes before constructing the record.

aabmass · 2026-03-11T03:02:01Z

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

        severity_number: Optional[SeverityNumber] = None,
        body: AnyValue = None,
        attributes: Optional[_ExtendedAttributes] = None,
+        exception: Optional[BaseException] = None,


This overload is deprecated

Suggested change

exception: Optional[BaseException] = None,

aabmass · 2026-03-11T03:05:15Z

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

+        if isinstance(attributes, BoundedAttributes):
+            for key, value in exception_attributes_map.items():
+                if key not in attributes:
+                    attributes[key] = value
+            return


What's the reason for this branch?

The branch is there to preserve BoundedAttributes behavior.

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

… into 4907

lzchen · 2026-03-12T22:28:44Z

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

+                record_exception = getattr(
+                    record.log_record, "exception", None
+                )
            if not isinstance(record, ReadWriteLogRecord):


Might not be relevant to this PR but do we know what this instance check is here? Doesn't emit expect a LogRecord and not a ReadWriteLogRecord? When would the latter be ever passed in?

ReadWriteLogRecord is an SDK-internal type, so (I guess) this branch is just a defensive fast path for already-materialized SDK records, while the public Logger.emit() API still expects callers to pass a LogRecord.

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

… 4907

iblancasa requested a review from a team as a code owner February 11, 2026 17:28

iblancasa marked this pull request as draft February 11, 2026 17:34

Add logger exception support for logs API/SDK

9c21864

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

iblancasa force-pushed the 4907 branch from 43f6746 to 9c21864 Compare February 11, 2026 17:40

iblancasa marked this pull request as ready for review February 11, 2026 17:55

This was referenced Feb 11, 2026

Stabilize the (optional) Exception parameter to Logger Emit open-telemetry/opentelemetry-specification#4858

Closed

Stabilize optional Exception parameter to Logger Emit open-telemetry/opentelemetry-specification#4886

Merged

xrmx added this to Python PR digest Feb 12, 2026

xrmx moved this to Ready for review in Python PR digest Feb 12, 2026

iblancasa added 2 commits February 17, 2026 10:54

Merge branch 'main' of github.com:open-telemetry/opentelemetry-python…

26ac99b

… into 4907

Merge branch 'main' into 4907

d226158

MikeGoldsmith approved these changes Feb 18, 2026

View reviewed changes

iblancasa and others added 6 commits February 23, 2026 12:48

Merge branch 'main' of github.com:open-telemetry/opentelemetry-python…

6581718

… into 4907

Merge branch '4907' of github.com:iblancasa/opentelemetry-python into…

0f2bcf0

… 4907

Merge branch 'main' into 4907

a42e569

Merge branch 'main' into 4907

b02a455

Merge branch 'main' of github.com:open-telemetry/opentelemetry-python…

4003ef3

… into 4907

Merge branch '4907' of github.com:iblancasa/opentelemetry-python into…

bcafcc1

… 4907

nagkumar91 reviewed Mar 10, 2026

View reviewed changes

herin049 reviewed Mar 10, 2026

View reviewed changes

aabmass reviewed Mar 11, 2026

View reviewed changes

iblancasa added 2 commits March 12, 2026 16:26

Apply changes requested in code review

9ca005d

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Merge branch 'main' of github.com:open-telemetry/opentelemetry-python…

a4c169c

… into 4907

lzchen reviewed Mar 12, 2026

View reviewed changes

lzchen approved these changes Mar 12, 2026

View reviewed changes

Fix CI

bf10796

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

iblancasa added 3 commits March 13, 2026 10:16

Merge branch 'main' into 4907

9689d30

Fix ci

b1d7f0d

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Merge branch '4907' of github.com:iblancasa/opentelemetry-python into…

bd6ba38

… 4907

Conversation

iblancasa commented Feb 11, 2026

Description

Type of change

How Has This Been Tested?

Does This PR Require a Contrib Repo Change?

Checklist:

Uh oh!

xrmx commented Feb 12, 2026

Uh oh!

trask commented Feb 13, 2026

Uh oh!

MikeGoldsmith left a comment

Choose a reason for hiding this comment

Uh oh!

xrmx commented Feb 18, 2026

Uh oh!

nagkumar91 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants