Skip to content

refactor: time requests from Flow request events - #453

Draft
totally-not-ai[bot] wants to merge 1 commit into
mainfrom
refactor/request-events
Draft

totally-not-ai[bot] wants to merge 1 commit into
mainfrom
refactor/request-events

Conversation

@totally-not-ai

@totally-not-ai totally-not-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Review risk: two-way door — request timing, request error outcome and vaadin.errors counting for every Vaadin request; one small guard in the navigation backstop

Summary

RequestMetricsBinder now uses the RequestStartedEvent and RequestEndedEvent that Flow 25.4 fires on the service event bus, instead of being a VaadinRequestInterceptor with its own thread-local timer, error flag and error type. The duration, the failure and the request handler now come from the ended event. Metric names, tag keys, bounded error tag values and observation names and key values stay the same.

What changed

Behavior change: the request type now comes from the request handler when it tells. Some requests now get a different vaadin.request.type:

  • Push connection requests (v-r=push, any transport) and the messages sent over a push connection are now push. Before, they were static, because their path is under /VAADIN/.
  • A request that Flow answers with index.html is now bootstrap, also without the browser page-load hints (Sec-Fetch-Dest: document, an Accept that starts with text/html). This covers uptime checks, crawlers and fetch() calls with Sec-Fetch-Dest: empty, which were other before.
  • The web component bootstrap (WebComponentBootstrapHandler) is now bootstrap. Before, it was classified by its URL.

Behavior change: the vaadin.request.type hook into the Spring HTTP observation now runs at the end of the request, so it gets the final type. The Spring HTTP observation reads it when it stops, so the result is the same.

  • Type mapping: UidlRequestHandler maps to uidl, HeartbeatHandler to heartbeat, StreamRequestHandler to stream, and BootstrapHandler and its subclasses to bootstrap. Every other case uses the existing URL classification: no handler (for example an expired session), another handler, or the request observation's provisional type at start. PushRequestHandler is not referenced on purpose, because it needs Atmosphere, which an application without push may not have. Every request it handles has v-r=push, which the URL classification now recognizes.
  • A request that the started event reports without a response is a push message, the only such request. This is recorded at the start, because a request tracked for daily active users also has no response at the end.
  • The direct-recording path records the Timer with RequestEndedEvent.getDuration(). The Observation path still opens the observation and scope on the started event and closes them on the ended event. The thread locals for the observation and scope stay, and a new one keeps the type from the start. The Timer.Sample, error flag and error type thread locals are removed.
  • Errors: the failure comes from getFailure(). A failure that the session error handler handled is still relayed through RequestError, because the ended event does not report it. Flow passes a failed request's exception to the session error handler before the ended event, so ErrorMetricsBinder now always counts what it gets. The request binder only counts a failure that the decorated handler never saw, for example when there is no session. Each failure is still counted once, with the same tags. RequestError no longer needs its COUNTED slot; only HANDLED is left.
  • Navigation interplay: Flow fires the ended event before it calls the request interceptors. The request binder now closes its scope, and any navigation scope left open above it, before the NavigationMetricsBinder backstop runs. That backstop closed its scope a second time, which put the stopped request observation back as the current one on the pooled thread. The backstop now closes the scope through ObservationScopes.closeWithNested, which leaves a scope that is already unwound alone. This is the only change in the navigation binder. Moving it to events is a separate PR.
  • Updated the README: binder overview, request types table and page-load paragraph, error metrics wording.

Test summary

Request binder:

  • Records the Timer with the duration Flow measured
  • Subscribes to both request events and stops after the registration is removed
  • The handler decides the type: UIDL, heartbeat, stream, index.html and init
  • Another handler falls back to the URL, and a push connection is push, not static
  • A push message, with no response and no handler, is push
  • The span name and type set at start are updated from the handler at the end
  • A failure from the ended event gives an error outcome, the bounded error tag and one vaadin.errors count when no error handler saw it
  • A failure plus an earlier handled error marks the HTTP observation once
  • Existing scope, hook, session id, parity and contract tests now use the events

Error binder:

  • A failure the session error handler saw and the ended event reports is counted once

Navigation binder:

  • The backstop after the request scope was unwound leaves no scope behind, and both observations are stopped. This test fails without the guard.

Service init listener:

  • The request binder subscribes to both events when requests or errors are on, and not when both are off

Part of #416

🤖 Generated with Claude Code

RequestMetricsBinder now listens for RequestStartedEvent and
RequestEndedEvent on the service event bus instead of being a request
interceptor. The direct-recording Timer uses the duration Flow measured,
the failure comes from the ended event, and the request type comes from
the handler that handled the request, with the URL classification as
the fallback. Metric names, tags and observation key values are
unchanged.

Push connection requests and the messages sent over them are now typed
push instead of static. A request Flow answers with index.html is
bootstrap even without browser page-load hints.

The session error handler now counts an exception that failed the
request, and the request binder only counts one that handler never saw,
so RequestError no longer needs its counted slot.

Flow fires the ended event before the request interceptors run, so the
request scope is closed before the navigation backstop. The navigation
binder now leaves a scope that was already unwound alone instead of
closing it twice, which would put the stopped request observation back
as current.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants