chore(reports): thread cache-key/execution-id log context through screenshot capture logs by eschutho · Pull Request #42657 · apache/superset

eschutho · 2026-07-31T21:36:34Z

SUMMARY

The screenshot capture code is reached by two call paths that identify their runs differently: scheduled reports carry an execution_id (already threaded end-to-end via log_context since #42253), while thumbnails and direct PDF/screenshot downloads are identified by their cache_key — which was never passed down. BaseScreenshot.compute_and_cache has the cache_key in hand and get_screenshot already accepts a log_context parameter, but the two were never connected: every capture-layer log line produced by a thumbnail or direct-download run is anonymous, so an "exact-cache-key production trace" (auth → navigation → readiness → capture → terminal cache state, joined on one key) is impossible on that path today. Incident investigations degenerate into filtered-log archaeology across thousands of unkeyed lines.

This PR threads the existing optional log_context through the remaining capture-layer log lines and populates it on the thumbnail path:

screenshots.py — the load-bearing change: compute_and_cache passes log_context=f"cache_key={cache_key}" into get_screenshot and resize_image; the thumbnail lifecycle lines ("trying to generate screenshot", generate/resize failures, "Updated thumbnail cache") now include the cache_key; driver() accepts log_context for its Playwright-unavailable fallback notice.
webdriver.py — the non-tiled Playwright log lines (navigation timeout, headstart sleep, element/chart-container waits, screenshot result, unexpected-error handler), the entire WebDriverSelenium.get_screenshot path, and find_unexpected_errors (both engines) now append the [cache_key=…] / [execution_id=…] suffix.
screenshot_utils.py — the non-budget tiled log lines (dashboard dimensions, tile count, scroll, capture, skip, combine) and combine_screenshot_tiles gain the same suffix.

Log-line/plumbing only — no behavior change. Split out of #42118 per its scope reduction to tiled-path budgeting only. The readiness log lines added by #42253/#42427 already carry log_context and are untouched; the report path already supplies execution_id and needs no changes.

New test pins the load-bearing wiring: compute_and_cache must call get_screenshot/resize_image with log_context="cache_key=<key>". Existing log-assertion tests updated for the added suffix argument.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable; logging only. Example log line after: Taking screenshot of url http://… as user reports_user [cache_key=a10e2389…]

TESTING INSTRUCTIONS

pytest -q tests/unit_tests/utils/   # 714 passed
ruff check / ruff format --check    # clean
mypy --check-untyped-defs superset/utils/{screenshots,webdriver,screenshot_utils}.py  # no new errors vs master

ADDITIONAL INFORMATION

Has associated issue: companion to fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill #42118 / fix(reports): positive readiness check for non-tiled screenshots #42253; enables exact-cache-key production traces for blank-screenshot investigations
Required feature flags:
Changes UI
Includes DB Migration
Introduces new feature or API
Removes existing feature or API

bito-code-review · 2026-07-31T21:36:44Z

Code Review Agent Run #c28ae9

Actionable Suggestions - 0

Additional Suggestions - 2

superset/utils/screenshots.py - 2
- Test gap: fallback log context · Line 204-211
  
  The existing test checks for string fragments in the fallback log message but doesn't verify the new `context_suffix` parameter is included when `driver(log_context=...)` is called. This means passing `log_context` to `driver()` silently has no effect on the fallback path's log output.
- Test gap: driver log_context param · Line 218-223
  
  The diff at line 223 passes `log_context` to `self.driver(...)` but the existing test `test_passes_cache_key_log_context_to_capture` only verifies `get_screenshot` and `resize_image` receive `log_context`. No test covers the `driver()` call's `log_context` parameter.

Review Details

Files reviewed - 6 · Commit Range: 560e52c..560e52c
- superset/utils/screenshot_utils.py
- superset/utils/screenshots.py
- superset/utils/webdriver.py
- tests/unit_tests/utils/screenshot_test.py
- tests/unit_tests/utils/test_screenshot_utils.py
- tests/unit_tests/utils/webdriver_test.py
Files skipped - 0
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

codecov · 2026-07-31T21:41:39Z

Codecov Report

❌ Patch coverage is 25.00000% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.44%. Comparing base (b452c16) to head (b5eca9a).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/utils/webdriver.py	40.00%	12 Missing ⚠️
superset/utils/screenshots.py	0.00%	9 Missing ⚠️
superset/utils/screenshot_utils.py	14.28%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #42657      +/-   ##
==========================================
+ Coverage   55.88%   65.44%   +9.56%     
==========================================
  Files        2810     2810              
  Lines      159375   159381       +6     
  Branches    36374    36373       -1     
==========================================
+ Hits        89062   104304   +15242     
+ Misses      69481    53034   -16447     
- Partials      832     2043    +1211

Flag	Coverage Δ
hive	`38.08% <8.33%> (-0.01%)`	⬇️
mysql	`57.82% <25.00%> (?)`
postgres	`57.86% <25.00%> (?)`
presto	`39.97% <8.33%> (-0.01%)`	⬇️
python	`59.25% <25.00%> (+19.21%)`	⬆️
sqlite	`57.49% <25.00%> (?)`
unit	`100.00% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…eenshot capture logs The screenshot capture code is reached by two call paths that identify their runs differently: scheduled reports carry an execution_id (already threaded end-to-end via log_context since #42253), while thumbnails and direct PDF/screenshot downloads are identified by their cache_key -- which was never passed down. BaseScreenshot.compute_and_cache had the cache_key in hand and get_screenshot already accepted a log_context parameter, but the two were never connected, so every capture-layer log line produced by a thumbnail or direct-download run is anonymous: there is no way to join "trying to generate screenshot" / webdriver navigation / readiness / capture-result log lines to the cached digest they were computing. This threads the existing optional log_context through the remaining capture-layer log lines, and populates it on the thumbnail path: - screenshots.py: compute_and_cache passes log_context=f"cache_key={cache_key}" into get_screenshot and resize_image; the thumbnail lifecycle log lines (generate/fail/resize/ cache-updated) now include the cache_key; driver() accepts log_context for its Playwright-unavailable fallback notice. - webdriver.py: the non-tiled Playwright log lines (navigation, headstart, element/chart-container waits, screenshot result), the entire WebDriverSelenium.get_screenshot path, and find_unexpected_errors (both engines) now append the context suffix. - screenshot_utils.py: the non-budget tiled log lines (dimensions, tile count, scroll, capture, skip, combine) and combine_screenshot_tiles gain the same suffix. Log-line/plumbing only -- no behavior change. Split out of #42118 per its scope reduction to tiled-path budgeting; the readiness log lines added by Co-Authored-By: Claude <noreply@anthropic.com>

bito-code-review · 2026-08-01T01:16:46Z

Code Review Agent Run #447e0e

Actionable Suggestions - 0

Review Details

Files reviewed - 6 · Commit Range: b5eca9a..b5eca9a
- superset/utils/screenshot_utils.py
- superset/utils/screenshots.py
- superset/utils/webdriver.py
- tests/unit_tests/utils/screenshot_test.py
- tests/unit_tests/utils/test_screenshot_utils.py
- tests/unit_tests/utils/webdriver_test.py
Files skipped - 0
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

pull-request-size Bot added the size/L label Jul 31, 2026

dosubot Bot added the logging Creates a UI or API endpoint that could benefit from logging. label Jul 31, 2026

github-actions Bot added the preset-io label Jul 31, 2026

eschutho force-pushed the fix-screenshot-log-context-tracing branch from 560e52c to b5eca9a Compare July 31, 2026 22:11

eschutho mentioned this pull request Jul 31, 2026

fix(reports): enforce dashboard readiness and execution budget #42624

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(reports): thread cache-key/execution-id log context through screenshot capture logs - #42657

chore(reports): thread cache-key/execution-id log context through screenshot capture logs#42657
eschutho wants to merge 1 commit into
masterfrom
fix-screenshot-log-context-tracing

eschutho commented Jul 31, 2026

Uh oh!

bito-code-review Bot commented Jul 31, 2026 •

edited

Loading

Code Review Agent Run #c28ae9

Uh oh!

codecov Bot commented Jul 31, 2026 •

edited

Loading

Uh oh!

bito-code-review Bot commented Aug 1, 2026 •

edited

Loading

Code Review Agent Run #447e0e

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

eschutho commented Jul 31, 2026

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

bito-code-review Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #c28ae9

Uh oh!

codecov Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bito-code-review Bot commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #447e0e

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bito-code-review Bot commented Jul 31, 2026 •

edited

Loading

codecov Bot commented Jul 31, 2026 •

edited

Loading

bito-code-review Bot commented Aug 1, 2026 •

edited

Loading