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 04bca07..36b1501 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 @@ -44,6 +44,20 @@ public final class MeterNames { public static final String TAG_ROUTE = "route"; public static final String TAG_OUTCOME = "outcome"; public static final String TAG_EXCEPTION = "exception"; + + /** + * Tag key: simple class name of the exception that ended the operation, or + * {@link #ERROR_NONE} when it raised none. This mirrors the tag that + * {@code DefaultMeterObservationHandler} adds by itself on the Observation + * path; the binders add it explicitly on their direct-recording path so + * both paths publish the same tag-key set. Distinct from + * {@link #TAG_EXCEPTION}, which tags the {@link #ERRORS} counter. + */ + public static final String TAG_ERROR = "error"; + + /** {@link #TAG_ERROR} value for an operation that raised no exception. */ + public static final String ERROR_NONE = "none"; + public static final String TAG_TRIGGER = "trigger"; public static final String TAG_KIND = "kind"; public static final String TAG_CONTEXT = "context"; diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/NavigationMetricsBinder.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/NavigationMetricsBinder.java index 5f55765..9844756 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/NavigationMetricsBinder.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/NavigationMetricsBinder.java @@ -30,6 +30,14 @@ * {@code DefaultMeterObservationHandler}, the Timer). Otherwise the binder * falls back to direct Timer recording. Per-UI state is stored as a UI * attribute so concurrent UIs are tracked independently. + *

+ * Both paths publish {@link MeterNames#NAVIGATION} with the same tag keys: + * {@code route}, {@code outcome} and {@code error}. A navigation that throws + * never reaches {@code afterNavigation}, so no sample is recorded for it on + * either path and {@code error} is always {@link MeterNames#ERROR_NONE} — the + * key is still emitted because {@code DefaultMeterObservationHandler} emits it + * on the Observation path, and a metrics backend such as Prometheus rejects + * same-named meters whose tag-key sets differ. */ final class NavigationMetricsBinder implements BeforeEnterListener, AfterNavigationListener { @@ -109,7 +117,8 @@ public void afterNavigation(AfterNavigationEvent event) { if (sample instanceof Timer.Sample s) { s.stop(registry.timer(MeterNames.NAVIGATION, MeterNames.TAG_ROUTE, route instanceof String r ? r : MeterNames.ROUTE_UNKNOWN, - MeterNames.TAG_OUTCOME, MeterNames.OUTCOME_SUCCESS)); + MeterNames.TAG_OUTCOME, MeterNames.OUTCOME_SUCCESS, + MeterNames.TAG_ERROR, MeterNames.ERROR_NONE)); } if (scopeObj instanceof Observation.Scope scope) { scope.close(); diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/RequestMetricsBinder.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/RequestMetricsBinder.java index b505e49..8ba80ea 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/RequestMetricsBinder.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/RequestMetricsBinder.java @@ -37,6 +37,20 @@ *

  • Otherwise (no obs registry / traces disabled / observation handler * unavailable), the binder falls back to recording the Timer directly.
    • * + *

    + * Both paths publish {@link MeterNames#REQUEST_DURATION} with the same tag + * keys, all bounded: {@code vaadin.request.type}, {@code vaadin.interaction}, + * {@code http.method}, {@code outcome} and {@code error}. Keeping the two in + * step matters because a metrics backend such as Prometheus rejects same-named + * meters whose tag-key sets differ, and dashboards must not have to know which + * path recorded a sample. The {@code error} tag is the one + * {@code DefaultMeterObservationHandler} adds by itself on the Observation + * path, so the direct-recording path adds it explicitly. + *

    + * The UI id and the client location are attached as high-cardinality + * key-values, so they enrich the span without multiplying the Timer's time + * series: a UI id is unbounded over an application's lifetime, and the client + * location is deliberately kept un-templated. */ final class RequestMetricsBinder implements VaadinRequestInterceptor { @@ -51,6 +65,10 @@ final class RequestMetricsBinder implements VaadinRequestInterceptor { private final ThreadLocal sample = new ThreadLocal<>(); private final ThreadLocal errored = ThreadLocal .withInitial(() -> Boolean.FALSE); + // Simple class name of the exception passed to handleException, mirroring + // what DefaultMeterObservationHandler reads off the Observation context so + // the direct-recording path can tag its Timer the same way. + private final ThreadLocal errorType = new ThreadLocal<>(); private final ThreadLocal observation = new ThreadLocal<>(); private final ThreadLocal observationScope = new ThreadLocal<>(); @@ -88,6 +106,7 @@ public void requestStart(VaadinRequest request, VaadinResponse response) { // this a pooled thread could carry errored=TRUE into the next request // and misreport it as an error. errored.remove(); + errorType.remove(); sample.remove(); observation.remove(); observationScope.remove(); @@ -108,9 +127,12 @@ public void requestStart(VaadinRequest request, VaadinResponse response) { type) .lowCardinalityKeyValue(ObservationNames.KEY_HTTP_METHOD, httpMethod(request)) - .lowCardinalityKeyValue(ObservationNames.KEY_UI_ID, + // Span-only: the UI id is unbounded over an application's + // lifetime and the client location is un-templated, so + // neither may become a Timer tag. + .highCardinalityKeyValue(ObservationNames.KEY_UI_ID, uiId(request)) - .lowCardinalityKeyValue( + .highCardinalityKeyValue( ObservationNames.KEY_CLIENT_LOCATION, clientLocation(request)) // Always emit the interaction key so every @@ -146,11 +168,12 @@ private static String uiId(VaadinRequest request) { /** * Extracts the page path the UIDL request was sent from. Falls back to the - * Referer header path so we always emit something useful for dashboards - * filtering by view, without ever exposing PII (the path goes through the - * parent navigation observation's route template mapping in dashboards; we - * deliberately keep it un-templated here so the span captures the literal - * client path). + * Referer header path so we always emit something useful when reading a + * trace, without ever exposing PII. The path is deliberately kept + * un-templated so the span captures the literal client path; that is also + * why it is attached as a high-cardinality key-value and never as a Timer + * tag. For a templated, cardinality-capped view attribution use the + * {@code route} tag of the navigation meters instead. */ private static String clientLocation(VaadinRequest request) { if (request == null) { @@ -192,6 +215,9 @@ private static String clientLocation(VaadinRequest request) { public void handleException(VaadinRequest request, VaadinResponse response, VaadinSession vaadinSession, Exception t) { errored.set(Boolean.TRUE); + if (t != null) { + errorType.set(t.getClass().getSimpleName()); + } if (settings.isErrors() && t != null) { Counter.builder(MeterNames.ERRORS) .tag(MeterNames.TAG_EXCEPTION, t.getClass().getSimpleName()) @@ -208,14 +234,10 @@ public void requestEnd(VaadinRequest request, VaadinResponse response, VaadinSession session) { boolean wasError = errored.get(); errored.remove(); + String error = errorType.get(); + errorType.remove(); String outcome = wasError ? MeterNames.OUTCOME_ERROR : MeterNames.OUTCOME_SUCCESS; - Timer.Sample s = sample.get(); - sample.remove(); - if (s != null) { - s.stop(registry.timer(MeterNames.REQUEST_DURATION, - MeterNames.TAG_OUTCOME, outcome)); - } Observation.Scope scope = observationScope.get(); observationScope.remove(); if (scope != null) { @@ -227,11 +249,33 @@ public void requestEnd(VaadinRequest request, VaadinResponse response, // request so the span name reflects what actually happened instead // of the opaque protocol-level "uidl". String interaction = RequestInteraction.take(); + String type = requestType(request); + // Resolve the interaction once, for whichever path records: a UIDL + // request takes the listener's marker (defaulting to the generic + // "rpc"), anything else has no interaction to report. + String kind = ObservationNames.REQUEST_TYPE_UIDL.equals(type) + ? (interaction != null ? interaction + : ObservationNames.INTERACTION_RPC) + : ObservationNames.INTERACTION_NONE; + Timer.Sample s = sample.get(); + sample.remove(); + if (s != null) { + // Tag with the very constants the Observation path uses above, so + // the two paths cannot drift into publishing + // vaadin.request.duration under differing tag-key sets. The error + // tag replicates the one DefaultMeterObservationHandler adds for + // us there. + s.stop(Timer.builder(MeterNames.REQUEST_DURATION) + .tag(ObservationNames.KEY_REQUEST_TYPE, type) + .tag(ObservationNames.KEY_HTTP_METHOD, httpMethod(request)) + .tag(ObservationNames.KEY_INTERACTION, kind) + .tag(ObservationNames.KEY_OUTCOME, outcome) + .tag(MeterNames.TAG_ERROR, + error != null ? error : MeterNames.ERROR_NONE) + .register(registry)); + } if (obs != null) { - String type = requestType(request); if (ObservationNames.REQUEST_TYPE_UIDL.equals(type)) { - String kind = interaction != null ? interaction - : ObservationNames.INTERACTION_RPC; obs.lowCardinalityKeyValue(ObservationNames.KEY_INTERACTION, kind); obs.contextualName(ObservationNames.REQUEST + "." + kind); diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/RpcMetricsBinder.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/RpcMetricsBinder.java index 789701c..e7c3210 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/RpcMetricsBinder.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/RpcMetricsBinder.java @@ -41,10 +41,14 @@ * unavailable), the binder falls back to recording the Timer directly. * *

    - * Timer tags (low cardinality): {@code type} (RPC invocation type) and - * {@code outcome} ({@code success}/{@code error}). The invocation name and node - * ID are deliberately omitted from the Timer tags because they are - * high-cardinality. + * Timer tags (low cardinality), identical on both paths: {@code type} (RPC + * invocation type), {@code outcome} ({@code success}/{@code error}) and + * {@code error} (the failing exception's simple class name, or {@code none}) — + * the last of these added by {@code DefaultMeterObservationHandler} on the + * Observation path and explicitly on the direct-recording one, so neither + * publishes {@link MeterNames#RPC_DURATION} under a tag-key set the other + * lacks. The invocation name and node ID are deliberately omitted from the + * Timer tags because they are high-cardinality. *

    * When tracing is enabled, the span additionally carries the invocation name * ({@link ObservationNames#KEY_EVENT_NAME}) and the targeted component class @@ -60,6 +64,10 @@ final class RpcMetricsBinder implements RpcInvocationListener { private final ThreadLocal errored = ThreadLocal .withInitial(() -> Boolean.FALSE); + // Simple class name of the failing exception, mirroring what + // DefaultMeterObservationHandler reads off the Observation context so the + // direct-recording path can tag its Timer the same way. + private final ThreadLocal errorType = new ThreadLocal<>(); private final ThreadLocal sample = new ThreadLocal<>(); private final ThreadLocal observation = new ThreadLocal<>(); private final ThreadLocal observationScope = new ThreadLocal<>(); @@ -80,6 +88,7 @@ public void invocationStarted(RpcInvocationEvent event) { // server shutdown). Without this, a pooled thread could carry // errored=TRUE into the next invocation and misreport it. errored.remove(); + errorType.remove(); sample.remove(); observation.remove(); observationScope.remove(); @@ -151,6 +160,9 @@ private static Optional resolveComponentType( @Override public void invocationFailed(RpcInvocationEvent event, Throwable error) { errored.set(Boolean.TRUE); + if (error != null) { + errorType.set(error.getClass().getSimpleName()); + } Observation obs = observation.get(); if (obs != null && error != null) { obs.error(error); @@ -160,6 +172,7 @@ public void invocationFailed(RpcInvocationEvent event, Throwable error) { @Override public void invocationEnded(RpcInvocationEvent event) { boolean wasError = errored.get(); + String error = errorType.get(); String outcome = wasError ? MeterNames.OUTCOME_ERROR : MeterNames.OUTCOME_SUCCESS; String type = event.getType(); @@ -170,6 +183,7 @@ public void invocationEnded(RpcInvocationEvent event) { // Clear all thread-locals before any calls that could throw. errored.remove(); + errorType.remove(); sample.remove(); observationScope.remove(); observation.remove(); @@ -181,9 +195,15 @@ public void invocationEnded(RpcInvocationEvent event) { } obs.stop(); } else if (s != null) { + // The error tag replicates the one + // DefaultMeterObservationHandler adds for us on the Observation + // path, keeping both paths' tag-key sets identical. s.stop(Timer.builder(MeterNames.RPC_DURATION) .tag(MeterNames.TAG_TYPE, type) - .tag(MeterNames.TAG_OUTCOME, outcome).register(registry)); + .tag(MeterNames.TAG_OUTCOME, outcome) + .tag(MeterNames.TAG_ERROR, + error != null ? error : MeterNames.ERROR_NONE) + .register(registry)); } } } diff --git a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/trace/ObservationNames.java b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/trace/ObservationNames.java index bbed049..f58aacb 100644 --- a/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/trace/ObservationNames.java +++ b/observability-kit-micrometer/src/main/java/com/vaadin/observability/micrometer/trace/ObservationNames.java @@ -29,7 +29,20 @@ public final class ObservationNames { public static final String KEY_ROUTE = "route"; public static final String KEY_HTTP_METHOD = "http.method"; public static final String KEY_SESSION_ID = "vaadin.session.id"; + + /** + * High-cardinality span attribute: the id of the UI the request belongs to, + * or {@link #UI_ID_UNKNOWN}. Span-only; UI ids are unbounded over an + * application's lifetime, so this is never added as a Timer tag. + */ public static final String KEY_UI_ID = "ui.id"; + + /** + * High-cardinality span attribute: the literal, un-templated browser path + * the request was sent from, or {@link #LOCATION_UNKNOWN}. Span-only; use + * the {@link #KEY_ROUTE} tag of the navigation meters for templated, + * cardinality-capped view attribution. + */ public static final String KEY_CLIENT_LOCATION = "vaadin.client.location"; /** diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MeterTagParityTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MeterTagParityTest.java new file mode 100644 index 0000000..d18c97d --- /dev/null +++ b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/MeterTagParityTest.java @@ -0,0 +1,225 @@ +/** + * 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.util.Map; +import java.util.stream.Collectors; + +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import io.micrometer.observation.ObservationRegistry; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import com.vaadin.flow.component.Component; +import com.vaadin.flow.component.UI; +import com.vaadin.flow.router.AfterNavigationEvent; +import com.vaadin.flow.router.BeforeEnterEvent; +import com.vaadin.flow.server.VaadinRequest; +import com.vaadin.flow.server.VaadinResponse; +import com.vaadin.flow.server.VaadinSession; +import com.vaadin.flow.server.communication.RpcInvocationEvent; +import com.vaadin.observability.micrometer.trace.ObservationNames; + +/** + * Every binder can publish its Timer two ways: through the Observation API + * (where {@code DefaultMeterObservationHandler} builds the Timer) or by + * recording it directly. A metrics backend such as Prometheus rejects + * same-named meters whose tag-key sets differ, so the two paths must agree — + * and since the choice is fixed at binder construction, a difference between + * them is invisible in any single application and only shows up as an + * unqueryable dashboard elsewhere. These tests drive both paths over the same + * input and require the resulting tags to be identical, keys and values alike. + */ +class MeterTagParityTest { + + @AfterEach + void tearDown() { + UI.setCurrent(null); + // beforeEnter marks the enclosing request as a navigation; drop the + // marker so it cannot leak into another test on this thread. + RequestInteraction.clear(); + } + + private static ObservationRegistry meterProducingObservations( + SimpleMeterRegistry registry) { + ObservationRegistry observations = ObservationRegistry.create(); + observations.observationConfig().observationHandler( + new DefaultMeterObservationHandler(registry)); + return observations; + } + + private static Map tags(SimpleMeterRegistry registry, + String meter) { + Timer timer = registry.find(meter).timer(); + Assertions.assertNotNull(timer, meter + " should have been recorded"); + return timer.getId().getTags().stream() + .collect(Collectors.toMap(Tag::getKey, Tag::getValue)); + } + + private static VaadinRequest uidlRequest() { + VaadinRequest request = Mockito.mock(VaadinRequest.class); + Mockito.when(request.getParameter("v-r")).thenReturn("uidl"); + Mockito.when(request.getMethod()).thenReturn("POST"); + return request; + } + + /** + * Runs one request through the binder and returns the tags of the Timer it + * produced. {@code traces} picks the recording path. + */ + private static Map recordRequest(boolean traces, + Exception failure) { + SimpleMeterRegistry registry = new SimpleMeterRegistry(); + RequestMetricsBinder binder = traces + ? new RequestMetricsBinder(registry, + meterProducingObservations(registry), + ObservabilitySettings.builder().build()) + : new RequestMetricsBinder(registry, + ObservabilitySettings.builder().traces(false).build()); + + VaadinRequest request = uidlRequest(); + VaadinResponse response = Mockito.mock(VaadinResponse.class); + VaadinSession session = Mockito.mock(VaadinSession.class); + + binder.requestStart(request, response); + if (failure != null) { + binder.handleException(request, response, session, failure); + } + binder.requestEnd(request, response, session); + + return tags(registry, MeterNames.REQUEST_DURATION); + } + + @Test + void requestDurationTagsMatchAcrossPaths() { + Assertions.assertEquals(Map.of(ObservationNames.KEY_REQUEST_TYPE, + ObservationNames.REQUEST_TYPE_UIDL, + ObservationNames.KEY_HTTP_METHOD, "POST", + ObservationNames.KEY_INTERACTION, + ObservationNames.INTERACTION_RPC, ObservationNames.KEY_OUTCOME, + MeterNames.OUTCOME_SUCCESS, MeterNames.TAG_ERROR, + MeterNames.ERROR_NONE), recordRequest(true, null), + "the Observation path defines the vaadin.request.duration tag set"); + Assertions.assertEquals(recordRequest(true, null), + recordRequest(false, null), + "recording the Timer directly must produce the same tags"); + } + + @Test + void requestDurationErrorTagsMatchAcrossPaths() { + Map traced = recordRequest(true, + new IllegalStateException("boom")); + Map direct = recordRequest(false, + new IllegalStateException("boom")); + + Assertions.assertEquals(MeterNames.OUTCOME_ERROR, + traced.get(ObservationNames.KEY_OUTCOME)); + Assertions.assertEquals("IllegalStateException", + traced.get(MeterNames.TAG_ERROR), + "DefaultMeterObservationHandler tags with the exception's simple name"); + Assertions.assertEquals(traced, direct, + "the direct path must name the failing exception the same way"); + } + + /** + * Runs one RPC invocation through the binder and returns the tags of the + * Timer it produced. {@code traces} picks the recording path. + */ + private static Map recordRpc(boolean traces, + Throwable failure) { + SimpleMeterRegistry registry = new SimpleMeterRegistry(); + RpcMetricsBinder binder = new RpcMetricsBinder(registry, + traces ? meterProducingObservations(registry) : null, + ObservabilitySettings.builder().traces(traces).build()); + + RpcInvocationEvent event = Mockito.mock(RpcInvocationEvent.class); + Mockito.when(event.getType()).thenReturn("event"); + Mockito.when(event.getNodeId()).thenReturn(-1); + + binder.invocationStarted(event); + if (failure != null) { + binder.invocationFailed(event, failure); + } + binder.invocationEnded(event); + + return tags(registry, MeterNames.RPC_DURATION); + } + + @Test + void rpcDurationTagsMatchAcrossPaths() { + Assertions.assertEquals( + Map.of(MeterNames.TAG_TYPE, "event", MeterNames.TAG_OUTCOME, + MeterNames.OUTCOME_SUCCESS, MeterNames.TAG_ERROR, + MeterNames.ERROR_NONE), + recordRpc(true, null), + "the Observation path defines the vaadin.rpc.duration tag set"); + Assertions.assertEquals(recordRpc(true, null), recordRpc(false, null), + "recording the Timer directly must produce the same tags"); + } + + @Test + void rpcDurationErrorTagsMatchAcrossPaths() { + Map traced = recordRpc(true, + new IllegalArgumentException("boom")); + Map direct = recordRpc(false, + new IllegalArgumentException("boom")); + + Assertions.assertEquals(MeterNames.OUTCOME_ERROR, + traced.get(MeterNames.TAG_OUTCOME)); + Assertions.assertEquals("IllegalArgumentException", + traced.get(MeterNames.TAG_ERROR)); + Assertions.assertEquals(traced, direct, + "the direct path must name the failing exception the same way"); + } + + private static final class ParityView extends Component { + } + + /** + * Runs one navigation through the binder and returns the tags of the Timer + * it produced. {@code traces} picks the recording path. + */ + private static Map recordNavigation(boolean traces) { + SimpleMeterRegistry registry = new SimpleMeterRegistry(); + NavigationMetricsBinder binder = new NavigationMetricsBinder(registry, + traces ? meterProducingObservations(registry) : null, + ObservabilitySettings.builder().traces(traces).build(), + new RouteTagResolver(10)); + + UI ui = Mockito.mock(UI.class); + UI.setCurrent(ui); + BeforeEnterEvent enter = Mockito.mock(BeforeEnterEvent.class); + Mockito.when(enter.getUI()).thenReturn(ui); + // doReturn: the wildcard in Class makes the + // type-safe when(...).thenReturn(...) form uncompilable here. + Mockito.doReturn(ParityView.class).when(enter).getNavigationTarget(); + + binder.beforeEnter(enter); + binder.afterNavigation(Mockito.mock(AfterNavigationEvent.class)); + + return tags(registry, MeterNames.NAVIGATION); + } + + @Test + void navigationTagsMatchAcrossPaths() { + Assertions.assertEquals( + Map.of(MeterNames.TAG_ROUTE, "ParityView", + MeterNames.TAG_OUTCOME, MeterNames.OUTCOME_SUCCESS, + MeterNames.TAG_ERROR, MeterNames.ERROR_NONE), + recordNavigation(true), + "the Observation path defines the vaadin.navigation tag set"); + Assertions.assertEquals(recordNavigation(true), recordNavigation(false), + "recording the Timer directly must produce the same tags"); + } +} diff --git a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/RequestMetricsBinderObservationTest.java b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/RequestMetricsBinderObservationTest.java index f2384b9..19773b3 100644 --- a/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/RequestMetricsBinderObservationTest.java +++ b/observability-kit-micrometer/src/test/java/com/vaadin/observability/micrometer/RequestMetricsBinderObservationTest.java @@ -12,9 +12,14 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import io.micrometer.common.KeyValue; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import io.micrometer.observation.Observation; import io.micrometer.observation.ObservationHandler; @@ -36,6 +41,7 @@ private static final class RecordingHandler final List names = new ArrayList<>(); final List contextualNames = new ArrayList<>(); final List> tags = new ArrayList<>(); + final List> highCardinalityTags = new ArrayList<>(); final AtomicBoolean errored = new AtomicBoolean(); @Override @@ -47,6 +53,11 @@ public void onStop(Observation.Context ctx) { snap.put(kv.getKey(), kv.getValue()); } tags.add(snap); + Map highSnap = new HashMap<>(); + for (KeyValue kv : ctx.getHighCardinalityKeyValues()) { + highSnap.put(kv.getKey(), kv.getValue()); + } + highCardinalityTags.add(highSnap); if (ctx.getError() != null) { errored.set(true); } @@ -170,6 +181,61 @@ void observationCarriesErrorAndOutcomeOnException() { Assertions.assertTrue(recorder.errored.get()); } + @Test + void uiIdAndClientLocationAreSpanOnlyAndNeverTimerTags() { + SimpleMeterRegistry registry = new SimpleMeterRegistry(); + ObservationRegistry obs = ObservationRegistry.create(); + RecordingHandler recorder = new RecordingHandler(); + obs.observationConfig() + .observationHandler( + new DefaultMeterObservationHandler(registry)) + .observationHandler(recorder); + + RequestMetricsBinder binder = new RequestMetricsBinder(registry, obs, + ObservabilitySettings.builder().build()); + + VaadinRequest req = Mockito.mock(VaadinRequest.class); + Mockito.when(req.getParameter("v-r")).thenReturn("uidl"); + Mockito.when(req.getParameter("v-uiId")).thenReturn("42"); + Mockito.when(req.getHeader("Referer")) + .thenReturn("https://example.com/orders/17"); + VaadinResponse resp = Mockito.mock(VaadinResponse.class); + VaadinSession session = Mockito.mock(VaadinSession.class); + + binder.requestStart(req, resp); + binder.requestEnd(req, resp, session); + + // Both values reach the span... + Assertions.assertEquals("42", recorder.highCardinalityTags.get(0) + .get(ObservationNames.KEY_UI_ID)); + Assertions.assertEquals("/orders/17", recorder.highCardinalityTags + .get(0).get(ObservationNames.KEY_CLIENT_LOCATION)); + + // ...but neither becomes a Timer tag: a UI id is unbounded and the + // client location is un-templated, so tagging with them would multiply + // the time series of vaadin.request.duration without limit. + Assertions.assertFalse( + recorder.tags.get(0).containsKey(ObservationNames.KEY_UI_ID), + "ui.id must not be a low-cardinality Timer tag"); + Assertions.assertFalse( + recorder.tags.get(0) + .containsKey(ObservationNames.KEY_CLIENT_LOCATION), + "vaadin.client.location must not be a low-cardinality Timer tag"); + + Timer timer = registry.find(MeterNames.REQUEST_DURATION).timer(); + Assertions.assertNotNull(timer, + "the observation should have produced the request timer"); + Assertions.assertEquals(Set.of(ObservationNames.KEY_REQUEST_TYPE, + ObservationNames.KEY_INTERACTION, + ObservationNames.KEY_HTTP_METHOD, ObservationNames.KEY_OUTCOME, + // Added by DefaultMeterObservationHandler itself; the + // exception class name, or "none". + "error"), + timer.getId().getTags().stream().map(Tag::getKey) + .collect(Collectors.toSet()), + "vaadin.request.duration should carry only bounded tags"); + } + @Test void noObservationWhenTracesDisabled() { ObservationRegistry obs = ObservationRegistry.create();