Skip to content

Commit cf39186

Browse files
cursoragentszokeasaurusrex
cursoragent
authored and
szokeasaurusrex
committed
fix(langchain): Make span_map an instance variable
1 parent 0e21fa2 commit cf39186

File tree

11 files changed

+1096
-7
lines changed

11 files changed

+1096
-7
lines changed

‎reproduction/README.md

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
# Langchain Integration Redundant Callbacks Bug Reproduction
2+
3+
This directory contains a comprehensive reproduction of the Sentry Langchain integration bug where redundant callbacks are created when a user manually configures a `SentryLangchainCallback` instance.
4+
5+
## Issue Description
6+
7+
When configuring a `SentryLangchainCallback` instance manually and passing it to a Langchain model's `invoke` call, the Sentry integration creates another instance of the same callback. This results in:
8+
9+
1. **Performance duplication**: Events being tracked twice in spans/transactions
10+
2. **Error duplication**: Issues being reported twice to Sentry's Issues dashboard
11+
3. **KeyError crashes**: Race conditions due to shared span_map causing internal SDK errors
12+
13+
## 🚨 CRITICAL: Two Interconnected Bugs Discovered
14+
15+
### Bug 1: Callback Duplication (Original Issue)
16+
17+
The `_wrap_configure` function fails to detect existing callbacks in `args[1]` (inheritable_callbacks), leading to duplicate callback instances.
18+
19+
### Bug 2: Shared span_map (Newly Discovered)
20+
21+
The `SentryLangchainCallback.span_map` is declared as a **class variable**, causing ALL instances to share the same span storage. This creates race conditions when multiple callbacks exist.
22+
23+
**Combined Effect**: The callback duplication bug triggers the span_map race condition, causing KeyErrors and preventing proper telemetry collection.
24+
25+
## What This Reproduction Shows
26+
27+
This enhanced reproduction includes **three test cases**:
28+
29+
1. **Successful Request Test**: Demonstrates duplicate spans in Sentry Performance monitoring
30+
2. **Failing Request Test**: Demonstrates duplicate issues in Sentry Issues dashboard
31+
3. **KeyError Investigation**: Shows the span_map class variable race condition
32+
33+
## Prerequisites
34+
35+
- Python 3.8 or higher
36+
- pip package manager
37+
- (Optional) A real Sentry project DSN to see the duplication in your dashboard
38+
39+
## Step-by-Step Instructions
40+
41+
### 1. Create a Virtual Environment (Recommended)
42+
43+
```bash
44+
# Create a new virtual environment
45+
python3 -m venv venv
46+
47+
# Activate the virtual environment
48+
# On Linux/Mac:
49+
source venv/bin/activate
50+
# On Windows:
51+
# venv\Scripts\activate
52+
```
53+
54+
### 2. Install Dependencies
55+
56+
```bash
57+
# Install the required packages
58+
pip install -r requirements.txt
59+
```
60+
61+
**Note**: The reproduction script uses the local `sentry_sdk` from the parent directory. If you want to run this reproduction standalone, remove the `sys.path.insert` line from `reproduce_issue.py` and ensure `sentry-sdk==2.27.0` is installed.
62+
63+
### 3. (Optional) Set Your Sentry DSN
64+
65+
To see the actual duplication in your Sentry dashboard:
66+
67+
```bash
68+
# Set your real Sentry DSN
69+
export SENTRY_DSN="https://[email protected]/your-project-id"
70+
```
71+
72+
If you don't set this, the script will use a dummy DSN and you'll only see the console output demonstrating the bug.
73+
74+
### 4. Run the Reproduction Scripts
75+
76+
#### Main Reproduction (shows callback duplication):
77+
78+
```bash
79+
python reproduce_issue.py
80+
```
81+
82+
#### KeyError Investigation (shows span_map race condition):
83+
84+
```bash
85+
python demonstrate_keyerror.py
86+
```
87+
88+
### 5. Observe the Output and Sentry Dashboard
89+
90+
#### Console Output
91+
92+
**reproduce_issue.py** will show:
93+
94+
```
95+
🔍 Inspecting callbacks in MockChatModel._generate:
96+
Handlers: 2 total
97+
➜ Found SentryLangchainCallback in handlers (id: 140445566088464)
98+
➜ Found SentryLangchainCallback in handlers (id: 140445577607568)
99+
100+
📊 Total UNIQUE SentryLangchainCallback instances: 2
101+
❌ BUG REPRODUCED: Multiple SentryLangchainCallback instances found!
102+
103+
[sentry] ERROR: Internal error in sentry_sdk
104+
KeyError: UUID('4b537014-b30e-472a-b1a0-4075d5de1f97')
105+
```
106+
107+
**demonstrate_keyerror.py** will show:
108+
109+
```
110+
🚨 CONFIRMED: Both callbacks share the same span_map!
111+
🚨 This is a class variable, not an instance variable
112+
113+
🚨 Callback 2 failed with KeyError: [UUID]
114+
🚨 This is exactly what we see in the actual error!
115+
```
116+
117+
#### Why You Might Not See Duplicate Spans/Issues
118+
119+
The reason duplicate spans/issues may not appear in Sentry is because:
120+
121+
1. **Race Condition**: The second callback overwrites the first callback's span entry
122+
2. **KeyError**: The second callback crashes before completing its work
123+
3. **Cleanup Interference**: Both callbacks interfere with each other's cleanup process
124+
125+
## Root Cause Analysis
126+
127+
### Bug 1: Callback Duplication
128+
129+
The `_wrap_configure` function in `sentry_sdk/integrations/langchain.py` checks for existing callbacks in:
130+
131+
- `kwargs["local_callbacks"]`
132+
- `args[2]` (local_callbacks parameter)
133+
134+
But it **misses** checking `args[1]` (inheritable_callbacks parameter), which is where user-provided callbacks from `RunnableConfig` are actually placed.
135+
136+
### Bug 2: Shared span_map
137+
138+
The `SentryLangchainCallback` class declares `span_map` as a class variable:
139+
140+
```python
141+
class SentryLangchainCallback(BaseCallbackHandler):
142+
span_map = OrderedDict() # ← CLASS VARIABLE! All instances share this
143+
```
144+
145+
This should be an instance variable to prevent race conditions between multiple callback instances.
146+
147+
## Expected Results
148+
149+
### With Both Bugs (Current Behavior)
150+
151+
- **Console**: Shows 2 unique SentryLangchainCallback instances
152+
- **Console**: Shows KeyError in Sentry debug output
153+
- **Sentry Performance**: May see no spans or single spans (due to race condition)
154+
- **Sentry Issues**: May see no duplicate issues (due to KeyError preventing reporting)
155+
156+
### After Both Fixes (Expected Behavior)
157+
158+
- **Console**: Shows 1 unique SentryLangchainCallback instance
159+
- **Console**: No KeyErrors in debug output
160+
- **Sentry Performance**: Single span per LLM operation
161+
- **Sentry Issues**: Single error event per failed operation
162+
163+
## Impact in Production
164+
165+
These bugs cause:
166+
167+
- **Silent failures** in telemetry collection due to KeyErrors
168+
- **Incomplete span tracking** due to race conditions
169+
- **Missing error reports** when the second callback crashes
170+
- **Unpredictable behavior** depending on callback execution timing
171+
- **SDK instability** with internal errors appearing in logs
172+
173+
## Files in this Directory
174+
175+
- `reproduce_issue.py` - Main reproduction script showing callback duplication and KeyError
176+
- `demonstrate_keyerror.py` - Detailed investigation of the span_map race condition
177+
- `requirements.txt` - Python dependencies needed to run the reproduction
178+
- `proposed_fix.py` - Shows the problematic code and the proposed fix for Bug 1
179+
- `README.md` - This file
180+
181+
## Complete Fix Required
182+
183+
Both bugs need to be fixed:
184+
185+
1. **Fix callback duplication**: Check `args[1]` in `_wrap_configure` function
186+
2. **Fix span_map sharing**: Change `span_map` from class variable to instance variable:
187+
```python
188+
def __init__(self, max_span_map_size, include_prompts, tiktoken_encoding_name=None):
189+
self.span_map = OrderedDict() # Instance variable instead of class variable
190+
# ... rest of init
191+
```

‎reproduction/proposed_fix.py

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
"""
2+
Proposed fix for the Langchain callback duplication issue.
3+
This shows the changes needed in sentry_sdk/integrations/langchain.py
4+
5+
The issue is in the _wrap_configure function around line 415.
6+
"""
7+
8+
from functools import wraps
9+
from typing import Any, List
10+
from sentry_sdk.integrations.langchain import (
11+
SentryLangchainCallback,
12+
LangchainIntegration,
13+
)
14+
from langchain_core.callbacks import BaseCallbackHandler
15+
import sentry_sdk
16+
from sentry_sdk.utils import capture_internal_exceptions, logger
17+
18+
# ============================================================================
19+
# CURRENT PROBLEMATIC CODE (from _wrap_configure function)
20+
# ============================================================================
21+
22+
23+
def _wrap_configure_CURRENT(f):
24+
"""Current implementation that has the bug"""
25+
26+
@wraps(f)
27+
def new_configure(*args, **kwargs):
28+
integration = sentry_sdk.get_client().get_integration(LangchainIntegration)
29+
if integration is None:
30+
return f(*args, **kwargs)
31+
32+
with capture_internal_exceptions():
33+
new_callbacks = [] # type: List[BaseCallbackHandler]
34+
35+
# Handle local_callbacks from kwargs or args[2]
36+
if "local_callbacks" in kwargs:
37+
existing_callbacks = kwargs["local_callbacks"]
38+
kwargs["local_callbacks"] = new_callbacks
39+
elif len(args) > 2:
40+
existing_callbacks = args[2] # ❌ ONLY checks args[2]!
41+
args = (
42+
args[0],
43+
args[1],
44+
new_callbacks,
45+
) + args[3:]
46+
else:
47+
existing_callbacks = []
48+
49+
# Copy existing callbacks to new_callbacks
50+
if existing_callbacks:
51+
if isinstance(existing_callbacks, list):
52+
for cb in existing_callbacks:
53+
new_callbacks.append(cb)
54+
elif isinstance(existing_callbacks, BaseCallbackHandler):
55+
new_callbacks.append(existing_callbacks)
56+
else:
57+
logger.debug("Unknown callback type: %s", existing_callbacks)
58+
59+
# ❌ BUG: Only checks callbacks that were copied to new_callbacks
60+
# Misses callbacks in args[1] (inheritable_callbacks)!
61+
already_added = False
62+
for callback in new_callbacks:
63+
if isinstance(callback, SentryLangchainCallback):
64+
already_added = True
65+
66+
if not already_added:
67+
new_callbacks.append(
68+
SentryLangchainCallback(
69+
integration.max_spans,
70+
integration.include_prompts,
71+
integration.tiktoken_encoding_name,
72+
)
73+
)
74+
return f(*args, **kwargs)
75+
76+
return new_configure
77+
78+
79+
# ============================================================================
80+
# FIXED IMPLEMENTATION
81+
# ============================================================================
82+
83+
84+
def _wrap_configure_FIXED(f):
85+
"""Fixed implementation that checks all callback locations"""
86+
87+
@wraps(f)
88+
def new_configure(*args, **kwargs):
89+
integration = sentry_sdk.get_client().get_integration(LangchainIntegration)
90+
if integration is None:
91+
return f(*args, **kwargs)
92+
93+
with capture_internal_exceptions():
94+
# ✅ FIX: Check if SentryLangchainCallback already exists in ANY location
95+
already_added = False
96+
97+
# Check in kwargs["local_callbacks"]
98+
if "local_callbacks" in kwargs:
99+
callbacks = kwargs.get("local_callbacks", [])
100+
if isinstance(callbacks, list):
101+
for cb in callbacks:
102+
if isinstance(cb, SentryLangchainCallback):
103+
already_added = True
104+
break
105+
106+
# ✅ KEY FIX: Check in args[1] (inheritable_callbacks)
107+
# This is where RunnableConfig callbacks are placed!
108+
if not already_added and len(args) > 1 and args[1] is not None:
109+
callbacks = args[1]
110+
if isinstance(callbacks, list):
111+
for cb in callbacks:
112+
if isinstance(cb, SentryLangchainCallback):
113+
already_added = True
114+
break
115+
elif isinstance(callbacks, BaseCallbackHandler) and isinstance(
116+
callbacks, SentryLangchainCallback
117+
):
118+
already_added = True
119+
120+
# Check in args[2] (local_callbacks)
121+
if not already_added and len(args) > 2 and args[2] is not None:
122+
callbacks = args[2]
123+
if isinstance(callbacks, list):
124+
for cb in callbacks:
125+
if isinstance(cb, SentryLangchainCallback):
126+
already_added = True
127+
break
128+
elif isinstance(callbacks, BaseCallbackHandler) and isinstance(
129+
callbacks, SentryLangchainCallback
130+
):
131+
already_added = True
132+
133+
# Only add a new callback if none was found in ANY location
134+
if not already_added:
135+
new_callbacks = [] # type: List[BaseCallbackHandler]
136+
137+
# Preserve existing callbacks from local_callbacks
138+
if "local_callbacks" in kwargs:
139+
existing_callbacks = kwargs["local_callbacks"]
140+
kwargs["local_callbacks"] = new_callbacks
141+
elif len(args) > 2:
142+
existing_callbacks = args[2]
143+
args = (
144+
args[0],
145+
args[1],
146+
new_callbacks,
147+
) + args[3:]
148+
else:
149+
existing_callbacks = []
150+
151+
# Copy existing callbacks
152+
if existing_callbacks:
153+
if isinstance(existing_callbacks, list):
154+
for cb in existing_callbacks:
155+
new_callbacks.append(cb)
156+
elif isinstance(existing_callbacks, BaseCallbackHandler):
157+
new_callbacks.append(existing_callbacks)
158+
else:
159+
logger.debug("Unknown callback type: %s", existing_callbacks)
160+
161+
# Add our callback
162+
new_callbacks.append(
163+
SentryLangchainCallback(
164+
integration.max_spans,
165+
integration.include_prompts,
166+
integration.tiktoken_encoding_name,
167+
)
168+
)
169+
170+
return f(*args, **kwargs)
171+
172+
return new_configure
173+
174+
175+
# ============================================================================
176+
# SUMMARY OF CHANGES
177+
# ============================================================================
178+
179+
"""
180+
Key changes in the fix:
181+
182+
1. EARLY DETECTION: Check for existing SentryLangchainCallback instances
183+
BEFORE modifying any callback lists
184+
185+
2. COMPREHENSIVE CHECKING: Check ALL possible callback locations:
186+
- kwargs["local_callbacks"]
187+
- args[1] (inheritable_callbacks) ← THE MISSING CHECK!
188+
- args[2] (local_callbacks)
189+
190+
3. PROPER LOGIC: Only create and add a new callback if NO existing
191+
SentryLangchainCallback is found in any location
192+
193+
The original bug occurred because the function only checked args[2] for
194+
existing callbacks, but user-provided callbacks from RunnableConfig
195+
are actually placed in args[1] (inheritable_callbacks).
196+
197+
This fix ensures that manually configured SentryLangchainCallback
198+
instances are properly detected and respected, preventing duplication.
199+
"""

0 commit comments

Comments
 (0)