fix(langchain): changed dictionary access from spans[run_id] to spans.get(run_id) #3403

Shivamp629 · 2025-10-09T15:27:00Z

Problem

When using LangChain with other tracing systems (e.g., LangSmith), the OpenTelemetry instrumentation crashes with a KeyError because it tries to access run_ids that weren't created by OpenTelemetry.

Solution

Changed dictionary access from spans[run_id] to spans.get(run_id) for safe lookup
Added conditional check to only inject tracing headers when OpenTelemetry created the span
Added debug logging when skipping header injection for external run_ids

Testing

Tested with LangSmith tracing enabled alongside OpenTelemetry
No more KeyError exceptions when external systems create run_ids

Important

Changed dictionary access in _OpenAITracingWrapper to use .get() for safe handling of missing run_id keys, with conditional header injection and logging.

Behavior:
- Changed dictionary access from spans[run_id] to spans.get(run_id) in _OpenAITracingWrapper to handle missing keys.
- Header injection into extra_headers only occurs if span_holder is found.
- Logs a debug message if run_id is from another system (e.g., LangSmith) and no span is found.

^{This description was created by}^{for 27213ef. You can customize this summary. It will automatically update as commits are pushed.}

I have added tests that cover my changes.
If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
(If applicable) I have updated the documentation accordingly.

Summary by CodeRabbit

Bug Fixes
- Improved robustness of tracing header injection by implementing safe lookups for spans. When spans are unavailable, the system now gracefully handles the condition with debug logging instead of failing, ensuring more reliable telemetry collection.

…safely handle missing keys

CLAassistant · 2025-10-09T15:28:43Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Shivamp629
❌ shivampatel-bwater
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

ellipsis-dev

Important

Looks good to me! 👍

Reviewed everything up to 27213ef in 1 minute and 23 seconds. Click for details.

Reviewed 41 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 2 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py:232

Draft comment:
Good change: using .get(run_id) avoids a potential KeyError when a run_id from an external system is encountered.
Reason this comment was not posted:
Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, as it praises the change without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest improvements, which makes it non-compliant with the rules.

2. packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py:246

Draft comment:
The debug log for missing span is clear. Consider adding tests to cover cases when the span is missing to ensure that header injection is properly skipped.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the comment is about actual code changes (the new error handling path), suggesting tests is often a good practice. However, this feels more like a "nice to have" suggestion rather than a critical issue. The code already handles the case gracefully with proper logging. The comment doesn't point out any actual problems with the implementation. The comment could be seen as valuable since test coverage is important for error handling paths. Missing tests could lead to regressions in this functionality. While test coverage is good, the comment doesn't identify any actual issues. The code change is straightforward error handling with logging, and we should trust that the author has tested their changes appropriately. The comment should be removed as it's more of a "nice to have" suggestion rather than identifying a concrete issue that needs to be fixed.

Workflow ID: wflow_xUHj7NgYWqOT4oGc

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

coderabbitai · 2025-10-09T15:31:59Z

Walkthrough

The change replaces direct span dictionary access with a safe get() lookup in the OpenAI tracing wrapper. When spans are missing, a debug log is emitted instead of raising an error. Header injection logic remains unchanged when spans exist.

Changes

Cohort / File(s)	Summary
OpenAI tracing wrapper span safety `packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py`	Replaces direct span access `self._callback_manager.spans[run_id]` with safe `get(run_id)` lookup, emitting debug log when span is absent and skipping header injection

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A safe path through the span-land deep,
Where missing traces no longer make us weep,
With gentle .get() and logs so bright,
We handle gracefully what once caused fright! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "fix(langchain): changed dictionary access from spans[run_id] to spans.get(run_id)" directly and specifically describes the main technical change in the code. The title accurately reflects the key modification made to the `_OpenAITracingWrapper` class, where direct dictionary access is replaced with a safer `.get()` method to handle cases where the run_id might not exist. The title is concise, clear, and specific enough that a teammate reviewing the commit history would immediately understand the primary change without needing additional context.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 383ed2a and 55c5f40.

📒 Files selected for processing (1)

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules

Files:

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e66894f and 27213ef.

📒 Files selected for processing (1)

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules

Files:

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py

.../opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py

Shivamp629 · 2025-10-10T14:08:28Z

@coderabbitai generate docstrings

coderabbitai · 2025-10-10T14:09:20Z

✅ Actions performed

Docstrings generation started.

@Shivamp629

Docstrings generation was requested by @Shivamp629. * #3403 (comment) The following files were modified: * `packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py` * `packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py`

coderabbitai · 2025-10-10T14:10:06Z

Note

Generated docstrings for this pull request at #3406

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py (1)
4-4: Remove unused imports.

MagicMock and patch are imported but not used anywhere in the test file.

Apply this diff to clean up the imports:
-from unittest.mock import MagicMock, Mock, patch
+from unittest.mock import Mock

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5afa753 and 383ed2a.

📒 Files selected for processing (1)

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules

Files:

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py

🧬 Code graph analysis (1)

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py (2)

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/callback_handler.py (1)

TraceloopCallbackHandler (147-908)

packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py (1)

_OpenAITracingWrapper (218-256)

🪛 Flake8 (7.3.0)

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py

[error] 4-4: 'unittest.mock.MagicMock' imported but unused

(F401)

[error] 4-4: 'unittest.mock.patch' imported but unused

(F401)

🪛 Ruff (0.13.3)

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py

28-28: Unused function argument: instrument_legacy

(ARG001)

71-71: Unused function argument: instrument_legacy

(ARG001)

116-116: Unused function argument: instrument_legacy

(ARG001)

🔇 Additional comments (5)

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py (5)

13-16: LGTM!

The fixture correctly creates a TraceloopCallbackHandler instance with the provided tracer provider.

19-24: LGTM!

The fixture appropriately creates a mock run manager with a unique run_id for testing external run_id scenarios.

70-113: LGTM! Strong integration test for internal run_id handling.

The test effectively validates that:

Internal run_ids (present in spans dictionary) trigger header injection

The traceparent header is properly set

The wrapped function executes successfully

Good practice using a real span for realistic testing and properly cleaning up with span.end().

116-138: LGTM! Good coverage for the no run_manager edge case.

The test correctly validates that:

Missing run_manager doesn't cause issues

No headers are injected when there's no run_manager

The wrapped function executes normally

This complements the other tests by ensuring robustness when run_manager is absent.

27-68: Approve test coverage for external run_id scenario

Validates no KeyError, proper debug logging, and no header injection when encountering external run IDs; wrapped function call remains unaffected

instrument_legacy fixture provides essential instrumentation setup and must remain intact

nirga · 2025-10-22T07:37:02Z

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py

please remove this - it's not testing anything

nirga · 2025-10-22T07:44:13Z

@Shivamp629 please fix failing lint and tests

Changed dictionary access from spans[run_id] to spans.get(run_id) to …

27213ef

…safely handle missing keys

Shivamp629 changed the title ~~Changed dictionary access from spans[run_id] to spans.get(run_id) to …~~ Fix KeyError in LangChain instrumentation when using external tracing systems Oct 9, 2025

Shivamp629 changed the title ~~Fix KeyError in LangChain instrumentation when using external tracing systems~~ Changed dictionary access from spans[run_id] to spans.get(run_id) to … Oct 9, 2025

ellipsis-dev bot reviewed Oct 9, 2025

View reviewed changes

coderabbitai bot reviewed Oct 9, 2025

View reviewed changes

.../opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py Outdated Show resolved Hide resolved

Shivamp629 changed the title ~~Changed dictionary access from spans[run_id] to spans.get(run_id) to …~~ Changed dictionary access from spans[run_id] to spans.get(run_id) Oct 9, 2025

Remove verbose comments

5afa753

Shivamp629 changed the title ~~Changed dictionary access from spans[run_id] to spans.get(run_id)~~ fix(langchain): changed dictionary access from spans[run_id] to spans.get(run_id) Oct 10, 2025

Add tests for external run_id handling

383ed2a

coderabbitai bot mentioned this pull request Oct 10, 2025

📝 Add docstrings to shivampatel/fix-key-issue #3406

Open

coderabbitai bot reviewed Oct 10, 2025

View reviewed changes

nirga approved these changes Oct 22, 2025

View reviewed changes

packages/opentelemetry-instrumentation-langchain/tests/test_external_run_id.py Outdated

Copy link

Member

nirga Oct 22, 2025

Choose a reason for hiding this comment

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

please remove this - it's not testing anything

shivampatel-bwater and others added 2 commits October 22, 2025 21:57

fix linter and remove test

f45add1

Merge branch 'main' into shivampatel/fix-key-issue

55c5f40

nirga merged commit 97cbcd9 into traceloop:main Oct 23, 2025
8 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(langchain): changed dictionary access from spans[run_id] to spans.get(run_id) #3403

fix(langchain): changed dictionary access from spans[run_id] to spans.get(run_id) #3403

Uh oh!

Shivamp629 commented Oct 9, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

CLAassistant commented Oct 9, 2025 •

edited

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

coderabbitai bot commented Oct 9, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Shivamp629 commented Oct 10, 2025

Uh oh!

coderabbitai bot commented Oct 10, 2025

Uh oh!

coderabbitai bot commented Oct 10, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

nirga Oct 22, 2025

Uh oh!

nirga commented Oct 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix(langchain): changed dictionary access from spans[run_id] to spans.get(run_id) #3403

fix(langchain): changed dictionary access from spans[run_id] to spans.get(run_id) #3403

Uh oh!

Conversation

Shivamp629 commented Oct 9, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Testing

Summary by CodeRabbit

Uh oh!

CLAassistant commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Shivamp629 commented Oct 10, 2025

Uh oh!

coderabbitai bot commented Oct 10, 2025

Uh oh!

coderabbitai bot commented Oct 10, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

nirga Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

nirga commented Oct 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Shivamp629 commented Oct 9, 2025 •

edited by coderabbitai bot

Loading

CLAassistant commented Oct 9, 2025 •

edited

Loading

coderabbitai bot commented Oct 9, 2025 •

edited

Loading