eschutho opened a new pull request, #42657:
URL: https://github.com/apache/superset/pull/42657
### 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
```bash
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
- [x] Has associated issue: companion to #42118 / #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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]