feat(shared): user-level AdSense impression and click analytics - #6526
Conversation
…nt shape AdSense units now log the same analytics interactions the internal ads do, in the same shape: event_name impression/click, target_type 'ad', the unit id as target_id and 'adsense' as ad_provider_id — one query covers every ad on the platform and GROUP BY ad_provider_id splits the demand sources. The impression is the MRC viewable impression the internal ads already use (useViewability, armed only after a creative fills). Clicks are inferred the only way a cross-origin creative allows: focus landing on the ad iframe as the window blurs — the standard AdSense click proxy, logged once per slot, with Google's own reporting remaining the exact count. The bespoke view/click event names are gone before any dashboard could depend on them; the remaining adsense-named events (request/fill/empty/error/test mode) are delivery diagnostics with no internal-ads equivalent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rebelchris
left a comment
There was a problem hiding this comment.
Summary
Solid, well-documented direction — a first-party per-user signal for AdSense is genuinely useful, the viewability arming is reused rather than reinvented, and the click inference is honestly caveated in the description. The blocking concern is not the mechanism, it is the event taxonomy: this reuses impression/click + target_type: 'ad', which are already occupied by internal ads with different definitions, so the merge silently changes what existing internal-ad queries return.
Details inline. CI is green (typecheck_strict_changed, test_shared, test_webapp, build_extension all pass).
Cross-cutting: blast radius on existing ad dashboards
Every current ClickHouse query that filters event_name IN ('impression','click') AND target_type = 'ad' without an ad_provider_id predicate starts absorbing AdSense volume the moment this merges. That is the stated goal, but it is also a retroactive change to internal-ad reporting, and:
target_idis a different namespace on each side — internal ads sendad.source(adLogEvent,lib/feed.ts:168), this sends the AdSense ad-unit id. Grouping bytarget_idfortarget_type='ad'now mixes demand-source names with numeric unit ids.- internal
clickis an actual anchor click; this one is an inference. Blended under one event name, a combined CTR mixes an exact and a proxy metric.
Can we get Ido's sign-off on the taxonomy before merge, and land it together with the ad_provider_id predicate added to the existing internal-ads dashboards? Worth stating in the description which saved queries need updating.
Reviewed by AI.
| trackingKey: `${surface}:${slot}:${unitId}:${format}`, | ||
| onViewable: (data) => { | ||
| logSlotEvent(LogEvent.ViewAdsenseSlot, viewabilityLogExtra(data)); | ||
| logAdInteraction(LogEvent.Impression, viewabilityLogExtra(data)); |
There was a problem hiding this comment.
Blocking — this is a viewable impression, not an impression.
The repo already draws exactly this distinction, in packages/shared/src/lib/ads.ts:5-12:
Impression = 'impression',
// Rendered and seen by IAB rules, unlike `Impression` which only means the
// creative reached the viewport.
Viewable = 'viewable impression',Internal ads log both: impression when the creative reaches the viewport (useLogImpression.ts:83-92, via logEventStart/logEventEnd, so it is a duration event carrying event_duration), and viewable impression from useViewability + viewabilityLogExtra (AdAsComment.tsx:64-68, PostSidebarAdWidget.tsx). This PR arms the strict MRC measurement and files it under the loose name, as a point event.
Failing scenario: the headline query the PR is built for — GROUP BY ad_provider_id on event_name='impression' AND target_type='ad' — returns viewport impressions for internal demand and MRC-viewable impressions for AdSense. AdSense's denominator is strictly smaller, so its CTR reads systematically higher than internal inventory for reasons that have nothing to do with the ads. Anything monetization decides off that comparison is wrong in a consistent direction, and nothing in the event shape reveals it.
Suggested direction: log this as AdActions.Viewable to match AdAsComment, and if a comparable loose impression is wanted, emit AdActions.Impression at fill (where hasLoggedFill already flips) so both providers populate both events with the same meaning. Using the AdActions enum here rather than LogEvent would also keep AdSense consistent with the rest of the ads surface.
Reviewed by AI.
|
|
||
| window.addEventListener('blur', onWindowBlur); | ||
| return () => window.removeEventListener('blur', onWindowBlur); | ||
| }, [isFilled, logAdInteraction]); |
There was a problem hiding this comment.
Non-blocking (verification gap) — the click proxy is likely desktop-only, and the tests cannot show that.
The two new specs drive iframe.focus() then fireEvent.blur(window) in jsdom, which asserts the handler's logic but not that the browser produces that sequence. Two real-world paths where it does not:
- Same-tab click-through. If the creative navigates the top window rather than opening
_blank, the document unloads without a windowblur. No event, and the most valuable click (the one that actually converted) is the one missed. - Touch. On iOS Safari in particular, tapping a cross-origin iframe does not reliably move
document.activeElementto the iframe, and the blur/unload ordering differs from desktop. Given the traffic mix, a metric that mostly works on desktop will read as a device-cohort CTR difference rather than an instrumentation artefact.
Since the whole point is the per-user join, could you verify on a real phone and on a same-tab creative before merge, and note the result in the description? If mobile does not produce the signal, that limitation belongs in the event (e.g. a signal: 'focus-blur' field in extra) so the analysis side cannot mistake it for real behaviour.
Reviewed by AI.
| return; | ||
| } | ||
| hasLoggedClick.current = true; | ||
| logAdInteraction(LogEvent.Click); |
There was a problem hiding this comment.
Non-blocking — unbounded delay between the focus and the blur.
The guard only checks that an iframe inside the wrapper holds focus at blur time, not when it took focus. A user who taps the creative without clicking through, keeps reading the article for five minutes, then alt-tabs to Slack produces a window blur with the iframe still document.activeElement — logged as an ad click. That is a broader overcount than the one the comment describes ("a tap that focuses without completing the click-through"), because the blur is unrelated to the ad entirely.
Cheapest tightening: record the timestamp when focus lands in the wrapper (focusin) and only log if the blur follows within a short window (a click-through blurs within a tick), or clear the armed state on window focus returning without navigation.
Reviewed by AI.
| const hasPushed = useRef(false); | ||
| const hasLoggedFill = useRef(false); | ||
| const hasLoggedEmpty = useRef(false); | ||
| const hasLoggedClick = useRef(false); |
There was a problem hiding this comment.
Non-blocking — refreshing slots are capped at one click per page-view.
hasLoggedClick is never reset, and neither is isFilled or the useViewability trackingKey (${surface}:${slot}:${unitId}:${format} — unchanged across a refresh). For a slot declared with refreshes, the second and subsequent creatives — different advertisers — can log neither a viewable impression nor a click. The comment's rationale ("a second click on the same creative is the same user leaving again") holds for a static unit but not for a rotating one.
That systematically depresses measured CTR precisely on the slots with the most inventory turnover. Worth either keying the guards off the creative rather than the slot, or explicitly documenting that refreshing slots are out of scope for this metric so the analysis does not read the gap as low engagement.
Reviewed by AI.
| format, | ||
| logEvent, | ||
| refreshes, | ||
| slot, |
There was a problem hiding this comment.
Non-blocking — logAdInteraction and logSlotEvent are the same function with three extra keys.
Both build the identical getAdsenseSlotLogExtra payload from the identical eight-value dependency list; the only difference is target_type/target_id/ad_provider_id. Two copies of the dependency array is exactly the kind of thing that drifts when someone later adds a field to one of them.
Suggest a single helper that takes the ad fields as an optional argument, or having logAdInteraction delegate to logSlotEvent.
Reviewed by AI.
…click proxy Review round on the analytics PR: - Taxonomy (blocking): the strict MRC measurement now logs as AdActions.Viewable, matching AdAsComment and PostSidebarAdWidget, and a loose AdActions.Impression is emitted at fill so both providers populate both events with the same meaning — a cross-provider CTR query no longer compares viewport impressions against MRC-viewable ones. Interactions use the AdActions enum throughout. - Click proxy: same-tab click-throughs are caught on pagehide (the document unloads without a window blur), a visitor returning with the creative still focused disarms it (no more alt-tab-minutes-later counting as a click), and every click event carries its inference signal (focus-blur | pagehide) so analysis can segment by mechanism and by device behavior. - The once-per-slot latches document that refreshing slots are out of scope until the Ad Manager migration — AdSense never rotates a creative, so the refreshes flag is only a forward-marker today. - logAdInteraction delegates to logSlotEvent; one dependency list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review addressed (c873216)Blocking — taxonomy: fixed exactly as suggested. The MRC measurement now logs as Click proxy hardening (non-blocking items):
Tests: 33 arbitrage-slot specs including the new pagehide click, disarm-on-return, impression-at-fill shape, and viewable-name reservation. 🤖 Generated with Claude Code |
Follow-up to #6500, answering "can we track AdSense at user level like our own ads".
What ships
AdActions.Impressionat fill (creative rendered near the viewport) andAdActions.Viewablefor the MRC-viewable measurement (sameuseViewability, armed only once a creative actually fills) — both shaped astarget_type: 'ad',target_id: <unit id>,ad_provider_id: 'adsense'— the exact shapeadLogEventgives internal ads, so one ClickHouse query covers every ad on the platform andGROUP BY ad_provider_idsplits internal vs AdSense demand. Slot, format, surface and viewability data ride inextra.AdActions.Clickin the same shape, with the inferencesignal(focus-blurfor new-tab click-throughs,pagehidefor same-tab) inextra; a visitor returning with the creative still focused disarms the proxy so unrelated later blurs can't count. Not yet verified on a real phone — until then, treat mobile CTR from this event as provisional and segment bysignal. Slightly approximate by nature (an iframe focus that doesn't complete the click-through counts); AdSense's own reporting stays the exact source of truth, this gives the per-user join it can't.view/click adsense slotnames are removed before any dashboard depends on them;request/fill/empty adsense slot,adsense slot errorandadsense test moderemain as delivery diagnostics (no internal-ads equivalent).Verification
User-level queries this unlocks: RPM per user cohort, clicked-an-ad user segments joined to retention/signup, per-slot CTR by geo — same joins the internal ad events already support.
🤖 Generated with Claude Code
Preview domain
https://claude-adsense-click-tracking.preview.app.daily.dev