Skip to content

fix: gracefully handle missing mocks for pre-app-start queries in replay mode#92

Merged
sohankshirsagar merged 3 commits intomainfrom
sohan/noop-for-missing-preappstart-spans
Apr 13, 2026
Merged

fix: gracefully handle missing mocks for pre-app-start queries in replay mode#92
sohankshirsagar merged 3 commits intomainfrom
sohan/noop-for-missing-preappstart-spans

Conversation

@sohankshirsagar
Copy link
Copy Markdown
Contributor

Summary

During replay mode, pre-app-start queries (e.g. Django's migration checks, table introspection) that weren't captured in the original trace would raise RuntimeError, crashing the replay. These framework-internal queries only occur with certain server configurations (e.g. runserver vs gunicorn) and are safe to return empty results for—Django handles "no tables found" gracefully.

This change makes all database and Redis instrumentations return a no-op/empty result for pre-app-start queries when no mock is found, instead of raising an error. Post-app-start queries with missing mocks still raise RuntimeError to catch genuine replay issues.

Changes

  • psycopg2 (instrumentation.py): _replay_execute and _replay_executemany now delegate to self._noop_execute(cursor) for pre-app-start queries with no mock, logging a warning instead of raising.
  • psycopg (instrumentation.py): Same fix applied to _replay_execute, _replay_executemany, _replay_stream (returns empty generator), and _replay_copy (yields empty MockCopy).
  • sqlalchemy (instrumentation.py): before_cursor_execute sets mock_result to an empty result dict ({"rows": [], "rowcount": 0, "description": None}) for pre-app-start queries, allowing the normal after_cursor_execute flow to proceed.
  • redis (instrumentation.py): _replay_execute_command returns self._get_default_response(command_name) (existing per-command-type defaults) and _replay_pipeline_execute returns [] for pre-app-start commands.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="drift/instrumentation/psycopg/instrumentation.py">

<violation number="1" location="drift/instrumentation/psycopg/instrumentation.py:589">
P1: Using `_noop_execute` here does not install mock fetch handlers, so missing pre-app-start mocks may still fail when code fetches rows instead of returning a deterministic empty replay result.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread drift/instrumentation/psycopg/instrumentation.py Outdated
Comment thread drift/instrumentation/psycopg2/instrumentation.py
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="drift/instrumentation/psycopg/instrumentation.py">

<violation number="1" location="drift/instrumentation/psycopg/instrumentation.py:566">
P1: `_noop_execute` assigns sync fetch methods even on async cursors, which breaks async replay reads (`await cursor.fetchone()` will fail).</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread drift/instrumentation/psycopg/instrumentation.py
@sohankshirsagar sohankshirsagar added the Tusk - Pause For Current PR Pause Tusk for future commits in the current PR label Apr 13, 2026
@tusk-dev
Copy link
Copy Markdown
Contributor

tusk-dev Bot commented Apr 13, 2026

Generated 41 tests - 41 passed

Commit tests View tests

Tip

New to Tusk Unit Tests? Learn more here.

Test Summary

  • PsycopgInstrumentation._noop_execute - 5 ✓
  • PsycopgInstrumentation._replay_copy - 4 ✓
  • PsycopgInstrumentation._replay_execute - 5 ✓
  • PsycopgInstrumentation._replay_stream - 4 ✓
  • Psycopg2Instrumentation._replay_execute - 5 ✓
  • Psycopg2Instrumentation._replay_executemany - 4 ✓
  • RedisInstrumentation._replay_execute_command - 5 ✓
  • RedisInstrumentation._replay_pipeline_execute - 4 ✓
  • before_cursor_execute - 5 ✓

Results

Tusk's tests validate the core behavior change: gracefully handling missing mocks for pre-app-start queries across all database and Redis instrumentations. All 41 tests pass, confirming that pre-app-start queries without mocks now return safe no-op/empty results instead of crashing, while post-app-start queries still raise RuntimeError to catch genuine replay issues. The test suite covers critical paths like _replay_execute and _replay_executemany in psycopg2, streaming and copy operations in psycopg, Redis command and pipeline execution, and SQLAlchemy's before_cursor_execute hook. Span lifecycle management is validated across all paths to prevent tracing leaks. The changes maintain backward compatibility for mocked queries while fixing the replay crash that occurred with framework-internal queries (e.g., Django migrations) that weren't captured in the original trace.

View check history

Commit Unit Tests Created (UTC)
e6b9998 26 ✓, 0 ✗ · Tests Apr 13, 2026 7:31PM
e2cfa88 41 ✓, 0 ✗ · Tests Apr 13, 2026 9:47PM

Was Tusk helpful? Give feedback by reacting with 👍 or 👎

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 89b815b. Configure here.

Comment thread drift/instrumentation/psycopg/instrumentation.py
@sohankshirsagar sohankshirsagar merged commit 9104222 into main Apr 13, 2026
26 checks passed
@sohankshirsagar sohankshirsagar deleted the sohan/noop-for-missing-preappstart-spans branch April 13, 2026 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tusk - Pause For Current PR Pause Tusk for future commits in the current PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants