feat(preview): add monitoring dimensions + preload outcome [PREVIEW-224]#1639
Open
ahorowitz123 wants to merge 9 commits into
Open
feat(preview): add monitoring dimensions + preload outcome [PREVIEW-224]#1639ahorowitz123 wants to merge 9 commits into
ahorowitz123 wants to merge 9 commits into
Conversation
Accept access_pattern, preview_mode, shared_link_auth as host-supplied options and attach them to every emitted metric/error. Expose getPreloadStatus() on BaseViewer and override in doc/video viewers so emitLoadMetrics can tag the load event with preload_status and emit a new preview_preload_outcome counter. PREVIEW-224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reads the existing Logger.log.cache.hit signal (set by Preview loadFromCache) and maps it to a hit/miss string on every emitted event. Gives the monitoring dashboard per-metric visibility into file metadata cache effectiveness without a join against the preview session log. PREVIEW-224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DocBaseViewer.getPreloadStatus() reads this.preloader.loadTime to report preload hit/miss on emitted metrics, but only DocFirstPreloader set loadTime. Standard DocPreloader and PresentationPreloader never did, so the dashboard always saw miss. Set loadTime when the preload element is revealed, matching the existing DocFirstPreloader pattern. PREVIEW-224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Redefine the two cache/prefetch dimensions to match what the dashboard actually wants to measure: - preload_status: did the host call Preview.prefetch(fileId) for this file before the full preview load? Tracked via a new prefetchedFiles Set populated in prefetch(). Replaces the brittle viewer-level getPreloadStatus() logic that relied on loadTime/wrapperEl signals that were often undefined even when preload worked. - prefetch_status: did the file-metadata cache (previewService.cache / Logger.setCached) serve this load? Renamed from file_info_cache_status to match its real meaning. Also removes BaseViewer.getPreloadStatus() and the doc/video overrides, the DocPreloader/PresentationPreloader loadTime writes, and the isConnected fallback in DocBaseViewer — all unnecessary now. PREVIEW-224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously these tags were attached to every emitted event via
emitLogEvent, which would skew the dashboard denominator since events
like thumbnail open, find bar open, and perf metrics all fired on
every session.
Now the tags live only on the three events where they are actually
needed: the load event, the preview_preload_outcome counter, and
preview_error. Exactly one of {load, error} fires per session, giving
a clean hit-rate numerator/denominator.
Extracted the computation into getLoadStateTags() so all three
callers share the same logic.
PREVIEW-224
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The ContentPreview wrapper creates a new Preview instance per preview session, while host apps call prefetch() on a different long-lived Preview instance in the file list / service layer. An internal Set of prefetched file IDs therefore lives on the wrong instance and is always empty at emit time. Accept preloadStatus as an option instead, matching how access_pattern, preview_mode, and shared_link_auth are already host-supplied. Host apps (EUA, preview-client) now compute the hit/miss value themselves based on their own prefetch tracking. Drops the internal prefetchedFiles Set and the .add() call in prefetch(). PREVIEW-224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lets the host supply both cache dimensions (preload_status and prefetch_status) explicitly rather than Preview inferring prefetch_status from its own Logger.cache.hit. Same architecture issue as preload_status: the host often calls prefetch() on a different Preview instance than the one running the session, so any Preview-local bookkeeping drifts out of sync. When the host omits either option, the corresponding field is left off the emitted payload. PREVIEW-224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The host-supplied values were recording 'did the host initiate a prefetch/preload attempt' rather than 'did it actually succeed before the user opened preview'. Users reported seeing hit values even while the preview was clearly still spinning on a network fetch. Derive both dimensions inside the SDK from real session-end signals: - preload_status: set when VIEWER_EVENT.preload fires (the viewer actually rendered the preload representation to the DOM) - prefetch_status: read from Logger.log.cache.hit which is set in loadFromCache() when the SDK skips the file-info API call entirely Drops the host preloadStatus / prefetchStatus options. PREVIEW-224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Doc viewers emit 'preload' on the preloader which DocBaseViewer handles locally in onPreload() and calls logger.setPreloaded() on — the event never bubbles up to Preview.handleViewerEvents so this.preloadEmitted was always false for docs. Fall back to this.logger.log.time.preload (set by setPreloaded) when the VIEWER_EVENT.preload case didn't fire. PREVIEW-224 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
access_pattern,preview_mode,shared_link_auth) as options onPreviewand attach them to every emittedpreview_metric/preview_errorpayload viaemitLogEvent.BaseViewer.getPreloadStatus()returning'na'by default; override inDocBaseViewer(this.preloader.loadTime→ hit/miss) andVideoBaseViewer(this.preloader.wrapperEl→ hit/miss). Generalizes the preload-hit signal beyond the doc + WebP-only path.emitLoadMetrics, tag theloadevent withpreload_statusand emit a newpreview_preload_outcomecounter event so the dashboard can compute preload hit rate directly.Why
Part of the Preview Performance Monitoring dashboard rollout under WEBAPP-53337. The dashboard needs to slice TTPL by access pattern, preview mode, shared-link auth, and preload hit/miss — none of which were tagged today. Companion host-side work lands in the EndUserApp, preview-client, and box-ui-elements tickets (PREVIEW-225/226/227).
Ticket
PREVIEW-224
Test plan
🤖 Generated with Claude Code