Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Comments

fix(logging): make LoggerAdapter OpenTelemetry-safe (Issue #837)#1283

Open
harsh543 wants to merge 11 commits intotemporalio:main from
harsh543:fix/loggeradapter-otel-extra-837
Open

fix(logging): make LoggerAdapter OpenTelemetry-safe (Issue #837) #1283
harsh543 wants to merge 11 commits intotemporalio:main from
harsh543:fix/loggeradapter-otel-extra-837

Conversation

@harsh543
Copy link

@harsh543 harsh543 commented Jan 20, 2026 •
edited
Loading

Summary

This PR adds an OTel-safe logging mode for Temporal's Python LoggerAdapter classes, addressing Issue #837 where nested dicts in LogRecord.extra break OpenTelemetry logging pipelines.

Problem

Temporal's workflow.LoggerAdapter and activity.LoggerAdapter inject nested dictionaries into LogRecord.extra:

extra["temporal_workflow"] = {"workflow_id": "...", "run_id": "...", ...}

OpenTelemetry log attributes must be AnyValue types (scalars, not nested dicts). This causes Temporal context to be dropped or cause errors in OTel pipelines.

Solution

Add a temporal_extra_mode attribute to both LoggerAdapter classes with two modes:

Mode Behavior Use Case
"dict" (default) Nested dict under temporal_workflow/temporal_activity Backward compatibility
"flatten" Scalar attributes with temporal.workflow.* / temporal.activity.* prefixes OpenTelemetry

Usage

from temporalio import workflow, activity
# For OpenTelemetry compatibility:
workflow.logger.temporal_extra_mode = "flatten"
activity.logger.temporal_extra_mode = "flatten"
# Log records now have flat attributes:
# - temporal.workflow.workflow_id
# - temporal.workflow.workflow_type
# - temporal.workflow.run_id
# - temporal.activity.activity_id
# etc.

Changes

  • New: temporalio/_log_utils.py - Shared helper functions and TemporalLogExtraMode type
  • Modified: temporalio/workflow.py - Added temporal_extra_mode to LoggerAdapter
  • Modified: temporalio/activity.py - Added temporal_extra_mode to LoggerAdapter
  • New: tests/test_log_utils.py - Unit tests for all modes
  • Modified: tests/worker/test_workflow.py - Parameterized integration tests for workflow logging modes
  • Modified: tests/worker/test_activity.py - Parameterized integration tests for activity logging modes

Verification Checklist

  • Default mode ("dict") preserves existing behavior - no breaking changes
  • Flatten mode produces only scalar values (OTel-safe)
  • Flatten mode uses temporal.workflow.* and temporal.activity.* prefixes
  • Flattened keys don't conflict with LogRecord core attributes
  • Non-primitive values converted to strings in flatten mode
  • Global logger settings restored in finally blocks
  • Type checking passes (pyright)
  • Linting passes (ruff)
  • Unit tests cover all modes and edge cases
  • Integration tests verify workflow and activity adapters

Test Plan

Verify default behavior unchanged:

# Should still produce nested dict
assert "temporal_workflow" in record.__dict__
assert isinstance(record.__dict__["temporal_workflow"], dict)

Verify flatten mode is OTel-safe:

workflow.logger.temporal_extra_mode = "flatten"
# Should produce flat scalar attributes
assert "temporal.workflow.workflow_id" in record.__dict__
assert "temporal_workflow" not in record.__dict__
# All values are primitives
for key, value in record.__dict__.items():
 if key.startswith("temporal.workflow."):
 assert isinstance(value, (str, int, float, bool, type(None)))

Fixes #837

🤖 Generated with Claude Code

@harsh543 harsh543 requested a review from a team as a code owner January 20, 2026 03:14
Copy link

CLAassistant commented Jan 20, 2026 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

harsh543 and others added 3 commits January 27, 2026 23:06 
- Add TestFlattenModeOTelSafety class with critical assertions
- Verify zero dict values exist for temporal keys in flatten mode
- Verify legacy nested keys (temporal_workflow, temporal_activity) don't exist
- Test both workflow and activity contexts
- Test update context handling in flatten mode
- Remove json mode (no known use case, simplifies code)
- Merge key/prefix params into single key param (prefix derived via replace)
- Revert unrelated README changes
- Parameterize unit tests, remove json-mode tests
- Remove json integration tests from test_activity and test_workflow
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@harsh543 harsh543 force-pushed the fix/loggeradapter-otel-extra-837 branch from a271c94 to 2307d25 Compare January 28, 2026 07:07 
...anges
- Merge test_activity_logging and test_activity_logging_flatten_mode
 into a single pytest-parameterized test covering dict and flatten modes
- Revert README import path changes that were unrelated to this PR
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@harsh543 harsh543 force-pushed the fix/loggeradapter-otel-extra-837 branch from 2d556bb to 3e29635 Compare February 1, 2026 00:36
Copy link
Author

@harsh543 harsh543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is now:

  • ✅ Tightly scoped to solving the OTel logging issue
  • ✅ Free of unrelated README changes
  • ✅ Simplified API with clear use cases
  • ✅ Fully backward compatible by default
  • ✅ Thoroughly tested with integration tests for both modes

Addressed feedback:

  • Removed json mode (no use case identified)
  • Reverted all README changes
  • Parameterized duplicate tests (test_activity_logging[dict/flatten],
    test_workflow_logging[dict/flatten])
 

- Add try/finally to restore temporal_extra_mode and full_workflow_info_on_extra
- Add clear phase comments: first execution, replay path, replay assertions
- Single parameterized test validates: extra mode formatting (dict/flatten),
 replay suppression, and full_workflow_info_on_extra
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@harsh543 harsh543 force-pushed the fix/loggeradapter-otel-extra-837 branch from de3217c to 3b51ef2 Compare February 3, 2026 08:05
Copy link
Author

harsh543 commented Feb 3, 2026

Re: test changes

Good feedback — consolidated into a single parameterized test that validates all concerns:

What test_workflow_logging[dict/flatten] now tests:

  1. Dict/flatten formatting — parameterized to verify LogRecord.extra structure for each mode
  2. Replay suppression — worker restart mid-workflow triggers history replay; asserts old logs don't reappear
  3. full_workflow_info_on_extra — verifies workflow.Info object is attached when enabled

Structure:

# --- First execution: logs should appear ---
await handle.signal(LoggingWorkflow.my_signal, "signal 1")
...
assert capturer.find_log("Signal: signal 1")
# --- Clear logs and continue execution (replay path) ---
capturer.log_queue.queue.clear()
# --- Replay execution: no duplicate logs ---
assert not capturer.find_log("Signal: signal 1") # suppressed during replay
assert capturer.find_log("Signal: signal 3") # new execution logs normally

Cleanup:

finally:
 workflow.logger.temporal_extra_mode = original_mode
 workflow.logger.full_workflow_info_on_extra = original_full_info

No separate replay test needed — one test, clear phases, all concerns covered.

@harsh543 harsh543 marked this pull request as draft February 3, 2026 08:12
@harsh543 harsh543 marked this pull request as ready for review February 3, 2026 08:13
Copy link
Author

harsh543 commented Feb 6, 2026

Fixed CI lint failures in 730abbf:

  • Ran ruff format on tests/test_log_utils.py and tests/worker/test_workflow.py
  • Changes are purely stylistic (line wrapping, assertion formatting)
  • No functional code changes

Safe to re-run CI.

 

Four type errors caused all 11 CI jobs to fail at the lint-types step:
- tests/test_log_utils.py: annotate `mode` as `TemporalLogExtraMode`
 instead of `str` so it is compatible with _apply_temporal_context_to_extra
 and _update_temporal_context_in_extra signatures
- tests/worker/test_activity.py: same fix for `temporal_extra_mode` param
- tests/worker/test_workflow.py: same fix for `temporal_extra_mode` param
- temporalio/_log_utils.py: suppress basedpyright reportUnusedFunction
 on both helper functions; basedpyright exits 1 on warnings, and it
 does not track cross-module usage of _-prefixed functions even though
 both are explicitly imported and called from activity.py and workflow.py
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Author

CI lint failures fixed — please re-run CI

All 11 build-lint-test jobs were failing at step 12 (Run poe lint), specifically at lint-types[0] (pyright). The sequence aborted before mypy or basedpyright even ran.

Root causes & fixes (commit 302af5d)

1. pyright: 4 type errors — str not assignable to TemporalLogExtraMode

TemporalLogExtraMode = Literal["dict", "flatten"] is narrower than str. Three test functions had parameters annotated as str that were then passed to functions/attributes expecting TemporalLogExtraMode:

File Line Fix
tests/test_log_utils.py 98 mode: str → mode: TemporalLogExtraMode
tests/worker/test_activity.py 1016 temporal_extra_mode: str → temporal_extra_mode: TemporalLogExtraMode
tests/worker/test_workflow.py 1996 temporal_extra_mode: str → temporal_extra_mode: TemporalLogExtraMode

2. basedpyright: 2 warnings → exit code 1

basedpyright exits 1 even on warnings. The new temporalio/_log_utils.py triggered reportUnusedFunction on both _apply_temporal_context_to_extra and _update_temporal_context_in_extra — basedpyright's rule does not track cross-module usage of _-prefixed functions, even though both are explicitly imported and called in activity.py and workflow.py.

Fix: added # pyright: ignore[reportUnusedFunction] on both function definitions (consistent with the project's pattern of targeted per-site suppressions).

Verified locally (replicating CI environment)

ruff check --select I → All checks passed ✅
ruff format --check → All checks passed ✅
pyright (changed files) → 0 errors ✅
mypy (changed files) → 0 errors ✅
basedpyright (changed files) → 0 errors, 0 warnings ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tconley1428 tconley1428 Awaiting requested review from tconley1428

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature Request] Make Temporal logger adapter accomodate to OpenTelemetry

AltStyle によって変換されたページ (->オリジナル) /