From 00eef914d700f65c0adbc5033b2c9efab600fbae Mon Sep 17 00:00:00 2001 From: "totally-not-ai[bot]" <290682512+totally-not-ai[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 11:33:06 +0300 Subject: [PATCH] fix: count resyncs from Flow events so the starter no longer needs spring-web (#415) (CP: 5.0) ## Summary The `vaadin.resync` metric is now counted from events that Flow fires, not by a servlet filter that reads UIDL request bodies. The starter no longer needs spring-web or the servlet API, so it compiles again against Flow 25.4. Resyncs over push are now counted too, and a new `out_of_sync` type is added. ## What changed **Breaking:** Several public classes are removed: `ResyncDetectionFilter`, `ResyncDetector` and `ResyncInspector` (micrometer module), and `SpringResyncDetectionFilter` (spring module). The starter no longer registers the resync `FilterRegistrationBean`. This only affects apps that used these classes directly or overrode that bean. Everyone else gets the same metric without doing anything. **Behavior change:** `vaadin.resync` now also counts events on push requests. It has a new tag value `type=out_of_sync` for messages with an id the server did not expect (the user sees the session synchronization error). Dashboards or alerts that expect only `resend` and `resync` may see new data. - New package-private `ResyncMetricsBinder` listens for `ClientMessageResentEvent`, `UIResynchronizationEvent` and `MessageIdSyncErrorEvent` on the service event bus. `MetricsServiceInitListener` registers it when `vaadin.observability.resync` is on (the default). It registers the three counters up front, so each `type` reports 0 before its first event. - Removed the `spring-web` and `jakarta.servlet-api` dependencies from the starter. Since spring-web may now be missing at runtime, `SpringMetricsServiceInitListener` checks for it before enriching the Spring HTTP observation. Without it there is no HTTP observation to enrich, and using the enricher would throw `NoClassDefFoundError` on the first request. - Fixed `MetricsCollectorElement`: a UI resync detaches and re-attaches every node in place. The element's detach listener then added it to the UI again, which caused an endless loop, so the resync request never finished. Now it only re-adds itself when it is no longer attached. That also covers a collector inside a layout that gets removed from the UI. - Updated the README metric and setting descriptions and the architecture diagram. ## API Changes 5 classes affected: 4 removed, 1 constant added ### com.vaadin.observability.micrometer.MeterNames ```java // Added public static final String RESYNC_TYPE_OUT_OF_SYNC = "out_of_sync" // message id the server did not expect ``` ### com.vaadin.observability.micrometer.ResyncDetectionFilter ```java // Removed public final class ResyncDetectionFilter implements Filter public ResyncDetectionFilter(MeterRegistry registry) public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) ``` ### com.vaadin.observability.micrometer.ResyncDetector ```java // Removed public final class ResyncDetector public static final int NO_CLIENT_ID = -1 public enum Kind { NORMAL, RESEND, RESYNC } public record Result(Kind kind, int lastClientId) public ResyncDetector(MeterRegistry registry) public Result inspect(String requestBody, int previousClientId) ``` ### com.vaadin.observability.micrometer.ResyncInspector ```java // Removed public final class ResyncInspector public ResyncInspector(MeterRegistry registry) public static boolean isUidl(HttpServletRequest request) public static String uiId(HttpServletRequest request) public void inspect(String body, HttpSession session, String uiId, Object mutex) ``` ### com.vaadin.observability.spring.SpringResyncDetectionFilter ```java // Removed public final class SpringResyncDetectionFilter extends OncePerRequestFilter public SpringResyncDetectionFilter(MeterRegistry registry) protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) ``` ## Test summary **Resync metrics binder:** - Every type reports 0 before its first event - Counts resend, resync and out-of-sync events under their own type - Stops counting after the registration is removed - Registered only when the resync setting is on **Metrics collector element:** - Added back to its UI after being removed, directly or with an enclosing layout - Not added again after a UI resynchronization **Spring listener:** - Updates the Spring HTTP observation when spring-web is on the classpath - HTTP observation enrichment is skipped, without errors, when spring-web is not on the classpath **Starter (end-to-end):** - Resend, out-of-sync and resync exported to Prometheus as `vaadin_resync_total` - Not covered: counting over push requests --------- Co-authored-by: totally-not-ai[bot] <290682512+totally-not-ai[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 Co-authored-by: Artur Signell SpringResyncDetectionFilter.java --- README.md | 9 +- .../CachedBodyHttpServletRequest.java | 163 ---------------- .../observability/micrometer/MeterNames.java | 13 +- .../MetricsServiceInitListener.java | 4 + .../micrometer/ObservabilitySettings.java | 5 +- .../micrometer/ResyncDetectionFilter.java | 103 ---------- .../micrometer/ResyncDetector.java | 154 --------------- .../micrometer/ResyncInspector.java | 126 ------------- .../micrometer/ResyncMetricsBinder.java | 78 ++++++++ .../client/MetricsCollectorElement.java | 10 +- .../CachedBodyHttpServletRequestTest.java | 88 --------- .../MetricsServiceInitListenerTest.java | 26 +++ .../micrometer/ResyncDetectorTest.java | 109 ----------- .../micrometer/ResyncMetricsBinderTest.java | 83 ++++++++ .../client/MetricsCollectorElementTest.java | 85 +++++++++ .../SpringMetricsServiceInitListener.java | 27 ++- .../spring/SpringResyncDetectionFilter.java | 140 -------------- .../SpringMetricsServiceInitListenerTest.java | 143 ++++++++++++++ .../SpringResyncDetectionFilterTest.java | 108 ----------- observability-kit-starter/pom.xml | 9 - .../boot/ObservabilityAutoConfiguration.java | 22 --- .../tests/starter/ResyncMetricsIT.java | 178 ++++++++++-------- 22 files changed, 564 insertions(+), 1119 deletions(-) delete mode 100644 observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/CachedBodyHttpServletRequest.java delete mode 100644 observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncDetectionFilter.java delete mode 100644 observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncDetector.java delete mode 100644 observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncInspector.java create mode 100644 observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncMetricsBinder.java delete mode 100644 observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/CachedBodyHttpServletRequestTest.java delete mode 100644 observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ResyncDetectorTest.java create mode 100644 observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ResyncMetricsBinderTest.java delete mode 100644 observability-kit-spring/src/main/java/com/vaadin/observability/spring/SpringResyncDetectionFilter.java create mode 100644 observability-kit-spring/src/test/java/com/vaadin/observability/spring/SpringMetricsServiceInitListenerTest.java delete mode 100644 observability-kit-spring/src/test/java/com/vaadin/observability/spring/SpringResyncDetectionFilterTest.java diff --git a/README.md b/README.md index e61701a1..81110517 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,6 @@ flowchart LR subgraph Server["Vaadin application"] httpobs["Spring HTTP observation
http.server.requests"] - resync["Resync detection filter
vaadin.resync"] flow["VaadinService + Flow SPIs
requests, RPC, navigation,
data queries, sessions, UIs, errors"] subgraph Kit["Observability Kit binders"] @@ -46,6 +45,7 @@ flowchart LR datab["DataQueryMetricsBinder
vaadin.data.fetch / count"] errb["ErrorMetricsBinder
vaadin.errors"] lifeb["Session / UI / Lock / State binders
vaadin.sessions.*, vaadin.ui.*"] + resyncb["ResyncMetricsBinder
vaadin.resync"] clientb["ClientMetricsBinder
vaadin.client.*"] dbproxy["DataSource proxy, Boot opt-in
vaadin.db.query / fetch.rows"] end @@ -68,10 +68,9 @@ flowchart LR collector -- "rate-limited batches
over a ClientCallable" --> clientb flow -- "listener and event-bus callbacks" --> Kit - resync --> meters reqb -- "enrich and mark" --> hooks --> httpobs reqb & rpcb & navb & datab --> obsreg - errb & lifeb & clientb & dbproxy --> meters + errb & lifeb & resyncb & clientb & dbproxy --> meters errb & reqb -- "failed / slow interactions" --> insights texec --> obsreg obsreg --> handler --> meters @@ -394,7 +393,7 @@ vaadin.observability.traces=false | `vaadin.observability.data` | `true` | Data provider count/fetch query timing and page sizes for lazy-loading components. | | `vaadin.observability.errors` | `true` | Error counters. | | `vaadin.observability.client` | `true` | Browser-side timing, connection state and errors collected from the client (see [Connection and client-side problems](#connection-and-client-side-problems)). | -| `vaadin.observability.resync` | `true` | Observe UIDL message resends and client-requested resynchronizations. | +| `vaadin.observability.resync` | `true` | Count client message resends, client-requested resynchronizations and unexpected message ids. | | `vaadin.observability.database` | `false` | Wrap `DataSource` beans to record JDBC result-set sizes per route, report the SQL queries behind each [slow data query insight](#what-a-slow-data-query-cost-in-the-database), and (when tracing is on) emit a span per query (Spring Boot starter only). | | `vaadin.observability.database-statement` | `false` | Attach the (parameterized) SQL as `db.statement` on the query span. Off by default since SQL is higher cardinality and may be sensitive. | | `vaadin.observability.database-span-limit` | `100` | Most `vaadin.db.query` spans under any one parent span (request, RPC, data fetch). Queries past it are still timed and counted but get no span; the parent carries how many as `vaadin.db.queries.unspanned`. `0` spans no query that has a parent span. See [N+1 loads and the span limit](#n1-loads-and-the-span-limit). | @@ -451,7 +450,7 @@ ObservabilitySettings.builder() | `vaadin.data.fetch.requested` | DistributionSummary | Items a fetch query asked for, tagged by `route`. | | `vaadin.data.fetch.rows` | DistributionSummary | Items a fetch query actually returned, tagged by `route`. Compared against `vaadin.data.fetch.requested` it shows a component asking for far more than it renders, or a data provider returning short pages. | | `vaadin.errors` | Counter | Server-side errors (tagged by `exception`, `route`, `component`). See [Error metrics](#error-metrics). | -| `vaadin.resync` | Counter | UIDL message recovery events observed on incoming requests, tagged by `type`: `resend` for a duplicate message the client re-sent because it never got the previous response, `resync` for a full client-requested UI-state rebuild. Both mean the client lost a server message. Disable with `vaadin.observability.resync=false`. | +| `vaadin.resync` | Counter | Client message recovery events, for both HTTP and push requests, tagged by `type`: `resend` for a duplicate message the client re-sent because it never got the previous response, `resync` for a full client-requested UI-state rebuild, `out_of_sync` for a message with an id the server did not expect, which shows the user the session synchronization error. `resend` and `resync` mean the client lost a server message; `out_of_sync` usually means the UI continued on a server with an older state. Disable with `vaadin.observability.resync=false`. | | `vaadin.client.bootstrap.duration` | Timer | Browser application bootstrap time. | | `vaadin.client.navigation.duration` | Timer | Browser-observed navigation time, tagged by `route` and by the `trigger` that started it (`back` or `programmatic`). | | `vaadin.client.request.duration` | Timer | One UIDL request as the browser saw it, from queueing to the last byte of the response, tagged by `route`. The browser's side of `vaadin.request.duration`. See [Interaction timing from the browser](#interaction-timing-from-the-browser). | diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/CachedBodyHttpServletRequest.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/CachedBodyHttpServletRequest.java deleted file mode 100644 index 5f8b47c0..00000000 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/CachedBodyHttpServletRequest.java +++ /dev/null @@ -1,163 +0,0 @@ -/** - * Copyright (C) 2000-2026 Vaadin Ltd - * - * This program is available under Vaadin Commercial License and Service Terms. - * - * See for the full - * license. - */ -package com.vaadin.observability.micrometer; - -import jakarta.servlet.ReadListener; -import jakarta.servlet.ServletInputStream; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletRequestWrapper; - -import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStreamReader; -import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; - -/** - * Wraps a request and buffers its body so it can be read more than once: the - * kit inspects it for resend/resync detection and Flow still reads the same - * bytes downstream. Used by {@link ResyncDetectionFilter}. - *

- * At most {@code limit} bytes are buffered. A longer body is not inspected - * ({@link #isTruncated()}): the buffered prefix is replayed and the rest is - * streamed from the original request, so Flow still reads it unchanged while - * the kit never holds more than {@code limit} bytes of it. - */ -final class CachedBodyHttpServletRequest extends HttpServletRequestWrapper { - - private final byte[] body; - private final boolean truncated; - private ServletInputStream overflowStream; - - CachedBodyHttpServletRequest(HttpServletRequest request, int limit) - throws IOException { - super(request); - // readNBytes grows its buffer as bytes arrive, so a forged - // Content-Length cannot make it allocate up front. - byte[] read = request.getInputStream().readNBytes(limit + 1); - this.truncated = read.length > limit; - this.body = read; - } - - /** - * Whether the body exceeded the buffering limit; the cached body is then - * only a prefix and must not be inspected. - * - * @return {@code true} if the body was longer than the limit - */ - boolean isTruncated() { - return truncated; - } - - String getCachedBody() { - return new String(body, charset()); - } - - private Charset charset() { - String enc = getCharacterEncoding(); - if (enc != null) { - try { - return Charset.forName(enc); - } catch (RuntimeException unsupported) { - // fall through to default - } - } - return StandardCharsets.UTF_8; - } - - @Override - public ServletInputStream getInputStream() throws IOException { - if (truncated) { - // The rest of the body can only be read once from the original - // stream, so hand out one stream that continues where the - // buffered prefix ends. - if (overflowStream == null) { - overflowStream = new PrefixedInputStream(body, - super.getInputStream()); - } - return overflowStream; - } - ByteArrayInputStream buffer = new ByteArrayInputStream(body); - return new ServletInputStream() { - @Override - public int read() { - return buffer.read(); - } - - @Override - public boolean isFinished() { - return buffer.available() == 0; - } - - @Override - public boolean isReady() { - return true; - } - - @Override - public void setReadListener(ReadListener readListener) { - // synchronous replay only; not used for async reads - } - }; - } - - @Override - public BufferedReader getReader() throws IOException { - return new BufferedReader( - new InputStreamReader(getInputStream(), charset())); - } - - /** - * Replays an already-read prefix, then continues with the remainder of the - * original request stream. - */ - private static final class PrefixedInputStream extends ServletInputStream { - - private final ByteArrayInputStream prefix; - private final ServletInputStream rest; - - PrefixedInputStream(byte[] prefix, ServletInputStream rest) { - this.prefix = new ByteArrayInputStream(prefix); - this.rest = rest; - } - - @Override - public int read() throws IOException { - int b = prefix.read(); - return b != -1 ? b : rest.read(); - } - - @Override - public int read(byte[] b, int off, int len) throws IOException { - if (len == 0) { - return 0; - } - if (prefix.available() > 0) { - return prefix.read(b, off, len); - } - return rest.read(b, off, len); - } - - @Override - public boolean isFinished() { - return prefix.available() == 0 && rest.isFinished(); - } - - @Override - public boolean isReady() { - return prefix.available() > 0 || rest.isReady(); - } - - @Override - public void setReadListener(ReadListener readListener) { - // synchronous replay only; not used for async reads - } - } -} diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/MeterNames.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/MeterNames.java index ff5994f1..1f07e8d1 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/MeterNames.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/MeterNames.java @@ -392,9 +392,9 @@ public final class MeterNames { public static final String TAG_TYPE = "type"; /** - * Counter: UIDL message recovery events observed on incoming requests. - * Tagged by {@link #TAG_TYPE} with {@link #RESYNC_TYPE_RESEND} or - * {@link #RESYNC_TYPE_RESYNC}. + * Counter: client message recovery events. Tagged by {@link #TAG_TYPE} with + * {@link #RESYNC_TYPE_RESEND}, {@link #RESYNC_TYPE_RESYNC} or + * {@link #RESYNC_TYPE_OUT_OF_SYNC}. */ public static final String RESYNC = "vaadin.resync"; @@ -412,6 +412,13 @@ public final class MeterNames { */ public static final String RESYNC_TYPE_RESYNC = "resync"; + /** + * {@link #TAG_TYPE} value for a message with an id the server did not + * expect; the message is not processed and the user is shown the session + * synchronization error. + */ + public static final String RESYNC_TYPE_OUT_OF_SYNC = "out_of_sync"; + private MeterNames() { } } diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/MetricsServiceInitListener.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/MetricsServiceInitListener.java index 48433e17..df01014b 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/MetricsServiceInitListener.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/MetricsServiceInitListener.java @@ -255,6 +255,10 @@ void bind(ServiceInitEvent event, MeterRegistry registry, .register(service.getEventBus()); } + if (settings.isResync()) { + new ResyncMetricsBinder(registry).register(service.getEventBus()); + } + // Browser errors are counted by the client collector and described // here: the message, the script and the first stack frame are what a // counter cannot hold, and they are the difference between knowing diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilitySettings.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilitySettings.java index 26986242..902bc387 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilitySettings.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilitySettings.java @@ -135,7 +135,10 @@ public boolean isClient() { return client; } - /** Whether to observe UIDL message resends and resynchronizations. */ + /** + * Whether to count client message resends, resynchronizations and + * unexpected message ids. + */ public boolean isResync() { return resync; } diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncDetectionFilter.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncDetectionFilter.java deleted file mode 100644 index ab1c9ffa..00000000 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncDetectionFilter.java +++ /dev/null @@ -1,103 +0,0 @@ -/** - * Copyright (C) 2000-2026 Vaadin Ltd - * - * This program is available under Vaadin Commercial License and Service Terms. - * - * See for the full - * license. - */ -package com.vaadin.observability.micrometer; - -import jakarta.servlet.Filter; -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.ServletRequest; -import jakarta.servlet.ServletResponse; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpSession; - -import java.io.IOException; - -import io.micrometer.core.instrument.MeterRegistry; - -import com.vaadin.flow.server.communication.UidlRequestHandler; - -/** - * Portable, framework-agnostic servlet filter that observes UIDL message - * resends and resynchronization requests (prototype, kit-only, no Flow - * changes). It depends only on the servlet API and Micrometer, so it is meant - * for plain (non-Spring) deployments; a Spring application does not need this - * class and should use {@code SpringResyncDetectionFilter} instead. - *

- * Registering it in a plain servlet app. There is no auto-configuration - * outside Spring, so register the filter yourself against every request. It - * should run early (before anything else consumes the request body). For - * example, in a {@code ServletContextListener} or - * {@code ServletContainerInitializer}: - * - * 

{@code
- * FilterRegistration.Dynamic reg = servletContext.addFilter(
- *         "vaadinResyncDetection", new ResyncDetectionFilter(meterRegistry));
- * // isMatchAfter=false so it precedes already-registered filters
- * reg.addMappingForUrlPatterns(null, false, "/*");
- * }
- * - * or the equivalent {@code }/{@code } in - * {@code web.xml}. The {@code MeterRegistry} is whichever instance the - * application records into. - *

- * Flow recovers from lost responses entirely inside {@link UidlRequestHandler} - * by catching {@code ClientResentPayloadException} (replay the cached response) - * and {@code ResynchronizationRequiredException} (rebuild the UI state); - * neither surfaces to any Flow listener SPI the kit uses. This filter - * reconstructs the same signal by buffering the UIDL body (via - * {@link CachedBodyHttpServletRequest} so Flow can still read it) and handing - * it to a {@link ResyncInspector}. - *

- * Because it has no Spring APIs available it buffers the body eagerly, up to - * {@link ResyncInspector#MAX_INSPECTED_BODY_BYTES} (a longer body is passed on - * uninspected); the Spring variant instead captures the bytes lazily as Flow - * reads them and only counts requests Flow actually consumed. Instrumentation - * never fails the request: any error while inspecting is swallowed. - */ -public final class ResyncDetectionFilter implements Filter { - - private final ResyncInspector inspector; - - /** - * Creates the filter recording into the given registry. - * - * @param registry - * the meter registry, not {@code null} - */ - public ResyncDetectionFilter(MeterRegistry registry) { - this.inspector = new ResyncInspector(registry); - } - - @Override - public void doFilter(ServletRequest request, ServletResponse response, - FilterChain chain) throws IOException, ServletException { - if (!(request instanceof HttpServletRequest http) - || !ResyncInspector.isUidl(http)) { - chain.doFilter(request, response); - return; - } - - CachedBodyHttpServletRequest wrapped = new CachedBodyHttpServletRequest( - http, ResyncInspector.MAX_INSPECTED_BODY_BYTES); - if (wrapped.isTruncated()) { - // Too large to buffer for inspection; let Flow handle it as usual. - chain.doFilter(wrapped, response); - return; - } - try { - HttpSession session = wrapped.getSession(false); - Object mutex = session != null ? session : this; - inspector.inspect(wrapped.getCachedBody(), session, - ResyncInspector.uiId(wrapped), mutex); - } catch (RuntimeException instrumentationFailure) { - // Never break a request because of observability. - } - chain.doFilter(wrapped, response); - } -} diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncDetector.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncDetector.java deleted file mode 100644 index dafa22db..00000000 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncDetector.java +++ /dev/null @@ -1,154 +0,0 @@ -/** - * Copyright (C) 2000-2026 Vaadin Ltd - * - * This program is available under Vaadin Commercial License and Service Terms. - * - * See for the full - * license. - */ -package com.vaadin.observability.micrometer; - -import io.micrometer.core.instrument.MeterRegistry; -import tools.jackson.databind.JsonNode; - -import com.vaadin.flow.internal.JacksonUtils; -import com.vaadin.flow.shared.ApplicationConstants; - -/** - * Prototype detector that classifies an incoming UIDL request body as a normal - * message, a re-sent (duplicate) message, or a full resynchronization request, - * and records a {@link MeterNames#RESYNC} counter for the two recovery cases. - *

- * The classification mirrors Flow's own server-side logic in - * {@code ServerRpcHandler#handleRpc}, which is otherwise invisible to the kit - * because the relevant {@code ClientResentPayloadException} and - * {@code ResynchronizationRequiredException} are caught internally by - * {@code UidlRequestHandler} and never surface to a {@code - * VaadinRequestInterceptor}: - *

    - *
  • Resync — the request carries - * {@link ApplicationConstants#RESYNCHRONIZE_ID}{@code = true}. This is checked - * first because a resync request also carries a normally advancing - * {@code clientId}.
    • - *
  • Resend — the request's - * {@link ApplicationConstants#CLIENT_TO_SERVER_ID} is not greater than the last - * one seen for the same UI, i.e. the client re-sent a message the server had - * already processed (Flow compares against - * {@code lastProcessedClientToServerId + 1}).
    • - *
- *

- * This is an approximation of Flow's own test. Flow only replays a cached - * response when {@code clientId == lastProcessedClientToServerId} and - * the message hash matches, discarding still-older ids as out-of-order. The - * message hash is not exposed to the kit, so we cannot discriminate on it and - * count every {@code clientId <= previous} as a resend; this may over-report - * relative to what Flow actually replays. - *

- * The detector is stateless: the caller supplies the previously seen - * {@code clientId} and stores the {@link Result#lastClientId()} returned here - * (e.g. as an HTTP session attribute keyed by UI id), so there is no unbounded - * per-session state held inside the kit. - */ -public final class ResyncDetector { - - /** No {@code clientId} has been seen yet for a UI. */ - public static final int NO_CLIENT_ID = -1; - - /** Classification of a single UIDL request. */ - public enum Kind { - /** An ordinary, in-order message. */ - NORMAL, - /** A duplicate the client re-sent; the server replays its response. */ - RESEND, - /** A client-requested full UI-state resynchronization. */ - RESYNC - } - - /** - * Result of classifying a request. - * - * @param kind - * the classification - * @param lastClientId - * the {@code clientId} the caller should remember for this UI - * for the next request (unchanged from the input for a resend) - */ - public record Result(Kind kind, int lastClientId) { - } - - private final MeterRegistry registry; - - /** - * Creates a detector recording into the given registry. - * - * @param registry - * the meter registry, not {@code null} - */ - public ResyncDetector(MeterRegistry registry) { - this.registry = registry; - } - - /** - * Classifies a UIDL request body and records a counter for resend/resync - * events. - * - * @param requestBody - * the raw UIDL request body (JSON); may be {@code null} or empty - * @param previousClientId - * the last {@code clientId} seen for this UI, or - * {@link #NO_CLIENT_ID} if none - * @return the classification and the {@code clientId} to remember next - */ - public Result inspect(String requestBody, int previousClientId) { - Result result = classify(requestBody, previousClientId); - switch (result.kind()) { - case RESEND -> registry.counter(MeterNames.RESYNC, MeterNames.TAG_TYPE, - MeterNames.RESYNC_TYPE_RESEND).increment(); - case RESYNC -> registry.counter(MeterNames.RESYNC, MeterNames.TAG_TYPE, - MeterNames.RESYNC_TYPE_RESYNC).increment(); - default -> { - // NORMAL: nothing to record - } - } - return result; - } - - private static Result classify(String requestBody, int previousClientId) { - if (requestBody == null || requestBody.isBlank()) { - return new Result(Kind.NORMAL, previousClientId); - } - JsonNode json; - try { - json = JacksonUtils.readTree(requestBody); - } catch (RuntimeException malformed) { - // Not our concern to validate the payload; let Flow handle it. - return new Result(Kind.NORMAL, previousClientId); - } - if (json == null) { - return new Result(Kind.NORMAL, previousClientId); - } - - int clientId = json.has(ApplicationConstants.CLIENT_TO_SERVER_ID) - ? json.get(ApplicationConstants.CLIENT_TO_SERVER_ID).intValue() - : NO_CLIENT_ID; - - // Resync takes precedence: such a request also carries a normally - // advancing clientId, so it must be classified before the comparison. - if (json.has(ApplicationConstants.RESYNCHRONIZE_ID) && json - .get(ApplicationConstants.RESYNCHRONIZE_ID).booleanValue()) { - int next = clientId >= 0 ? Math.max(previousClientId, clientId) - : previousClientId; - return new Result(Kind.RESYNC, next); - } - - if (clientId < 0) { - return new Result(Kind.NORMAL, previousClientId); - } - if (previousClientId != NO_CLIENT_ID && clientId <= previousClientId) { - // The client re-sent a message id the server already advanced past; - // do not advance the remembered id. - return new Result(Kind.RESEND, previousClientId); - } - return new Result(Kind.NORMAL, clientId); - } -} diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncInspector.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncInspector.java deleted file mode 100644 index 05dfb30f..00000000 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncInspector.java +++ /dev/null @@ -1,126 +0,0 @@ -/** - * Copyright (C) 2000-2026 Vaadin Ltd - * - * This program is available under Vaadin Commercial License and Service Terms. - * - * See for the full - * license. - */ -package com.vaadin.observability.micrometer; - -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpSession; - -import io.micrometer.core.instrument.MeterRegistry; - -import com.vaadin.flow.shared.ApplicationConstants; - -/** - * Servlet-level glue shared by the resync/resend detection filters: it - * recognizes UIDL requests, derives the per-UI key, and drives a - * {@link ResyncDetector} against per-UI state kept in the HTTP session. - *

- * This class is deliberately framework-agnostic (servlet API only) so it can - * back both the portable {@link ResyncDetectionFilter} and Spring-specific - * filters. It holds no state of its own; the {@code clientId} history lives in - * the session, keyed by UI id, so it is bounded by and cleaned up with the - * session. - */ -public final class ResyncInspector { - - /** - * The largest UIDL body, in bytes, the detection filters buffer for - * inspection. A longer body is passed on to Flow untouched but not - * classified: buffering it without a bound would let any client (the - * filters run before Flow and security, and a session is optional) make the - * server hold arbitrarily large copies of a request, and inspecting only a - * prefix could miss the {@code clientId}/{@code resynchronize} fields, - * which the UIDL JSON places after the potentially large {@code rpc} array. - * Regular UIDL messages are a few kilobytes, so this only skips the rare - * huge one. - */ - public static final int MAX_INSPECTED_BODY_BYTES = 1024 * 1024; - - private static final String LAST_CLIENT_ID_ATTR_PREFIX = ResyncInspector.class - .getName() + ".lastClientId."; - - private final ResyncDetector detector; - - /** - * Creates an inspector recording into the given registry. - * - * @param registry - * the meter registry, not {@code null} - */ - public ResyncInspector(MeterRegistry registry) { - this.detector = new ResyncDetector(registry); - } - - /** - * A UIDL request is a POST whose query string carries {@code v-r=uidl}. - * Checking the query string (rather than {@code getParameter}) avoids - * triggering body parsing on the original request. - * - * @param request - * the request to test - * @return {@code true} if this is a UIDL POST - */ - public static boolean isUidl(HttpServletRequest request) { - if (!"POST".equalsIgnoreCase(request.getMethod())) { - return false; - } - String query = request.getQueryString(); - return query != null - && query.contains(ApplicationConstants.REQUEST_TYPE_PARAMETER - + "=" + ApplicationConstants.REQUEST_TYPE_UIDL); - } - - /** - * Returns the UI id carried by the request, or {@code "-"} when absent. - * - * @param request - * the request - * @return the UI id, never {@code null} - */ - public static String uiId(HttpServletRequest request) { - String id = request.getParameter(ApplicationConstants.UI_ID_PARAMETER); - return id != null ? id : "-"; - } - - /** - * Classifies a UIDL body against the {@code clientId} last seen for the - * same UI, records a counter for resend/resync, and stores the new - * {@code clientId} back on the session. - *

- * The read-modify-write on the session attribute is guarded by - * {@code mutex}: a resend can overlap the original request for the same UI - * (both reach the filter before Flow's per-session lock), and without the - * guard the two could clobber each other's stored {@code clientId}. - * - * @param body - * the UIDL request body (JSON); may be {@code null}/empty - * @param session - * the HTTP session holding per-UI state, or {@code null} - * @param uiId - * the UI id used to key the session attribute - * @param mutex - * the monitor to guard the read-modify-write on - */ - public void inspect(String body, HttpSession session, String uiId, - Object mutex) { - String attr = LAST_CLIENT_ID_ATTR_PREFIX + uiId; - synchronized (mutex) { - int previous = ResyncDetector.NO_CLIENT_ID; - if (session != null - && session.getAttribute(attr) instanceof Integer stored) { - previous = stored; - } - - ResyncDetector.Result result = detector.inspect(body, previous); - - if (session != null) { - session.setAttribute(attr, result.lastClientId()); - } - } - } -} diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncMetricsBinder.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncMetricsBinder.java new file mode 100644 index 00000000..563bd432 --- /dev/null +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ResyncMetricsBinder.java @@ -0,0 +1,78 @@ +/** + * Copyright (C) 2000-2026 Vaadin Ltd + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See for the full + * license. + */ +package com.vaadin.observability.micrometer; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.MeterRegistry; + +import com.vaadin.flow.server.VaadinServiceEventBus; +import com.vaadin.flow.server.communication.ClientMessageResentEvent; +import com.vaadin.flow.server.communication.MessageIdSyncErrorEvent; +import com.vaadin.flow.server.communication.UIResynchronizationEvent; +import com.vaadin.flow.shared.Registration; + +/** + * Counts client message recovery from the events on the + * {@link com.vaadin.flow.server.VaadinService#getEventBus() service event bus} + * into {@link MeterNames#RESYNC}. + *

+ * Flow fires the events while handling the client's message ids, for both HTTP + * and push requests: a re-sent message it answers with the previous response + * ({@link MeterNames#RESYNC_TYPE_RESEND}), a resynchronization the client asked + * for ({@link MeterNames#RESYNC_TYPE_RESYNC}), and an unexpected message id + * that shows the user the session synchronization error + * ({@link MeterNames#RESYNC_TYPE_OUT_OF_SYNC}). The counters are registered up + * front, so each type reports 0 before its first event instead of no data. + */ +final class ResyncMetricsBinder { + + private final Counter resent; + private final Counter resynchronized; + private final Counter outOfSync; + + ResyncMetricsBinder(MeterRegistry registry) { + this.resent = counter(registry, MeterNames.RESYNC_TYPE_RESEND); + this.resynchronized = counter(registry, MeterNames.RESYNC_TYPE_RESYNC); + this.outOfSync = counter(registry, MeterNames.RESYNC_TYPE_OUT_OF_SYNC); + } + + /** + * Subscribes to the message recovery events on the given bus. + * + * @param eventBus + * the service event bus to listen on + * @return a handle removing every subscription made here + */ + Registration register(VaadinServiceEventBus eventBus) { + return Registration.combine( + eventBus.addListener(ClientMessageResentEvent.class, + this::messageResent), + eventBus.addListener(UIResynchronizationEvent.class, + this::uiResynchronized), + eventBus.addListener(MessageIdSyncErrorEvent.class, + this::messageIdSyncError)); + } + + void messageResent(ClientMessageResentEvent event) { + resent.increment(); + } + + void uiResynchronized(UIResynchronizationEvent event) { + resynchronized.increment(); + } + + void messageIdSyncError(MessageIdSyncErrorEvent event) { + outOfSync.increment(); + } + + private static Counter counter(MeterRegistry registry, String type) { + return Counter.builder(MeterNames.RESYNC).tag(MeterNames.TAG_TYPE, type) + .register(registry); + } +} diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/client/MetricsCollectorElement.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/client/MetricsCollectorElement.java index 3ab0565b..dfaf38e0 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/client/MetricsCollectorElement.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/client/MetricsCollectorElement.java @@ -76,7 +76,15 @@ public MetricsCollectorElement(ClientMetricsBinder binder, addDetachListener(event -> { UI ui = event.getUI(); if (ui != null && !ui.isClosing()) { - ui.access(() -> ui.add(this)); + // A resynchronization detaches and re-attaches in place: + // adding the element again would move it, detach it once more + // and loop forever. Checking attachment rather than the parent + // also re-adds it when a layout around it was removed. + ui.access(() -> { + if (!isAttached()) { + ui.add(this); + } + }); } }); } diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/CachedBodyHttpServletRequestTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/CachedBodyHttpServletRequestTest.java deleted file mode 100644 index a289e48e..00000000 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/CachedBodyHttpServletRequestTest.java +++ /dev/null @@ -1,88 +0,0 @@ -/** - * Copyright (C) 2000-2026 Vaadin Ltd - * - * This program is available under Vaadin Commercial License and Service Terms. - * - * See for the full - * license. - */ -package com.vaadin.observability.micrometer; - -import jakarta.servlet.ReadListener; -import jakarta.servlet.ServletInputStream; -import jakarta.servlet.http.HttpServletRequest; - -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.nio.charset.StandardCharsets; - -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.mockito.Mockito; - -class CachedBodyHttpServletRequestTest { - - @Test - void bodyWithinLimitIsCachedAndReplayable() throws IOException { - CachedBodyHttpServletRequest wrapped = new CachedBodyHttpServletRequest( - requestWithBody("{\"clientId\":3}"), 64); - - Assertions.assertFalse(wrapped.isTruncated()); - Assertions.assertEquals("{\"clientId\":3}", wrapped.getCachedBody()); - Assertions.assertEquals("{\"clientId\":3}", - new String(wrapped.getInputStream().readAllBytes(), - StandardCharsets.UTF_8)); - } - - @Test - void bodyExactlyAtLimitIsNotTruncated() throws IOException { - CachedBodyHttpServletRequest wrapped = new CachedBodyHttpServletRequest( - requestWithBody("12345678"), 8); - - Assertions.assertFalse(wrapped.isTruncated()); - Assertions.assertEquals("12345678", wrapped.getCachedBody()); - } - - @Test - void bodyOverLimitIsTruncatedButStillReadInFull() throws IOException { - String body = "x".repeat(100) + "{\"clientId\":3}"; - CachedBodyHttpServletRequest wrapped = new CachedBodyHttpServletRequest( - requestWithBody(body), 16); - - Assertions.assertTrue(wrapped.isTruncated()); - Assertions.assertEquals(body, - new String(wrapped.getInputStream().readAllBytes(), - StandardCharsets.UTF_8)); - Assertions.assertTrue(wrapped.getInputStream().isFinished()); - } - - private static HttpServletRequest requestWithBody(String body) - throws IOException { - ByteArrayInputStream in = new ByteArrayInputStream( - body.getBytes(StandardCharsets.UTF_8)); - ServletInputStream stream = new ServletInputStream() { - @Override - public int read() { - return in.read(); - } - - @Override - public boolean isFinished() { - return in.available() == 0; - } - - @Override - public boolean isReady() { - return true; - } - - @Override - public void setReadListener(ReadListener readListener) { - // not used - } - }; - HttpServletRequest request = Mockito.mock(HttpServletRequest.class); - Mockito.when(request.getInputStream()).thenReturn(stream); - return request; - } -} diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MetricsServiceInitListenerTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MetricsServiceInitListenerTest.java index 6533bd70..b64c9085 100644 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MetricsServiceInitListenerTest.java +++ b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MetricsServiceInitListenerTest.java @@ -15,6 +15,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.ArgumentCaptor; import com.vaadin.flow.component.UI; @@ -31,9 +33,12 @@ import com.vaadin.flow.server.VaadinServiceEventBus; import com.vaadin.flow.server.VaadinSession; import com.vaadin.flow.server.communication.AbstractRpcInvocationEvent; +import com.vaadin.flow.server.communication.ClientMessageResentEvent; +import com.vaadin.flow.server.communication.MessageIdSyncErrorEvent; import com.vaadin.flow.server.communication.RpcInvocationEndedEvent; import com.vaadin.flow.server.communication.RpcInvocationFailedEvent; import com.vaadin.flow.server.communication.RpcInvocationStartedEvent; +import com.vaadin.flow.server.communication.UIResynchronizationEvent; import com.vaadin.observability.micrometer.insights.CapturedInteraction; import com.vaadin.observability.micrometer.insights.RecentInteractions; @@ -200,6 +205,27 @@ void skipsSessionBinderWhenSessionsDisabled() { "sessions disabled should not subscribe the lock binder"); } + @ParameterizedTest + @ValueSource(booleans = { true, false }) + void subscribesResyncBinderOnlyWhenResyncEnabled(boolean resync) { + ObservabilityKit.install(new SimpleMeterRegistry(), + ObservabilitySettings.builder().resync(resync).build()); + VaadinService service = licensedService(); + ServiceInitEvent event = mock(ServiceInitEvent.class); + when(event.getSource()).thenReturn(service); + + new MetricsServiceInitListener().serviceInit(event); + + VaadinServiceEventBus bus = service.getEventBus(); + int expected = resync ? 1 : 0; + assertEquals(expected, + bus.getListeners(ClientMessageResentEvent.class).size()); + assertEquals(expected, + bus.getListeners(UIResynchronizationEvent.class).size()); + assertEquals(expected, + bus.getListeners(MessageIdSyncErrorEvent.class).size()); + } + @Test void registersErrorBinderWhenErrorsEnabled() { // The session error handler is where Flow routes the failures a user diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ResyncDetectorTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ResyncDetectorTest.java deleted file mode 100644 index 21f540cf..00000000 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ResyncDetectorTest.java +++ /dev/null @@ -1,109 +0,0 @@ -/** - * Copyright (C) 2000-2026 Vaadin Ltd - * - * This program is available under Vaadin Commercial License and Service Terms. - * - * See for the full - * license. - */ -package com.vaadin.observability.micrometer; - -import io.micrometer.core.instrument.search.MeterNotFoundException; -import io.micrometer.core.instrument.simple.SimpleMeterRegistry; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import com.vaadin.observability.micrometer.ResyncDetector.Kind; -import com.vaadin.observability.micrometer.ResyncDetector.Result; - -class ResyncDetectorTest { - - private SimpleMeterRegistry registry; - private ResyncDetector detector; - - @BeforeEach - void setUp() { - registry = new SimpleMeterRegistry(); - detector = new ResyncDetector(registry); - } - - private static String message(int clientId, int syncId) { - return "{\"csrfToken\":\"x\",\"rpc\":[],\"syncId\":" + syncId - + ",\"clientId\":" + clientId + "}"; - } - - private static String resyncMessage(int clientId, int syncId) { - return "{\"csrfToken\":\"x\",\"rpc\":[],\"syncId\":" + syncId - + ",\"clientId\":" + clientId + ",\"resynchronize\":true}"; - } - - private double count(String type) { - try { - return registry.get(MeterNames.RESYNC) - .tag(MeterNames.TAG_TYPE, type).counter().count(); - } catch (MeterNotFoundException notRecorded) { - return 0d; - } - } - - @Test - void inOrderMessages_areNormal_andAdvanceClientId() { - int last = ResyncDetector.NO_CLIENT_ID; - for (int id = 0; id < 5; id++) { - Result r = detector.inspect(message(id, id - 1), last); - Assertions.assertEquals(Kind.NORMAL, r.kind()); - Assertions.assertEquals(id, r.lastClientId()); - last = r.lastClientId(); - } - Assertions.assertEquals(0d, count(MeterNames.RESYNC_TYPE_RESEND)); - Assertions.assertEquals(0d, count(MeterNames.RESYNC_TYPE_RESYNC)); - } - - @Test - void resentDuplicate_isDetected_andDoesNotAdvanceClientId() { - // client sent and the server processed message 0 and 1 - Result first = detector.inspect(message(0, -1), - ResyncDetector.NO_CLIENT_ID); - Result second = detector.inspect(message(1, 0), first.lastClientId()); - Assertions.assertEquals(Kind.NORMAL, second.kind()); - - // response to message 1 was lost; client re-sends message 1 verbatim - Result resend = detector.inspect(message(1, 0), second.lastClientId()); - Assertions.assertEquals(Kind.RESEND, resend.kind()); - Assertions.assertEquals(1, resend.lastClientId(), - "a resend must not advance the remembered clientId"); - - // the next genuine message is normal again - Result next = detector.inspect(message(2, 1), resend.lastClientId()); - Assertions.assertEquals(Kind.NORMAL, next.kind()); - - Assertions.assertEquals(1d, count(MeterNames.RESYNC_TYPE_RESEND)); - Assertions.assertEquals(0d, count(MeterNames.RESYNC_TYPE_RESYNC)); - } - - @Test - void resynchronizeFlag_isDetected_evenWithAdvancingClientId() { - Result first = detector.inspect(message(0, -1), - ResyncDetector.NO_CLIENT_ID); - // resync request carries the next, advancing clientId plus the flag - Result resync = detector.inspect(resyncMessage(1, 0), - first.lastClientId()); - Assertions.assertEquals(Kind.RESYNC, resync.kind()); - Assertions.assertEquals(1, resync.lastClientId()); - - Assertions.assertEquals(1d, count(MeterNames.RESYNC_TYPE_RESYNC)); - Assertions.assertEquals(0d, count(MeterNames.RESYNC_TYPE_RESEND)); - } - - @Test - void emptyOrMalformedBody_isNormal_andDoesNotThrow() { - Assertions.assertEquals(Kind.NORMAL, detector.inspect(null, 3).kind()); - Assertions.assertEquals(Kind.NORMAL, detector.inspect("", 3).kind()); - Assertions.assertEquals(Kind.NORMAL, - detector.inspect("not json", 3).kind()); - // remembered id is preserved across an unparseable body - Assertions.assertEquals(3, - detector.inspect("not json", 3).lastClientId()); - } -} diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ResyncMetricsBinderTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ResyncMetricsBinderTest.java new file mode 100644 index 00000000..8403f3bf --- /dev/null +++ b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ResyncMetricsBinderTest.java @@ -0,0 +1,83 @@ +/** + * Copyright (C) 2000-2026 Vaadin Ltd + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See for the full + * license. + */ +package com.vaadin.observability.micrometer; + +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.vaadin.flow.component.UI; +import com.vaadin.flow.server.VaadinService; +import com.vaadin.flow.server.VaadinServiceEventBus; +import com.vaadin.flow.server.communication.ClientMessageResentEvent; +import com.vaadin.flow.server.communication.MessageIdSyncErrorEvent; +import com.vaadin.flow.server.communication.UIResynchronizationEvent; +import com.vaadin.flow.shared.Registration; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; + +class ResyncMetricsBinderTest { + + private final UI ui = mock(UI.class); + + private SimpleMeterRegistry registry; + private VaadinServiceEventBus eventBus; + + @BeforeEach + void setUp() { + registry = new SimpleMeterRegistry(); + eventBus = new VaadinServiceEventBus(mock(VaadinService.class)); + } + + @Test + void everyTypeReportsZeroBeforeItsFirstEvent() { + new ResyncMetricsBinder(registry).register(eventBus); + + for (String type : new String[] { MeterNames.RESYNC_TYPE_RESEND, + MeterNames.RESYNC_TYPE_RESYNC, + MeterNames.RESYNC_TYPE_OUT_OF_SYNC }) { + assertEquals(0d, count(type), type); + } + } + + @Test + void eachRecoveryEventIsCountedUnderItsType() { + new ResyncMetricsBinder(registry).register(eventBus); + + eventBus.fireEvent(new ClientMessageResentEvent(ui)); + eventBus.fireEvent(new ClientMessageResentEvent(ui)); + eventBus.fireEvent(new UIResynchronizationEvent(ui)); + eventBus.fireEvent(new MessageIdSyncErrorEvent(ui, 3, 7)); + + assertEquals(2d, count(MeterNames.RESYNC_TYPE_RESEND)); + assertEquals(1d, count(MeterNames.RESYNC_TYPE_RESYNC)); + assertEquals(1d, count(MeterNames.RESYNC_TYPE_OUT_OF_SYNC)); + } + + @Test + void removingTheRegistrationStopsCounting() { + Registration registration = new ResyncMetricsBinder(registry) + .register(eventBus); + registration.remove(); + + eventBus.fireEvent(new ClientMessageResentEvent(ui)); + eventBus.fireEvent(new UIResynchronizationEvent(ui)); + eventBus.fireEvent(new MessageIdSyncErrorEvent(ui, 3, 7)); + + assertEquals(0d, count(MeterNames.RESYNC_TYPE_RESEND)); + assertEquals(0d, count(MeterNames.RESYNC_TYPE_RESYNC)); + assertEquals(0d, count(MeterNames.RESYNC_TYPE_OUT_OF_SYNC)); + } + + private double count(String type) { + return registry.get(MeterNames.RESYNC).tag(MeterNames.TAG_TYPE, type) + .counter().count(); + } +} diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/client/MetricsCollectorElementTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/client/MetricsCollectorElementTest.java index bb03bf28..44cb01f8 100644 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/client/MetricsCollectorElementTest.java +++ b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/client/MetricsCollectorElementTest.java @@ -8,6 +8,7 @@ */ package com.vaadin.observability.micrometer.client; +import java.util.ArrayList; import java.util.List; import java.util.stream.Stream; @@ -18,13 +19,31 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import com.vaadin.flow.component.Component; +import com.vaadin.flow.component.HasComponents; +import com.vaadin.flow.component.Tag; +import com.vaadin.flow.component.UI; +import com.vaadin.flow.server.Command; import com.vaadin.flow.server.VaadinService; import com.vaadin.flow.server.VaadinSession; import com.vaadin.observability.micrometer.MeterNames; import com.vaadin.observability.micrometer.ObservabilitySettings; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + class MetricsCollectorElementTest { + @Tag("div") + private static final class Layout extends Component + implements HasComponents { + } + + /** The tasks {@code UI.access} queued, run by the test. */ + private final List accessTasks = new ArrayList<>(); + private SimpleMeterRegistry registry; private ObservabilitySettings settings; private ClientMetricsBinder binder; @@ -64,6 +83,28 @@ private static List samples(int count) { return Stream.generate(ClientSample::new).limit(count).toList(); } + /** + * A UI whose session queues {@code UI.access} tasks for the test to run. + */ + private UI uiQueueingAccessTasks() { + VaadinSession session = mock(VaadinSession.class, RETURNS_DEEP_STUBS); + when(session.hasLock()).thenReturn(true); + when(session.access(any())).thenAnswer(invocation -> { + accessTasks.add(invocation.getArgument(0)); + return null; + }); + UI ui = new UI(); + ui.getInternals().setSession(session); + return ui; + } + + /** Runs the queued tasks, including ones queued while running them. */ + private void runAccessTasks() { + for (int i = 0; i < accessTasks.size() && i < 10; i++) { + accessTasks.get(i).execute(); + } + } + @Test void tabsOfOneSessionShareTheBudget() { VaadinSession.setCurrent(lockedSession()); @@ -98,4 +139,48 @@ void withoutASessionTheElementStillLimits() { Assertions.assertEquals(2.0, throttled()); } + + @Test + void removedElementIsAddedBackToItsUi() { + UI ui = uiQueueingAccessTasks(); + MetricsCollectorElement element = tab(); + ui.add(element); + + ui.remove(element); + runAccessTasks(); + + Assertions.assertSame(ui, element.getParent().orElse(null)); + } + + @Test + void elementInARemovedLayoutIsAddedBackToItsUi() { + UI ui = uiQueueingAccessTasks(); + Layout layout = new Layout(); + MetricsCollectorElement element = tab(); + layout.add(element); + ui.add(layout); + + ui.remove(layout); + runAccessTasks(); + + Assertions.assertSame(ui, element.getParent().orElse(null)); + } + + /** + * A resynchronization detaches and re-attaches every node in place. The + * element is still in the UI then, so adding it again would only move it, + * detach it once more and queue another add, forever. + */ + @Test + void resynchronizationDoesNotAddTheElementAgain() { + UI ui = uiQueueingAccessTasks(); + MetricsCollectorElement element = tab(); + ui.add(element); + + ui.getInternals().getStateTree().prepareForResync(); + runAccessTasks(); + + Assertions.assertEquals(1, accessTasks.size()); + Assertions.assertSame(ui, element.getParent().orElse(null)); + } } diff --git a/observability-kit-spring/src/main/java/com/vaadin/observability/spring/SpringMetricsServiceInitListener.java b/observability-kit-spring/src/main/java/com/vaadin/observability/spring/SpringMetricsServiceInitListener.java index 3d240da2..76f73eff 100644 --- a/observability-kit-spring/src/main/java/com/vaadin/observability/spring/SpringMetricsServiceInitListener.java +++ b/observability-kit-spring/src/main/java/com/vaadin/observability/spring/SpringMetricsServiceInitListener.java @@ -10,6 +10,7 @@ import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.observation.ObservationRegistry; +import org.springframework.util.ClassUtils; import com.vaadin.flow.server.VaadinRequest; import com.vaadin.observability.micrometer.MetricsServiceInitListener; @@ -27,7 +28,9 @@ * {@link SpringHttpObservationEnricher}, making the parent HTTP span render as * e.g. {@code http post /vaadin/uidl} instead of the generic * {@code http post /**}, and marking it errored when Vaadin request handling - * raises an exception the servlet chain never sees. + * raises an exception the servlet chain never sees. The enrichment is skipped + * when spring-web is not on the classpath: there is then no HTTP observation to + * enrich, and the enricher could not even be loaded. *

* This class is declared {@code public} so it can be reused by both * {@link ObservabilityConfiguration} (plain-Spring import) and the Boot @@ -36,6 +39,16 @@ public final class SpringMetricsServiceInitListener extends MetricsServiceInitListener { + /** + * Whether spring-web, which {@link SpringHttpObservationEnricher} links + * against, is on the classpath. Neither the starter nor vaadin-spring bring + * it at runtime, and a missing class would surface as a + * {@link NoClassDefFoundError} on the first request. + */ + private static final boolean SPRING_WEB_PRESENT = ClassUtils.isPresent( + "org.springframework.web.filter.ServerHttpObservationFilter", + SpringMetricsServiceInitListener.class.getClassLoader()); + /** * Creates a new listener. * @@ -65,18 +78,24 @@ protected void installDefaultObservationHandlers( @Override protected void enrichHttpObservation(VaadinRequest request, String requestType) { - SpringHttpObservationEnricher.enrich(request, requestType); + if (SPRING_WEB_PRESENT) { + SpringHttpObservationEnricher.enrich(request, requestType); + } } @Override protected void enrichHttpObservationRoute(VaadinRequest request, String routeTemplate) { - SpringHttpObservationEnricher.route(request, routeTemplate); + if (SPRING_WEB_PRESENT) { + SpringHttpObservationEnricher.route(request, routeTemplate); + } } @Override protected void markHttpObservationError(VaadinRequest request, Throwable failure) { - SpringHttpObservationEnricher.error(request, failure); + if (SPRING_WEB_PRESENT) { + SpringHttpObservationEnricher.error(request, failure); + } } } diff --git a/observability-kit-spring/src/main/java/com/vaadin/observability/spring/SpringResyncDetectionFilter.java b/observability-kit-spring/src/main/java/com/vaadin/observability/spring/SpringResyncDetectionFilter.java deleted file mode 100644 index d443e577..00000000 --- a/observability-kit-spring/src/main/java/com/vaadin/observability/spring/SpringResyncDetectionFilter.java +++ /dev/null @@ -1,140 +0,0 @@ -/** - * Copyright (C) 2000-2026 Vaadin Ltd - * - * This program is available under Vaadin Commercial License and Service Terms. - * - * See for the full - * license. - */ -package com.vaadin.observability.spring; - -import jakarta.servlet.FilterChain; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import jakarta.servlet.http.HttpSession; - -import java.io.IOException; -import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; - -import io.micrometer.core.instrument.MeterRegistry; -import org.springframework.web.filter.OncePerRequestFilter; -import org.springframework.web.util.ContentCachingRequestWrapper; -import org.springframework.web.util.WebUtils; - -import com.vaadin.flow.server.communication.UidlRequestHandler; -import com.vaadin.observability.micrometer.ResyncInspector; - -/** - * Spring-optimized servlet filter that observes UIDL message resends and - * resynchronization requests (prototype, kit-only, no Flow changes). Shares its - * detection logic with the portable - * {@link com.vaadin.observability.micrometer.ResyncDetectionFilter} via - * {@link ResyncInspector}, but is preferred wherever Spring is on the - * classpath. - *

- * Flow recovers from lost responses entirely inside {@link UidlRequestHandler} - * by catching {@code ClientResentPayloadException} (replay the cached response) - * and {@code ResynchronizationRequiredException} (rebuild the UI state); - * neither surfaces to any Flow listener SPI the kit uses. - *

- * Rather than buffer the body eagerly, this filter wraps the request in a - * {@link ContentCachingRequestWrapper} and inspects after the chain - * has run: ordering is irrelevant for a counter, and this way we read exactly - * the bytes Flow read and only count requests Flow actually consumed. The - * per-UI read-modify-write is guarded by the session mutex. Instrumentation - * never fails the request: any error while inspecting is swallowed. - */ -public final class SpringResyncDetectionFilter extends OncePerRequestFilter { - - private final ResyncInspector inspector; - - /** - * Creates the filter recording into the given registry. - * - * @param registry - * the meter registry, not {@code null} - */ - public SpringResyncDetectionFilter(MeterRegistry registry) { - this.inspector = new ResyncInspector(registry); - } - - @Override - protected void doFilterInternal(HttpServletRequest request, - HttpServletResponse response, FilterChain chain) - throws ServletException, IOException { - if (!ResyncInspector.isUidl(request)) { - chain.doFilter(request, response); - return; - } - - // Bounded cache: Spring sizes its first buffer from the client-sent - // Content-Length (capped by this limit), so an unbounded limit would - // let a forged header allocate gigabytes. A body over the limit is - // not inspected, as a truncated one could hide the resync/clientId - // fields, which the UIDL JSON places after the (potentially large) - // rpc array. - LimitedCachingRequestWrapper wrapped = new LimitedCachingRequestWrapper( - request, ResyncInspector.MAX_INSPECTED_BODY_BYTES); - try { - chain.doFilter(wrapped, response); - } finally { - try { - inspect(wrapped); - } catch (RuntimeException instrumentationFailure) { - // Never break a request because of observability. - } - } - } - - private void inspect(LimitedCachingRequestWrapper request) { - if (request.overflowed) { - // Only a prefix was cached; nothing reliable to classify. - return; - } - byte[] cached = request.getContentAsByteArray(); - if (cached.length == 0) { - // Flow never read the body; nothing to classify. - return; - } - String body = new String(cached, charset(request)); - - HttpSession session = request.getSession(false); - Object mutex = session != null ? WebUtils.getSessionMutex(session) - : this; - inspector.inspect(body, session, ResyncInspector.uiId(request), mutex); - } - - private static Charset charset(HttpServletRequest request) { - String enc = request.getCharacterEncoding(); - if (enc != null) { - try { - return Charset.forName(enc); - } catch (RuntimeException unsupported) { - // fall through to default - } - } - return StandardCharsets.UTF_8; - } - - /** - * Caching wrapper that remembers whether the body outgrew the cache limit. - * Spring stops caching at the limit but keeps passing the bytes through, so - * Flow still reads the whole body. - */ - private static final class LimitedCachingRequestWrapper - extends ContentCachingRequestWrapper { - - private boolean overflowed; - - LimitedCachingRequestWrapper(HttpServletRequest request, int limit) { - super(request, limit); - } - - @Override - protected void handleContentOverflow(int contentCacheLimit) { - overflowed = true; - } - } -} diff --git a/observability-kit-spring/src/test/java/com/vaadin/observability/spring/SpringMetricsServiceInitListenerTest.java b/observability-kit-spring/src/test/java/com/vaadin/observability/spring/SpringMetricsServiceInitListenerTest.java new file mode 100644 index 00000000..a79ba037 --- /dev/null +++ b/observability-kit-spring/src/test/java/com/vaadin/observability/spring/SpringMetricsServiceInitListenerTest.java @@ -0,0 +1,143 @@ +/** + * Copyright (C) 2026 Vaadin Ltd + * + * This program is available under Vaadin Commercial License and Service Terms. + * + * See for the full + * license. + */ +package com.vaadin.observability.spring; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; + +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import io.micrometer.observation.ObservationRegistry; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.springframework.http.server.observation.ServerRequestObservationContext; +import org.springframework.web.filter.ServerHttpObservationFilter; + +import com.vaadin.flow.server.VaadinRequest; +import com.vaadin.observability.micrometer.MetricsServiceInitListener; +import com.vaadin.observability.micrometer.ObservabilitySettings; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +class SpringMetricsServiceInitListenerTest { + + @Test + void enrichmentReachesTheSpringObservationWithSpringWeb() { + SpringMetricsServiceInitListener listener = new SpringMetricsServiceInitListener( + new SimpleMeterRegistry(), ObservationRegistry.NOOP, + ObservabilitySettings.builder().build()); + ServerRequestObservationContext context = new ServerRequestObservationContext( + Mockito.mock(HttpServletRequest.class), + Mockito.mock(HttpServletResponse.class)); + VaadinRequest request = Mockito.mock(VaadinRequest.class); + Mockito.when(request.getMethod()).thenReturn("POST"); + Mockito.when(request.getAttribute( + ServerHttpObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE)) + .thenReturn(context); + + listener.enrichHttpObservation(request, "uidl"); + + assertThat(context.getPathPattern()).isEqualTo("/vaadin/uidl"); + } + + /** + * Neither the starter nor vaadin-spring bring spring-web at runtime, so the + * listener must not touch Spring's HTTP observation classes when they are + * missing. Linking against them there would throw + * {@link NoClassDefFoundError}, which no best-effort catch stops. + */ + @Test + void enrichmentIsSkippedWithoutSpringWeb() throws Exception { + ClassLoader withoutSpringWeb = new WithoutSpringWebClassLoader( + getClass().getClassLoader()); + Class listenerClass = withoutSpringWeb + .loadClass(SpringMetricsServiceInitListener.class.getName()); + Constructor constructor = listenerClass.getConstructor( + MeterRegistry.class, ObservationRegistry.class, + ObservabilitySettings.class); + Object listener = constructor.newInstance(new SimpleMeterRegistry(), + ObservationRegistry.NOOP, + ObservabilitySettings.builder().build()); + VaadinRequest request = Mockito.mock(VaadinRequest.class); + + assertThatCode(() -> { + invoke(listener, "enrichHttpObservation", String.class, request, + "uidl"); + invoke(listener, "enrichHttpObservationRoute", String.class, + request, "orders/:id"); + invoke(listener, "markHttpObservationError", Throwable.class, + request, new IllegalStateException("boom")); + }).doesNotThrowAnyException(); + } + + private static void invoke(Object listener, String name, + Class argumentType, VaadinRequest request, Object argument) + throws Exception { + Method method = MetricsServiceInitListener.class.getDeclaredMethod(name, + VaadinRequest.class, argumentType); + method.setAccessible(true); + method.invoke(listener, request, argument); + } + + /** + * Loads this module's classes itself and hides spring-web, delegating + * everything else to the test class loader. + */ + private static final class WithoutSpringWebClassLoader extends ClassLoader { + + private static final String OWN_PACKAGE = SpringMetricsServiceInitListener.class + .getPackageName() + "."; + + WithoutSpringWebClassLoader(ClassLoader parent) { + super(parent); + } + + @Override + protected Class loadClass(String name, boolean resolve) + throws ClassNotFoundException { + synchronized (getClassLoadingLock(name)) { + if (name.startsWith("org.springframework.web.") + || name.startsWith("org.springframework.http.")) { + throw new ClassNotFoundException(name); + } + if (!name.startsWith(OWN_PACKAGE)) { + return super.loadClass(name, resolve); + } + Class loaded = findLoadedClass(name); + if (loaded == null) { + loaded = define(name); + } + if (resolve) { + resolveClass(loaded); + } + return loaded; + } + } + + private Class define(String name) throws ClassNotFoundException { + String resource = name.replace('.', '/') + ".class"; + try (InputStream in = getParent().getResourceAsStream(resource)) { + if (in == null) { + throw new ClassNotFoundException(name); + } + byte[] bytes = in.readAllBytes(); + return defineClass(name, bytes, 0, bytes.length); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + } +} diff --git a/observability-kit-spring/src/test/java/com/vaadin/observability/spring/SpringResyncDetectionFilterTest.java b/observability-kit-spring/src/test/java/com/vaadin/observability/spring/SpringResyncDetectionFilterTest.java deleted file mode 100644 index 02094e3f..00000000 --- a/observability-kit-spring/src/test/java/com/vaadin/observability/spring/SpringResyncDetectionFilterTest.java +++ /dev/null @@ -1,108 +0,0 @@ -/** - * Copyright (C) 2000-2026 Vaadin Ltd - * - * This program is available under Vaadin Commercial License and Service Terms. - * - * See for the full - * license. - */ -package com.vaadin.observability.spring; - -import jakarta.servlet.FilterChain; - -import java.nio.charset.StandardCharsets; - -import io.micrometer.core.instrument.simple.SimpleMeterRegistry; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.mock.web.MockHttpSession; - -import com.vaadin.observability.micrometer.ResyncInspector; - -class SpringResyncDetectionFilterTest { - - private static final String LAST_CLIENT_ID_ATTR = ResyncInspector.class - .getName() + ".lastClientId.0"; - - private final SpringResyncDetectionFilter filter = new SpringResyncDetectionFilter( - new SimpleMeterRegistry()); - - /** Stands in for Flow: reads the whole body, as UidlRequestHandler does. */ - private final FilterChain readBody = (req, res) -> req.getInputStream() - .readAllBytes(); - - @Test - void regularUidlBodyIsInspected() throws Exception { - MockHttpServletRequest request = uidl(message(0)); - - filter.doFilter(request, new MockHttpServletResponse(), readBody); - - Assertions.assertEquals(0, - request.getSession().getAttribute(LAST_CLIENT_ID_ATTR)); - } - - @Test - void forgedContentLengthDoesNotDriveTheAllocation() throws Exception { - // Before the cache was bounded, Spring sized its first buffer from - // this header and allocated ~2 GB for a tiny body. - MockHttpServletRequest request = new MockHttpServletRequest("POST", - "/") { - @Override - public int getContentLength() { - return Integer.MAX_VALUE - 8; - } - - @Override - public long getContentLengthLong() { - return Integer.MAX_VALUE - 8; - } - }; - prepareUidl(request, message(0)); - - filter.doFilter(request, new MockHttpServletResponse(), readBody); - - Assertions.assertEquals(0, - request.getSession().getAttribute(LAST_CLIENT_ID_ATTR)); - } - - @Test - void bodyOverTheLimitReachesFlowButIsNotInspected() throws Exception { - String body = "{\"rpc\":[\"" - + "x".repeat(ResyncInspector.MAX_INSPECTED_BODY_BYTES) - + "\"],\"syncId\":0,\"clientId\":0}"; - MockHttpServletRequest request = uidl(body); - byte[][] seenByFlow = new byte[1][]; - - filter.doFilter(request, new MockHttpServletResponse(), (req, - res) -> seenByFlow[0] = req.getInputStream().readAllBytes()); - - Assertions.assertEquals(body, - new String(seenByFlow[0], StandardCharsets.UTF_8)); - Assertions.assertNull( - request.getSession().getAttribute(LAST_CLIENT_ID_ATTR)); - } - - private static MockHttpServletRequest uidl(String body) { - MockHttpServletRequest request = new MockHttpServletRequest("POST", - "/"); - prepareUidl(request, body); - return request; - } - - private static void prepareUidl(MockHttpServletRequest request, - String body) { - request.setQueryString("v-r=uidl&v-uiId=0"); - request.addParameter("v-r", "uidl"); - request.addParameter("v-uiId", "0"); - request.setCharacterEncoding("UTF-8"); - request.setContent(body.getBytes(StandardCharsets.UTF_8)); - request.setSession(new MockHttpSession()); - } - - private static String message(int clientId) { - return "{\"csrfToken\":\"x\",\"rpc\":[],\"syncId\":" + (clientId - 1) - + ",\"clientId\":" + clientId + "}"; - } -} diff --git a/observability-kit-starter/pom.xml b/observability-kit-starter/pom.xml index 92b77d23..77154f98 100644 --- a/observability-kit-starter/pom.xml +++ b/observability-kit-starter/pom.xml @@ -61,15 +61,6 @@ spring-boot-actuator true - - - jakarta.servlet - jakarta.servlet-api - ${servlet.api.version} - provided - diff --git a/observability-kit-starter/src/main/java/com/vaadin/observability/spring/boot/ObservabilityAutoConfiguration.java b/observability-kit-starter/src/main/java/com/vaadin/observability/spring/boot/ObservabilityAutoConfiguration.java index 90b4f674..1472f7d3 100644 --- a/observability-kit-starter/src/main/java/com/vaadin/observability/spring/boot/ObservabilityAutoConfiguration.java +++ b/observability-kit-starter/src/main/java/com/vaadin/observability/spring/boot/ObservabilityAutoConfiguration.java @@ -23,7 +23,6 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.micrometer.metrics.autoconfigure.CompositeMeterRegistryAutoConfiguration; import org.springframework.boot.micrometer.metrics.autoconfigure.MetricsAutoConfiguration; -import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Role; @@ -34,7 +33,6 @@ import com.vaadin.observability.micrometer.MetricsServiceInitListener; import com.vaadin.observability.micrometer.ObservabilitySettings; import com.vaadin.observability.spring.SpringMetricsServiceInitListener; -import com.vaadin.observability.spring.SpringResyncDetectionFilter; /** * Auto-configures the Observability Kit {@link MetricsServiceInitListener} when @@ -77,26 +75,6 @@ MetricsServiceInitListener metricsServiceInitListener( observationRegistry.getIfAvailable(), settings); } - /** - * Registers the prototype {@link SpringResyncDetectionFilter}, which - * observes UIDL message resends and client-requested resynchronizations by - * inspecting UIDL request bodies. Runs at highest precedence so it wraps - * the request before any other filter consumes the body, and gated by - * {@code vaadin.observability.resync} (default {@code true}). - */ - @Bean - @ConditionalOnBean(MeterRegistry.class) - @ConditionalOnMissingBean - @ConditionalOnProperty(prefix = "vaadin.observability", name = "resync", havingValue = "true", matchIfMissing = true) - FilterRegistrationBean resyncDetectionFilter( - MeterRegistry registry) { - FilterRegistrationBean registration = new FilterRegistrationBean<>( - new SpringResyncDetectionFilter(registry)); - registration.addUrlPatterns("/*"); - registration.setOrder(Ordered.HIGHEST_PRECEDENCE); - return registration; - } - /** * Wraps {@link DataSource} beans to record {@code vaadin.db.fetch.rows}. * Opt-in via {@code vaadin.observability.database=true} since it reaches diff --git a/observability-kit-tests/observability-kit-tests-starter/src/test/java/com/vaadin/observability/tests/starter/ResyncMetricsIT.java b/observability-kit-tests/observability-kit-tests-starter/src/test/java/com/vaadin/observability/tests/starter/ResyncMetricsIT.java index 99972c6f..3bb55239 100644 --- a/observability-kit-tests/observability-kit-tests-starter/src/test/java/com/vaadin/observability/tests/starter/ResyncMetricsIT.java +++ b/observability-kit-tests/observability-kit-tests-starter/src/test/java/com/vaadin/observability/tests/starter/ResyncMetricsIT.java @@ -8,16 +8,12 @@ */ package com.vaadin.observability.tests.starter; -import java.io.BufferedReader; import java.io.IOException; -import java.io.InputStreamReader; -import java.io.OutputStream; -import java.net.HttpURLConnection; -import java.net.URI; -import java.nio.charset.StandardCharsets; +import java.time.Duration; -import org.openqa.selenium.Cookie; +import org.openqa.selenium.JavascriptExecutor; +import com.vaadin.flow.component.html.testbench.NativeButtonElement; import com.vaadin.flow.component.html.testbench.SpanElement; import com.vaadin.observability.tests.common.AbstractIT; import com.vaadin.testbench.BrowserTest; @@ -26,19 +22,20 @@ /** * Drives the Spring Boot + {@code observability-kit-starter} app and verifies - * the prototype {@code vaadin.resync} counter is exported via Prometheus when - * the UIDL request stream contains a re-sent (duplicate) message and a - * client-requested resynchronization. + * the {@code vaadin.resync} counter is exported via Prometheus for each kind of + * client message recovery Flow reports on the service event bus. * *

- * Flow's own resend/resync recovery is invisible to listener SPIs (it is caught - * internally in {@code UidlRequestHandler}). The kit reconstructs the signal in - * {@code ResyncDetectionFilter} by inspecting the UIDL body, so this test - * replays forged UIDL POSTs through the real servlet filter chain: the filter - * wraps each request, lets Flow read the body, then classifies and counts from - * the bytes Flow consumed. A real browser load first establishes the HTTP - * session whose cookie the replayed requests reuse, so the filter's per-UI - * {@code clientId} state persists across the duplicate. + * Flow only fires the events for messages it accepts: the UI must exist and the + * CSRF token must match. So the test records the UIDL messages the real client + * sends, stops the client from sending more once a click has been answered, and + * replays that last message from the page, as is, and with the message id + * changed: + *

    + *
  • the same message again is a resend,
    • + *
  • a message id the server does not expect is out of sync,
    • + *
  • the next message id with the resynchronize flag is a resync.
    • + *
* *

* The Micrometer counter {@code vaadin.resync} is exposed by Prometheus as @@ -46,8 +43,54 @@ */ public class ResyncMetricsIT extends AbstractIT { - /** A UI id unlikely to collide with the browser's own UIDL traffic. */ - private static final String UI_ID = "999"; + private static final Duration TIMEOUT = Duration.ofSeconds(20); + + /** + * Records every UIDL message the client sends, and drops the ones sent + * after {@code blocked} is set so the server's last processed message id + * stays put while the test replays. + */ + private static final String RECORD_UIDL = """ + window.__uidl = { sent: [], blocked: false }; + const open = XMLHttpRequest.prototype.open; + const send = XMLHttpRequest.prototype.send; + XMLHttpRequest.prototype.open = function (method, url) { + this.__url = String(url); + return open.apply(this, arguments); + }; + XMLHttpRequest.prototype.send = function (body) { + if (this.__url && this.__url.indexOf('v-r=uidl') >= 0) { + if (window.__uidl.blocked) { + return; + } + const entry = { url: this.__url, body: body, done: false }; + window.__uidl.sent.push(entry); + this.addEventListener('loadend', () => entry.done = true); + } + return send.apply(this, arguments); + }; + """; + + /** + * Replays the last recorded message as is (a resend), with a message id the + * server does not expect (out of sync), and with the next message id and + * the resynchronize flag (a resync). + */ + private static final String REPLAY_UIDL = """ + const done = arguments[arguments.length - 1]; + const last = window.__uidl.sent[window.__uidl.sent.length - 1]; + const message = JSON.parse(last.body); + const post = body => fetch(last.url, { method: 'POST', + headers: { 'Content-Type': 'application/json; charset=UTF-8' }, + body: body }); + post(last.body) + .then(() => post(JSON.stringify( + Object.assign({}, message, { clientId: 99999 })))) + .then(() => post(JSON.stringify(Object.assign({}, message, + { clientId: message.clientId + 1, rpc: [], + resynchronize: true })))) + .then(() => done(true), () => done(false)); + """; @Override protected String getTestPath() { @@ -55,78 +98,47 @@ protected String getTestPath() { } @BrowserTest - public void resendAndResyncAreCountedAndExported() throws IOException { - // Ensure the app has loaded and a server session exists. + public void resendResyncAndOutOfSyncAreCountedAndExported() + throws IOException { SpanElement greeting = $(SpanElement.class).id("greeting"); assertThat(greeting.getText()).isEqualTo("Hello micrometer boot"); - String sessionCookie = jsessionId(); - assertThat(sessionCookie).as("JSESSIONID cookie from the page load") - .isNotNull(); - - String uidlUrl = getRootURL() + "/?v-r=uidl&v-uiId=" + UI_ID; - - // 1) baseline message establishes the last seen clientId for this UI - postUidl(uidlUrl, sessionCookie, uidlBody(10, false)); - // 2) same clientId again: the client re-sent a message the server - // already processed -> resend - postUidl(uidlUrl, sessionCookie, uidlBody(10, false)); - // 3) advancing clientId carrying the resynchronize flag -> resync - postUidl(uidlUrl, sessionCookie, uidlBody(11, true)); - - String prometheus = fetchPrometheus(); + JavascriptExecutor js = (JavascriptExecutor) getDriver(); + js.executeScript(RECORD_UIDL); + + $(NativeButtonElement.class).id("bump").click(); + waitUntil(driver -> "1" + .equals($(SpanElement.class).id("clicks").getText())); + + // A dropped message never completes, so Flow never reports itself + // idle again: stop waiting for it before blocking the client. + testBench().disableWaitForVaadin(); + js.executeScript("window.__uidl.blocked = true;"); + waitUntil(driver -> Boolean.TRUE.equals(js.executeScript( + "const s = window.__uidl.sent; return s.length > 0 && s.every(e => e.done);"))); + + assertThat(js.executeAsyncScript(REPLAY_UIDL)) + .as("the replayed UIDL requests should complete") + .isEqualTo(Boolean.TRUE); + + String prometheus = awaitPrometheus( + body -> prometheusValue(body, "vaadin_resync_total", + "type=\"resend\"") >= 1.0 + && prometheusValue(body, "vaadin_resync_total", + "type=\"out_of_sync\"") >= 1.0 + && prometheusValue(body, "vaadin_resync_total", + "type=\"resync\"") >= 1.0, + "a resend, an out-of-sync message and a resync", TIMEOUT); assertThat(prometheusValue(prometheus, "vaadin_resync_total", "type=\"resend\"")).as("vaadin_resync_total{type=\"resend\"}") .isGreaterThanOrEqualTo(1.0); + assertThat(prometheusValue(prometheus, "vaadin_resync_total", + "type=\"out_of_sync\"")) + .as("vaadin_resync_total{type=\"out_of_sync\"}") + .isGreaterThanOrEqualTo(1.0); assertThat(prometheusValue(prometheus, "vaadin_resync_total", "type=\"resync\"")).as("vaadin_resync_total{type=\"resync\"}") .isGreaterThanOrEqualTo(1.0); } - - private String jsessionId() { - Cookie cookie = getDriver().manage().getCookieNamed("JSESSIONID"); - return cookie != null ? cookie.getValue() : null; - } - - private static String uidlBody(int clientId, boolean resync) { - String base = "{\"csrfToken\":\"x\",\"rpc\":[],\"syncId\":0,\"clientId\":" - + clientId; - return resync ? base + ",\"resynchronize\":true}" : base + "}"; - } - - private static void postUidl(String url, String jsessionId, String body) - throws IOException { - HttpURLConnection conn = (HttpURLConnection) URI.create(url).toURL() - .openConnection(); - conn.setRequestMethod("POST"); - conn.setDoOutput(true); - conn.setRequestProperty("Content-Type", - "application/json; charset=UTF-8"); - conn.setRequestProperty("Cookie", "JSESSIONID=" + jsessionId); - try (OutputStream out = conn.getOutputStream()) { - out.write(body.getBytes(StandardCharsets.UTF_8)); - } - // The forged UIDL is rejected by Flow (bad CSRF / no matching UI), but - // the filter has already counted it; the response status is irrelevant. - conn.getResponseCode(); - conn.disconnect(); - } - - private String fetchPrometheus() throws IOException { - HttpURLConnection conn = (HttpURLConnection) URI - .create(getRootURL() + "/actuator/prometheus").toURL() - .openConnection(); - conn.setRequestMethod("GET"); - assertThat(conn.getResponseCode()).isEqualTo(200); - StringBuilder out = new StringBuilder(); - try (BufferedReader reader = new BufferedReader(new InputStreamReader( - conn.getInputStream(), StandardCharsets.UTF_8))) { - String line; - while ((line = reader.readLine()) != null) { - out.append(line).append('\n'); - } - } - return out.toString(); - } }