Repository navigation
refactor: time requests from Flow request events - #453
Draft
totally-not-ai[bot] wants to merge 1 commit into
Draft
totally-not-ai[bot] wants to merge 1 commit into
totally-not-ai[bot] wants to merge 1 commit into
Conversation
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>
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
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.
Review risk: two-way door — request timing, request error outcome and
vaadin.errorscounting for every Vaadin request; one small guard in the navigation backstopSummary
RequestMetricsBindernow uses theRequestStartedEventandRequestEndedEventthat Flow 25.4 fires on the service event bus, instead of being aVaadinRequestInterceptorwith 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, boundederrortag 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:v-r=push, any transport) and the messages sent over a push connection are nowpush. Before, they werestatic, because their path is under/VAADIN/.index.htmlis nowbootstrap, also without the browser page-load hints (Sec-Fetch-Dest: document, anAcceptthat starts withtext/html). This covers uptime checks, crawlers andfetch()calls withSec-Fetch-Dest: empty, which wereotherbefore.WebComponentBootstrapHandler) is nowbootstrap. Before, it was classified by its URL.Behavior change: the
vaadin.request.typehook 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.UidlRequestHandlermaps touidl,HeartbeatHandlertoheartbeat,StreamRequestHandlertostream, andBootstrapHandlerand its subclasses tobootstrap. 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.PushRequestHandleris not referenced on purpose, because it needs Atmosphere, which an application without push may not have. Every request it handles hasv-r=push, which the URL classification now recognizes.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. TheTimer.Sample, error flag and error type thread locals are removed.getFailure(). A failure that the session error handler handled is still relayed throughRequestError, because the ended event does not report it. Flow passes a failed request's exception to the session error handler before the ended event, soErrorMetricsBindernow 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.RequestErrorno longer needs itsCOUNTEDslot; onlyHANDLEDis left.NavigationMetricsBinderbackstop 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 throughObservationScopes.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.Test summary
Request binder:
push, notstaticpusherrortag and onevaadin.errorscount when no error handler saw itError binder:
Navigation binder:
Service init listener:
Part of #416
🤖 Generated with Claude Code