diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ClientResourceLoader.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ClientResourceLoader.java deleted file mode 100644 index 7d07955c..00000000 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ClientResourceLoader.java +++ /dev/null @@ -1,95 +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 java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; - -import org.slf4j.LoggerFactory; - -import com.vaadin.flow.component.ComponentUtil; -import com.vaadin.flow.component.UI; -import com.vaadin.flow.internal.StringUtil; - -/** - * Loads a bundled client-side JavaScript resource into a {@link UI} exactly - * once. The classpath resource is read, stripped of comments and executed via - * {@link com.vaadin.flow.component.page.Page#executeJs}. A per-UI flag keyed by - * {@code initKey} guards against repeated injection. - */ -public final class ClientResourceLoader { - - private ClientResourceLoader() { - } - - /** - * Injects {@code resource} into {@code ui} once. Subsequent calls with the - * same {@code initKey} for the same UI are no-ops. Missing resources or - * read failures are logged against {@code owner} and otherwise ignored. - * - * @param ui - * the target UI; {@code null} is ignored - * @param initKey - * the per-UI data key used to ensure single injection - * @param resource - * the classpath resource path of the JavaScript to load - * @param owner - * the class whose class loader and logger are used - */ - public static void loadOnce(UI ui, String initKey, String resource, - Class owner) { - loadOnce(ui, initKey, resource, owner, null); - } - - /** - * Injects {@code resource} into {@code ui} once, preceded by - * {@code prelude} in the same script. - * - * @param ui - * the target UI; {@code null} is ignored - * @param initKey - * the per-UI data key used to ensure single injection - * @param resource - * the classpath resource path of the JavaScript to load - * @param owner - * the class whose class loader and logger are used - * @param prelude - * JavaScript run immediately before the resource, or - * {@code null} for none. The way to hand the script a - * server-side decision it cannot read for itself; it is executed - * as written, so it must be server-authored and must carry no - * comments, which are stripped from the resource but not from - * this - */ - public static void loadOnce(UI ui, String initKey, String resource, - Class owner, String prelude) { - if (ui == null || ComponentUtil.getData(ui, initKey) != null) { - return; - } - ComponentUtil.setData(ui, initKey, Boolean.TRUE); - try (InputStream in = owner.getClassLoader() - .getResourceAsStream(resource)) { - if (in == null) { - LoggerFactory.getLogger(owner).warn( - "observability-kit client resource not found: {}", - resource); - return; - } - String js = StringUtil.removeComments( - new String(in.readAllBytes(), StandardCharsets.UTF_8), - true); - ui.getPage().executeJs(prelude == null ? js : prelude + "\n" + js); - } catch (IOException e) { - LoggerFactory.getLogger(owner).warn( - "Could not load observability-kit client resource: {}", - resource, e); - } - } -} 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 b68369ce..9e0f09ce 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 @@ -222,6 +222,7 @@ public void serviceInit(ServiceInitEvent event) { boolean productionMode = event.getSource().getDeploymentConfiguration() .isProductionMode(); if (!ObservabilityLicense.isLicensed(productionMode)) { + ObservabilityKit.setLicenseMissing(true); LOGGER.warn( "No valid {} license found. Observability Kit instrumentation " + "will not be registered and no telemetry will be collected. " @@ -234,14 +235,10 @@ public void serviceInit(ServiceInitEvent event) { : ObservabilityKit.getObservationRegistry(); // Record the bound registry so the dev-mode Copilot metrics panel can // read the live meters regardless of deployment type. + ObservabilityKit.setLicenseMissing(false); ObservabilityKit.setActiveMeterRegistry(r); ObservabilityUsage.markAsUsed(s); bind(event, r, or, s); - if (!productionMode) { - event.getSource() - .addUIInitListener(uiEvent -> ObservabilityDevToolsClient - .inject(uiEvent.getUI())); - } } void bind(ServiceInitEvent event, MeterRegistry registry, diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilityDevToolsClient.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilityDevToolsClient.java deleted file mode 100644 index 3461b3be..00000000 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilityDevToolsClient.java +++ /dev/null @@ -1,33 +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 com.vaadin.flow.component.UI; - -/** - * Loads the in-browser Vaadin Copilot metrics panel. The panel registers itself - * with Copilot's plugin API and pulls metric snapshots from the server over the - * dev-tools websocket (see {@code ObservabilityDevToolsHandler}). - *

- * Injected once per UI and only in development mode; in production Copilot and - * the dev-tools connection do not exist, so this is never called. - */ -final class ObservabilityDevToolsClient { - - private static final String INIT_KEY = "vaadinObservabilityDevToolsInitialized"; - private static final String CLIENT_RESOURCE = "META-INF/frontend/VaadinObservabilityDevTools.js"; - - private ObservabilityDevToolsClient() { - } - - static void inject(UI ui) { - ClientResourceLoader.loadOnce(ui, INIT_KEY, CLIENT_RESOURCE, - ObservabilityDevToolsClient.class); - } -} diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilityKit.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilityKit.java index ae40645b..352923f9 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilityKit.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/ObservabilityKit.java @@ -9,6 +9,7 @@ package com.vaadin.observability.micrometer; import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import io.micrometer.core.instrument.MeterRegistry; @@ -55,6 +56,13 @@ public final class ObservabilityKit { */ private static final AtomicReference RECENT_CLIENT_ERRORS = new AtomicReference<>(); + /** + * Whether {@code serviceInit} skipped binding because the kit has no valid + * license. Read by the dev-mode Copilot panel, which says so rather than + * showing empty sections that read like an idle application. + */ + private static final AtomicBoolean LICENSE_MISSING = new AtomicBoolean(); + private ObservabilityKit() { } @@ -91,6 +99,24 @@ static void setActiveMeterRegistry(MeterRegistry registry) { ACTIVE_METER_REGISTRY.set(registry); } + /** + * Records whether the license check at {@code serviceInit} failed. Called + * from {@code MetricsServiceInitListener}. + */ + static void setLicenseMissing(boolean missing) { + LICENSE_MISSING.set(missing); + } + + /** + * Whether instrumentation was not bound because the kit has no valid + * license. Read by the dev-mode Copilot metrics panel. + * + * @return {@code true} if the last license check failed + */ + public static boolean isLicenseMissing() { + return LICENSE_MISSING.get(); + } + /** * The registry instrumentation is currently publishing to, or {@code null} * if instrumentation has not been bound. Read by the dev-mode Copilot @@ -171,5 +197,6 @@ static void reset() { RECENT_INTERACTIONS.set(null); RECENT_QUERIES.set(null); RECENT_CLIENT_ERRORS.set(null); + LICENSE_MISSING.set(false); } } 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 8da1b3fa..0cb0e54f 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 @@ -10,26 +10,24 @@ import java.util.List; -import com.vaadin.flow.component.AttachEvent; import com.vaadin.flow.component.ClientCallable; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.Tag; import com.vaadin.flow.component.UI; -import com.vaadin.observability.micrometer.ClientResourceLoader; +import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.observability.micrometer.ObservabilitySettings; /** * Hidden helper component attached to each UI when client metrics are enabled. * Exposes a {@link ClientCallable} that receives batches of - * {@link ClientSample} from the in-browser collector. Loads the client JS on - * first attach and re-attaches itself to its UI if removed. + * {@link ClientSample} from the in-browser collector, whose script defines the + * element and starts collecting once it is attached. Re-attaches itself to its + * UI if removed. */ +@JsModule("./VaadinMetricsClient.js") @Tag("vaadin-metrics-collector") public final class MetricsCollectorElement extends Component { - private static final String CLIENT_INIT_KEY = "vaadinMetricsClientInitialized"; - private static final String CLIENT_RESOURCE = "META-INF/frontend/VaadinMetricsClient.js"; - /** * Tells the in-browser collector whether to gather the message of an error * it reports. Decided on the server rather than left to its retention rule: @@ -37,11 +35,10 @@ public final class MetricsCollectorElement extends Component { * either, since the browser holds its buffer in {@code sessionStorage} * across a reload and an outage. */ - private static final String DETAILS_PRELUDE = "window.__vaadinMicrometerDetails=%s;"; + private static final String DETAILS_ATTRIBUTE = "details"; private final transient ClientMetricsBinder binder; private final transient ClientRateLimiter limiter; - private final boolean collectErrorMessages; /** * @deprecated use @@ -66,10 +63,10 @@ public MetricsCollectorElement(ClientMetricsBinder binder, public MetricsCollectorElement(ClientMetricsBinder binder, ObservabilitySettings settings, boolean collectErrorMessages) { this.binder = binder; - this.collectErrorMessages = collectErrorMessages; this.limiter = new ClientRateLimiter( settings.getClientRatePerSession()); getElement().getStyle().set("display", "none"); + getElement().setAttribute(DETAILS_ATTRIBUTE, collectErrorMessages); addDetachListener(event -> { UI ui = event.getUI(); if (ui != null && !ui.isClosing()) { @@ -78,13 +75,6 @@ public MetricsCollectorElement(ClientMetricsBinder binder, }); } - @Override - protected void onAttach(AttachEvent event) { - ClientResourceLoader.loadOnce(event.getUI(), CLIENT_INIT_KEY, - CLIENT_RESOURCE, MetricsCollectorElement.class, - DETAILS_PRELUDE.formatted(collectErrorMessages)); - } - @ClientCallable public void recordSamples(List samples) { if (binder == null || samples == null || samples.isEmpty()) { diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/devtools/ObservabilityDevToolsHandler.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/devtools/ObservabilityDevToolsHandler.java index 88847d72..59a615b5 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/devtools/ObservabilityDevToolsHandler.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/devtools/ObservabilityDevToolsHandler.java @@ -27,6 +27,7 @@ import com.vaadin.base.devserver.DevToolsInterface; import com.vaadin.base.devserver.DevToolsMessageHandler; +import com.vaadin.flow.component.dependency.JsModule; import com.vaadin.observability.micrometer.ObservabilityKit; import com.vaadin.observability.micrometer.insights.InsightsService; @@ -46,7 +47,11 @@ * meter table is only worth polling while the panel is open, whereas the panel * watches for new insights whether or not anyone is looking at it, and that * background poll should not snapshot the whole registry every time. + *

+ * The panel itself is the development-only module declared here, which the + * frontend build picks up because this class is a dev-tools entry point. */ +@JsModule(value = "./VaadinObservabilityDevTools.js", developmentOnly = true) public class ObservabilityDevToolsHandler implements DevToolsMessageHandler { static final String COMMAND_REFRESH = "observability-kit-refresh"; @@ -82,6 +87,9 @@ public boolean handleMessage(String command, JsonNode data, private void sendSnapshot(DevToolsInterface devToolsInterface) { Map payload = new LinkedHashMap<>(); payload.put("timestamp", System.currentTimeMillis()); + // The panel is shown whether or not the kit is licensed; this is what + // lets it say why its sections are empty. + payload.put("licensed", !ObservabilityKit.isLicenseMissing()); payload.put("meters", snapshot()); devToolsInterface.send(COMMAND_METRICS, payload); } diff --git a/observability-kit-micrometer/src/main/resources/META-INF/frontend/VaadinMetricsClient.js b/observability-kit-micrometer/src/main/resources/META-INF/frontend/VaadinMetricsClient.js index eeafd9de..79a87095 100644 --- a/observability-kit-micrometer/src/main/resources/META-INF/frontend/VaadinMetricsClient.js +++ b/observability-kit-micrometer/src/main/resources/META-INF/frontend/VaadinMetricsClient.js @@ -1,16 +1,19 @@ // Copyright 2000-2026 Vaadin Ltd. // Licensed under the Vaadin Commercial License and Service Terms. // -// In-browser collector for observability-kit. Injected per UI by -// MetricsCollectorElement via Page.executeJs. The IIFE is idempotent so the -// re-attach path does not double-install hooks. -(function () { +// In-browser collector for observability-kit, bundled through the @JsModule +// on MetricsCollectorElement. Loading the module only defines the +// element; nothing is collected until the server +// attaches one, which it does only when client metrics are enabled. The +// install is idempotent so the re-attach path does not double-install hooks. +var COLLECTOR_TAG = 'vaadin-metrics-collector'; + +function installCollector() { if (window.__vaadinMicrometerInstalled) { return; } window.__vaadinMicrometerInstalled = true; - var COLLECTOR_TAG = 'vaadin-metrics-collector'; var BUFFER_MAX = 200; var FLUSH_INTERVAL_MS = 5000; // How long a handed-over batch may go unanswered before a flush takes it @@ -473,15 +476,17 @@ return text.length > DETAIL_MAX ? text.slice(0, DETAIL_MAX) : text; } - // Whether the application asked for error messages to be collected. Set by - // the server ahead of this script. The message is gathered only when it is - // on, rather than gathered and discarded later: a browser error message can - // quote whatever the page was working with, and buffering one puts it in - // sessionStorage and then on the wire, neither of which a server-side - // retention rule can undo. The server applies the same rule again, for the - // page that was already open when the setting changed. + // Whether the application asked for error messages to be collected, which the + // server says with the `details` attribute on the collector element. The + // message is gathered only when it is on, rather than gathered and discarded + // later: a browser error message can quote whatever the page was working + // with, and buffering one puts it in sessionStorage and then on the wire, + // neither of which a server-side retention rule can undo. The server applies + // the same rule again, for the page that was already open when the setting + // changed. function detailsEnabled() { - return window.__vaadinMicrometerDetails === true; + var el = document.querySelector(COLLECTOR_TAG); + return !!el && el.hasAttribute('details'); } @@ -1269,4 +1274,15 @@ return buffer.length; } }; -})(); +} + +if (!customElements.get(COLLECTOR_TAG)) { + customElements.define( + COLLECTOR_TAG, + class extends HTMLElement { + connectedCallback() { + installCollector(); + } + } + ); +} diff --git a/observability-kit-micrometer/src/main/resources/META-INF/frontend/VaadinObservabilityDevTools.js b/observability-kit-micrometer/src/main/resources/META-INF/frontend/VaadinObservabilityDevTools.js index 91020974..384294e6 100644 --- a/observability-kit-micrometer/src/main/resources/META-INF/frontend/VaadinObservabilityDevTools.js +++ b/observability-kit-micrometer/src/main/resources/META-INF/frontend/VaadinObservabilityDevTools.js @@ -1,8 +1,8 @@ // Copyright 2000-2026 Vaadin Ltd. // Licensed under the Vaadin Commercial License and Service Terms. // -// Dev-mode Vaadin Copilot panel for observability-kit. Injected per UI by -// ObservabilityDevToolsClient via Page.executeJs (development mode only). +// Dev-mode Vaadin Copilot panel for observability-kit, bundled through the +// development-only @JsModule on ObservabilityDevToolsHandler. // Registers a Copilot plugin with two sections: the insights the server built // from the retained interactions, queries and browser errors, ranked so the // worst is first; and below them, collapsed, the live vaadin.* Micrometer @@ -23,13 +23,7 @@ // listening for until the log panel opens - and the log panel is usually not // open either. // -// The IIFE is idempotent so repeated injection does not re-register the plugin. -// -// NOTE: this file is injected through ClientResourceLoader, which strips its -// comments with a parser that is not a JavaScript parser: a double slash with -// code after it on the same line deletes the rest of that line. Hence no regex -// literals and no URLs in string literals here. ClientResourceIntegrityTest -// enforces it. +// The IIFE is idempotent so a second load does not re-register the plugin. (function () { if (window.__vaadinObservabilityDevToolsInstalled) { return; @@ -996,6 +990,13 @@ return true; } + // Whether the server said the kit has no license, which it says on the meter + // snapshot: the insights payload is the actuator's, unaltered. The panel is + // shown either way, with its sections empty and a notice saying why. + function unlicensed() { + return !!latest && latest.licensed === false; + } + var ticks = 0; // Insights are polled whether or not the panel is open - that is what makes @@ -1115,16 +1116,31 @@ if (!this._insightsEl) { this.innerHTML = '

' + + '
' + '
' + '
' + '
'; + this._licenseEl = this.querySelector('[data-region="license"]'); this._insightsEl = this.querySelector('[data-region="insights"]'); this._metersEl = this.querySelector('[data-region="metrics"]'); } + this.renderLicense(); this.renderInsights(); this.renderMeters(); } + renderLicense() { + this._licenseEl.innerHTML = unlicensed() + ? '
' + + 'Observability Kit needs a license. Without one, no metrics or ' + + 'insights are collected. See ' + + 'vaadin.com/commercial-license-and-service-terms.' + + '
' + : ''; + } + renderInsights(force) { var insights = (latestInsights && latestInsights.insights) || []; var instrumentation = latestInsights && latestInsights.instrumentation; @@ -1161,6 +1177,7 @@ // all, and hiding one changes it without the server being involved. var signature = JSON.stringify([ !!latestInsights, + unlicensed(), instrumentation, this._ranked, Object.keys(expanded).sort(), @@ -1204,6 +1221,13 @@ '
' + 'Waiting for the first snapshot…' + '
'; + } else if (unlicensed()) { + // Not "no problems detected", and not the insights setting either: + // the notice above says why nothing is here. + body = + '
' + + 'Not collected without a license.' + + '
'; } else if (quiet.length > 0) { // Findings exist; none is being counted. Saying "no problems // detected" here would be a claim the panel's own fold contradicts. @@ -1257,7 +1281,9 @@ this._metersEl.innerHTML = header + '
' + - 'No Vaadin meters yet. Interact with the application to generate metrics.' + + (unlicensed() + ? 'Not collected without a license.' + : 'No Vaadin meters yet. Interact with the application to generate metrics.') + '
'; return; } @@ -1305,37 +1331,41 @@ }); } + function addPanel() { + copilot.addPanel({ + header: 'Observability', + tag: PANEL_TAG, + // Plain HTMLElements don't self-position the way Copilot's BasePanel + // does, and the panel manager skips viewport adjustment when no + // position is set - so it would open off-screen. Give it an explicit + // on-screen position and size. + position: { + top: 80, + left: 80, + width: 720, + height: 460 + }, + toolbarOptions: { + iconKey: 'barChart', + // The toolbar only renders an icon for panels mapped to an active + // mode; 'common' alone gives no entry point. 'play' hides the panel + // container, so expose the icon in the remaining modes. + allowedModesWithOrder: { + edit: 100, + inspect: 100, + test: 100 + } + } + }); + } + var plugin = { init: function (copilotInterface) { copilot = copilotInterface; listen(); poll(); setInterval(poll, REFRESH_INTERVAL_MS); - copilotInterface.addPanel({ - header: 'Observability', - tag: PANEL_TAG, - // Plain HTMLElements don't self-position the way Copilot's BasePanel - // does, and the panel manager skips viewport adjustment when no - // position is set - so it would open off-screen. Give it an explicit - // on-screen position and size. - position: { - top: 80, - left: 80, - width: 720, - height: 460 - }, - toolbarOptions: { - iconKey: 'barChart', - // The toolbar only renders an icon for panels mapped to an active - // mode; 'common' alone gives no entry point. 'play' hides the panel - // container, so expose the icon in the remaining modes. - allowedModesWithOrder: { - edit: 100, - inspect: 100, - test: 100 - } - } - }); + addPanel(); } }; diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ClientResourceIntegrityTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ClientResourceIntegrityTest.java deleted file mode 100644 index 33fa2ca9..00000000 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ClientResourceIntegrityTest.java +++ /dev/null @@ -1,197 +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 java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.List; -import java.util.concurrent.TimeUnit; - -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Assumptions; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - -import com.vaadin.flow.internal.StringUtil; - -/** - * What survives {@link ClientResourceLoader}'s comment stripping. - *

- * The loader does not inject the client script as written: it runs - * {@link StringUtil#removeComments} over it first, and that parser is not a - * JavaScript parser. A double slash inside a regex literal reads to it as the - * start of a line comment, so it deletes the rest of that line — and since a - * regex literal for a URL scheme ends in an escaped slash followed by the - * closing delimiter, writing one silently truncated the whole file and the - * collector never installed. No unit test noticed, because every unit test - * reads the file directly; the only thing that failed was an IT, asserting that - * some {@code vaadin.client.*} meter had arrived. - *

- * So this checks the artefact that actually reaches the browser rather than the - * source, and the check that settles it is {@code node --check} over the - * stripped text: a truncated line usually leaves a syntax error, which counting - * brackets and looking for function declarations can only approximate — a regex - * or template literal holding an unbalanced bracket passes those and still - * breaks the injected script. The cheaper checks are kept because they say - * which declaration went missing, and because they run on a machine - * with no node. - *

- * Both scripts the loader injects are checked, not just the collector: the - * dev-tools panel goes through the same stripping, and its failure mode is the - * quieter one — a truncated panel script leaves the developer with no panel and - * no meter or insight to notice missing. - */ -class ClientResourceIntegrityTest { - - private static final String RESOURCE = "META-INF/frontend/VaadinMetricsClient.js"; - - private static final String PANEL_RESOURCE = "META-INF/frontend/VaadinObservabilityDevTools.js"; - - /** - * The functions the collector is built out of. Named explicitly rather than - * counted, so that a line disappearing says which one went with it. - */ - private static final List FUNCTIONS = List.of("monotonicNow", - "connectionStore", "isLoading", "normalizeState", "isOfflineState", - "offline", "offlineElapsed", "bufferedMs", "pushSample", - "currentRoute", "persist", "restore", "priority", "priorityFirst", - "makeRoom", "flush", "settle", "detailText", "detailsEnabled", - "hasScheme", "numberStart", "isLocation", "hasLineAndColumn", - "separatorIn", "partOfPath", "hasUserInfo", "parseFrame", - "firstFrame", "errorDetail"); - - private static String source(String resource) throws IOException { - try (InputStream in = ClientResourceIntegrityTest.class.getClassLoader() - .getResourceAsStream(resource)) { - Assertions.assertNotNull(in, resource + " is missing"); - return new String(in.readAllBytes(), StandardCharsets.UTF_8); - } - } - - @Test - void everyFunctionSurvivesTheCommentStripping() throws IOException { - String source = source(RESOURCE); - String injected = StringUtil.removeComments(source, true); - - for (String function : FUNCTIONS) { - Assertions.assertTrue(source.contains("function " + function + "("), - () -> "this test is out of date: the collector no longer " - + "declares " + function); - Assertions.assertTrue( - injected.contains("function " + function + "("), - () -> "comment stripping ate the declaration of " + function - + " -- something on its line reads as " - + "the start of a comment"); - } - } - - /** - * The check the others approximate: hand the stripped text to a JavaScript - * parser and see whether it is still JavaScript. - *

- * Skipped rather than failed where node is missing, the way the browser - * suites are with {@code -Dskip.js.tests} — this is the same tool, and a - * machine without it should not fail a build over the one check that needs - * it. - */ - @ParameterizedTest - @ValueSource(strings = { RESOURCE, PANEL_RESOURCE }) - void theStrippedScriptStillParses(String resource) throws Exception { - Assumptions.assumeTrue(nodeIsAvailable(), "node is not on PATH"); - - Path file = Files.createTempFile("observability-kit-stripped-", ".js"); - try { - Files.writeString(file, - StringUtil.removeComments(source(resource), true), - StandardCharsets.UTF_8); - Process node = new ProcessBuilder("node", "--check", - file.toAbsolutePath().toString()).redirectErrorStream(true) - .start(); - String output = new String(node.getInputStream().readAllBytes(), - StandardCharsets.UTF_8); - Assertions.assertTrue(node.waitFor(60, TimeUnit.SECONDS), - "node --check did not finish"); - Assertions.assertEquals(0, node.exitValue(), - () -> resource + " does not parse after comment stripping, " - + "so a line was eaten:\n" + output); - } finally { - Files.deleteIfExists(file); - } - } - - private static boolean nodeIsAvailable() { - try { - Process node = new ProcessBuilder("node", "--version") - .redirectErrorStream(true).start(); - return node.waitFor(60, TimeUnit.SECONDS) && node.exitValue() == 0; - } catch (IOException e) { - return false; - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - return false; - } - } - - /** - * The collector only. Counting brackets assumes none of them sits unmatched - * inside a string literal, which holds for the collector and does not for - * the panel — its route-template parsing looks for the literal - * {@code "?("}, and the count is off by one per such string. That is the - * limit of this check rather than a fault in the script, and - * {@link #theStrippedScriptStillParses} is what covers the panel properly. - */ - @Test - void bracketsStillBalanceAfterTheCommentStripping() throws IOException { - String injected = StringUtil.removeComments(source(RESOURCE), true); - - // A crude parser, and enough for a file that keeps to the rule: what a - // swallowed line does is leave an opening bracket without its partner. - for (char[] pair : new char[][] { { '{', '}' }, { '(', ')' }, - { '[', ']' } }) { - long open = injected.chars().filter(c -> c == pair[0]).count(); - long close = injected.chars().filter(c -> c == pair[1]).count(); - Assertions.assertEquals(open, close, - () -> "unbalanced " + pair[0] + pair[1] - + " after comment stripping, so a line was eaten"); - } - } - - @ParameterizedTest - @ValueSource(strings = { RESOURCE, PANEL_RESOURCE }) - void theSourceHoldsNoDoubleSlashOutsideAComment(String resource) - throws IOException { - // The rule that keeps the above true, stated where it can be checked: - // a double slash anywhere but at the start of a comment is a line the - // stripper will truncate. This is why hasScheme is a character scan - // and not a pattern, and why neither script writes a URL out in full. - int line = 0; - for (String text : source(resource).split("\n")) { - line++; - String trimmed = text.strip(); - if (trimmed.startsWith("//")) { - continue; - } - int at = text.indexOf("//"); - if (at < 0) { - continue; - } - // A trailing comment is fine; what is not is a double slash with - // code after it on the same line. - String after = text.substring(at + 2); - int number = line; - Assertions.assertFalse(after.contains(";") || after.contains("{"), - () -> "line " + number + " has code after a double slash, " - + "which comment stripping will delete: " + text); - } - } -} diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MetricsServiceInitListenerLicenseTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MetricsServiceInitListenerLicenseTest.java index 8c143594..cf6ac7e6 100644 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MetricsServiceInitListenerLicenseTest.java +++ b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MetricsServiceInitListenerLicenseTest.java @@ -10,6 +10,7 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -68,28 +69,31 @@ void developmentMode_withoutValidLicense_skipsInstrumentation() { verify(service, never()).addSessionInitListener(any()); verify(service, never()).addUIInitListener(any()); verify(event, never()).addVaadinRequestInterceptor(any()); + // Recorded so the Copilot panel can say why it has nothing to show. + Assertions.assertTrue(ObservabilityKit.isLicenseMissing()); } @Test void developmentMode_withValidLicense_registersInstrumentation() { when(service.getDeploymentConfiguration().isProductionMode()) .thenReturn(false); + // Left over from an earlier start without a license + ObservabilityKit.setLicenseMissing(true); // An unstubbed static checkLicense is a no-op, i.e. a valid license try (var licenseChecker = mockStatic(LicenseChecker.class)) { new MetricsServiceInitListener().serviceInit(event); } - // Three UI init listeners in development mode: the UiMetricsBinder, - // the ErrorMetricsBinder (which re-instruments the session error - // handler), and the dev-tools Copilot panel injector (the last is - // skipped in production - see - // productionMode_registersWithoutCheckingLicense). - verify(service, times(3)).addUIInitListener(any(UIInitListener.class)); + // Two UI init listeners: the UiMetricsBinder and the ErrorMetricsBinder + // (which re-instruments the session error handler). + verify(service, times(2)).addUIInitListener(any(UIInitListener.class)); // Two request interceptors: request timing/errors, and the navigation // binder closing out navigations that never complete. verify(event, times(2)).addVaadinRequestInterceptor( any(VaadinRequestInterceptor.class)); + // Cleared, or the Copilot panel would keep asking for a license + Assertions.assertFalse(ObservabilityKit.isLicenseMissing()); } @Test @@ -104,8 +108,7 @@ void productionMode_registersWithoutCheckingLicense() { licenseChecker.verifyNoInteractions(); } - // The UiMetricsBinder and the ErrorMetricsBinder; no dev-tools - // injector in production mode. + // The UiMetricsBinder and the ErrorMetricsBinder. verify(service, times(2)).addUIInitListener(any(UIInitListener.class)); } diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ObservabilityDevToolsHandlerTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ObservabilityDevToolsHandlerTest.java index 0131044f..ce75c2ff 100644 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ObservabilityDevToolsHandlerTest.java +++ b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/ObservabilityDevToolsHandlerTest.java @@ -163,6 +163,22 @@ void noBuffersBound_saysInstrumentationIsInactive() { devTools.payloads.get(COMMAND_METRICS).get("meters")); } + @Test + void licenseMissing_stillAnswersAndSaysSo() { + ObservabilityKit.setLicenseMissing(true); + + handler.handleConnect(devTools); + + // The panel is shown either way; the flag is what lets it explain + // its empty sections instead of looking like an idle application. + Assertions.assertEquals(List.of(COMMAND_METRICS, COMMAND_INSIGHTS_DATA), + devTools.commands); + Assertions.assertEquals(false, + devTools.payloads.get(COMMAND_METRICS).get("licensed")); + Assertions.assertEquals(List.of(), + devTools.payloads.get(COMMAND_METRICS).get("meters")); + } + @Test void announce_isNotRelayedThroughTheServer() { // The panel writes its own log line on Copilot's event bus. A 'log' diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/UiLifecycleTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/UiLifecycleTest.java index 166cb8a6..6f64b13e 100644 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/UiLifecycleTest.java +++ b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/UiLifecycleTest.java @@ -12,6 +12,8 @@ import io.micrometer.observation.ObservationRegistry; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.mockito.ArgumentCaptor; import com.vaadin.flow.component.Component; @@ -21,6 +23,8 @@ import com.vaadin.flow.server.UIInitEvent; import com.vaadin.flow.server.VaadinService; import com.vaadin.observability.micrometer.client.MetricsCollectorElement; +import com.vaadin.observability.micrometer.insights.ClientErrorCollector; +import com.vaadin.observability.micrometer.insights.RecentClientErrors; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; @@ -185,6 +189,34 @@ void uiInitAddsMetricsCollectorElementWhenClientEnabled() { assertEquals(MetricsCollectorElement.class, added[0].getClass()); } + /** + * The browser is told to gather error messages only when the application + * asked for them and something on the server will retain them; otherwise a + * message nothing keeps would still be buffered in the tab and posted. + */ + @ParameterizedTest + @CsvSource({ "true, true, true", "true, false, false", + "false, true, false" }) + void metricsCollectorElementCarriesDetailsFlag(boolean insightsDetails, + boolean retainsErrors, boolean expectedDetails) { + ObservabilitySettings settings = ObservabilitySettings.builder() + .insightsDetails(insightsDetails).build(); + UiMetricsBinder b = new UiMetricsBinder(new SimpleMeterRegistry(), null, + settings, + retainsErrors + ? new ClientErrorCollector(new RecentClientErrors(1), + settings) + : null); + UI ui = mock(UI.class); + b.uiInit(new UIInitEvent(ui, mock(VaadinService.class))); + + ArgumentCaptor captor = ArgumentCaptor + .forClass(Component[].class); + verify(ui).add(captor.capture()); + assertEquals(expectedDetails, + captor.getValue()[0].getElement().hasAttribute("details")); + } + @Test void uiInitDoesNotAddMetricsCollectorElementWhenClientDisabled() { UiMetricsBinder b = new UiMetricsBinder(new SimpleMeterRegistry(), null, diff --git a/observability-kit-micrometer/src/test/js/VaadinMetricsClient.test.js b/observability-kit-micrometer/src/test/js/VaadinMetricsClient.test.js index 51b6061b..b7bc81b2 100644 --- a/observability-kit-micrometer/src/test/js/VaadinMetricsClient.test.js +++ b/observability-kit-micrometer/src/test/js/VaadinMetricsClient.test.js @@ -23,6 +23,15 @@ const src = fs.readFileSync( 'utf8' ); +// Loading the module only defines the collector element; the stand-in for the +// browser connects one as soon as it is defined, which is what attaching it +// from the server does. +global.HTMLElement = class {}; +global.customElements = { + get: () => undefined, + define: (tag, type) => new type().connectedCallback() +}; + // The frame corpus, read from the same file StackFramesTest reads, so that // "the two copies of this rule agree" is enforced rather than asserted. const CORPUS = path.join(__dirname, '../resources/stack-frames-corpus.tsv'); @@ -42,15 +51,17 @@ const corpus = fs return { verdict: columns[0], line: columns[1], location: columns[2] }; }); -// parseFrame and isLocation live inside the IIFE; reach them by evaluating the -// body with a probe appended, in a throwaway environment separate from the one -// below. Both are needed: parseFrame is the stack-line rule, isLocation the -// rule applied to `source` and to a bare `frame`, and the corpus has rows that -// only one of the two decides. +// parseFrame and isLocation live inside installCollector; reach them by +// evaluating its body, after the declarations above it, with a probe appended, +// in a throwaway environment separate from the one below. Both are needed: +// parseFrame is the stack-line rule, isLocation the rule applied to `source` +// and to a bare `frame`, and the corpus has rows that only one of the two +// decides. function frameRules() { - const open = src.indexOf('(function () {'); - const close = src.lastIndexOf('})();'); - const body = src.slice(open + '(function () {'.length, close); + const head = src.slice(0, src.indexOf('function installCollector() {')); + const open = src.indexOf('function installCollector() {'); + const close = src.indexOf('\n}\n\nif (!customElements'); + const body = head + src.slice(open + 'function installCollector() {'.length, close); return new Function( 'window', 'document', @@ -84,7 +95,9 @@ const store = { }; let sent = []; -const collector = { $server: { recordSamples: (batch) => { sent.push(batch); return Promise.resolve(); } } }; +// `details` stands for the attribute the server sets when error messages are +// to be gathered. +const collector = { details: true, hasAttribute(name) { return name === 'details' && this.details; }, $server: { recordSamples: (batch) => { sent.push(batch); return Promise.resolve(); } } }; global.window = { addEventListener: add, @@ -93,8 +106,7 @@ global.window = { // exactly what must not be published. location: { pathname: '/orders/17', href: 'https://app.example.com/orders/17?token=abc123' }, sessionStorage: { store: {}, getItem(k) { return this.store[k] || null; }, setItem(k, v) { this.store[k] = v; }, removeItem(k) { delete this.store[k]; } }, - Vaadin: { connectionState: store }, - __vaadinMicrometerDetails: true + Vaadin: { connectionState: store } }; global.document = { querySelector: (s) => (s === 'vaadin-metrics-collector' ? collector : null), addEventListener: add, visibilityState: 'visible' }; global.performance = { getEntriesByType: () => [], now: () => clock }; @@ -144,8 +156,7 @@ function freshCollector(stored) { getItem(k) { return this.store[k] === undefined ? null : this.store[k]; }, setItem(k, v) { this.store[k] = v; }, removeItem(k) { delete this.store[k]; } - }, - __vaadinMicrometerDetails: false + } }; new Function('window', 'document', 'performance', 'PerformanceObserver', 'history', 'setInterval', 'requestAnimationFrame', src)( win, @@ -250,18 +261,18 @@ function err(message, stack) { // 5b'''. The function name travels separately, and only under the gate. { const stack = 'Error: boom\n at handleCardNumber4111 (chart.js:44:13)'; - global.window.__vaadinMicrometerDetails = true; + collector.details = true; fire('error', { message: 'boom', filename: '', lineno: 0, error: err('boom', stack) }); let one = (await recoverAndFlush())[0]; check('the location is the frame', one.detail.frame, 'chart.js:44:13'); check('the name is its own field when detail is on', one.detail.function, 'handleCardNumber4111'); - global.window.__vaadinMicrometerDetails = false; + collector.details = false; fire('error', { message: 'boom', filename: '', lineno: 0, error: err('boom', stack) }); one = (await recoverAndFlush())[0]; check('the location is published either way', one.detail.frame, 'chart.js:44:13'); check('the name is not gathered when detail is off', one.detail.function, undefined); - global.window.__vaadinMicrometerDetails = true; + collector.details = true; } // 5b'. Separators that are whitespace to one engine and not the other. @@ -421,7 +432,7 @@ function err(message, stack) { } // 7. The message gate, and what it keeps out of sessionStorage. - global.window.__vaadinMicrometerDetails = false; + collector.details = false; store.go('connection-lost'); fire('error', { message: 'secret payload', filename: '/app.js', lineno: 1, error: err('secret payload') }); check('nothing sensitive reaches sessionStorage while offline', @@ -456,12 +467,11 @@ function err(message, stack) { Vaadin: { connectionState: cs, Flow: { clients: { app: { getProfilingData: () => [flow.last, flow.total, -1, -1, 0] } } } - }, - __vaadinMicrometerDetails: false + } }; const doc = { querySelector: (sel) => (sel === 'vaadin-metrics-collector' - ? { $server: { recordSamples: (batch) => { batches.push(batch); return promiseless ? undefined : new Promise((r) => answers.push(r)); } } } + ? { hasAttribute: () => false, $server: { recordSamples: (batch) => { batches.push(batch); return promiseless ? undefined : new Promise((r) => answers.push(r)); } } } : null), addEventListener() {}, visibilityState: 'visible' @@ -753,5 +763,47 @@ function err(message, stack) { timing(all), [['request', '/orders/17', 180]]); } + // 9. Loading the module is not attaching the element. The module is in the + // bundle of every application with the kit, client metrics or not, so + // only the element the server attaches may start the collector; and a + // module evaluated a second time must not define the element again. + { + let defined = null; + let defines = 0; + const registry = { + get: (tag) => (tag === 'vaadin-metrics-collector' ? defined : undefined), + define: (tag, type) => { defines++; defined = type; } + }; + const win = { + handlers: {}, + addEventListener(name, cb) { (this.handlers[name] = this.handlers[name] || []).push(cb); }, + location: { pathname: '/x', href: 'https://app.example.com/x' }, + sessionStorage: { store: {}, getItem(k) { return this.store[k] === undefined ? null : this.store[k]; }, setItem(k, v) { this.store[k] = v; }, removeItem(k) { delete this.store[k]; } } + }; + const load = () => new Function('window', 'document', 'performance', 'PerformanceObserver', 'history', 'setInterval', 'requestAnimationFrame', 'customElements', 'HTMLElement', src)( + win, + { querySelector: () => null, addEventListener() {}, visibilityState: 'visible' }, + { getEntriesByType: () => [], now: () => 0 }, + function () { throw new Error('unsupported'); }, + {}, + () => 0, + () => 0, + registry, + class {} + ); + + load(); + check('loading the module defines the element', defines, 1); + check('but installs nothing', win.__vaadinMicrometerInstalled, undefined); + check('and listens to nothing', Object.keys(win.handlers), []); + + load(); + check('a second load does not define the element again', defines, 1); + + new defined().connectedCallback(); + check('attaching the element installs the collector', win.__vaadinMicrometerInstalled, true); + check('which then listens for errors', (win.handlers.error || []).length > 0, true); + } + process.exit(failures === 0 ? 0 : 1); })(); diff --git a/observability-kit-micrometer/src/test/js/VaadinObservabilityDevTools.test.js b/observability-kit-micrometer/src/test/js/VaadinObservabilityDevTools.test.js index f126db50..6633897f 100644 --- a/observability-kit-micrometer/src/test/js/VaadinObservabilityDevTools.test.js +++ b/observability-kit-micrometer/src/test/js/VaadinObservabilityDevTools.test.js @@ -158,6 +158,7 @@ function harness(pathname) { const logged = []; const logListener = (event) => logged.push(event.detail); + let panelsAdded = 0; const copilot = { _uiState: {}, plugins: [], @@ -165,7 +166,9 @@ function harness(pathname) { send: (command, data) => { sent.push({ command, data }); }, - addPanel: () => {} + addPanel: () => { + panelsAdded++; + } }; const win = { location: { pathname: pathname }, Vaadin: { copilot } }; @@ -247,6 +250,7 @@ function harness(pathname) { metricsHtml, clickIn, commands: () => sent.map((message) => message.command), + panelsAdded: () => panelsAdded, unclaimed: () => unclaimed }; } @@ -528,6 +532,25 @@ check('every server message was claimed on the event bus', app.unclaimed(), 0); check('a later payload leaves the fold alone', waiting.metricsHtml().includes('▸'), true); } +// 14b. The panel is offered to Copilot whether or not the kit is licensed; +// without a license it says why its sections are empty rather than looking +// like an idle application, or like one with the insights setting off. +{ + const bare = harness('/orders/17'); + check('the panel is added without waiting for the server', bare.panelsAdded(), 1); + bare.open(); + bare.meters({ timestamp: Date.now(), licensed: false, meters: [] }); + bare.insights(payload([], 'inactive')); + check('an unlicensed kit shows the license notice', bare.panel.regions['[data-region="license"]'].innerHTML.includes('needs a license'), true); + check('and does not blame the insights setting', bare.insightsHtml().includes('vaadin.observability.insights'), false); + check('nor invite interaction to generate meters', bare.metricsHtml().includes('Interact with the application'), false); + + const licensed = harness('/orders/17'); + licensed.open(); + licensed.meters({ timestamp: Date.now(), licensed: true, meters: [] }); + check('a licensed kit shows no notice', licensed.panel.regions['[data-region="license"]'].innerHTML.includes('needs a license'), false); +} + // 15. A typed parameter carries its regex after the modifier, which is where // Flow writes it. Reading the modifier off the last character instead makes // this a required single segment, and no group is current.