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]

Reply via email to