[Metrics][Discover] Fix multi-dimension breakdown picker to match current grid#263629
Conversation
… fetches (elastic#263309) When multiple breakdown dimensions are selected, the METRICS_INFO query adds WHERE filters that narrow results to metrics having all selected dimensions. This caused `allDimensions` to be recomputed solely from the filtered response, making previously-available dimensions vanish from the picker. Fix: accumulate dimensions across fetches using a ref-based high-water mark in `useFetchMetricsData`. New dimensions are merged with the accumulated set on each fetch. The accumulated set resets when the base ES|QL query changes (i.e., the user switches data sources).
The JSDoc described the WHERE filter as triggering on "more than one item" and matching "at least one" selected dimension. In practice the filter triggers on any non-empty selection and joins with AND (all dimensions required). Update wording to match buildMetricsInfoQuery's actual behavior.
Relocate the esql-change detection and accumulator reset into the useAsyncFn callback. Keeping all ref mutations inside the async boundary removes a render-time side effect and colocates the reset with the merge logic that depends on it.
Previously the dimension accumulator only reset when the base ES|QL query string changed. If filters, timeRange, or esqlVariables narrowed the METRICS_INFO response, stale dimensions remained in the picker with no matching metrics in the grid. Compose a dataContextKey from esql + timeRange + filters + esqlVariables and reset the accumulator whenever it changes. The fix's original purpose — preserving dimensions across selected-dimension-driven WHERE filtering — is unaffected because selectedDimensionNames is intentionally excluded from the key.
Add hook-level tests that verify the full accumulator wiring, not just the pure mergeDimensions utility: - dimensions dropped by a filtered refetch remain in allDimensions - accumulator resets when the base ES|QL query changes - accumulator resets when timeRange changes These directly protect the dimension-picker fix from future regressions where the ref plumbing could be broken without the util tests failing.
An in-flight fetch whose signal has been aborted (because a data-context change triggered effect cleanup and a new fetch) can still resolve and reach the ref write on accumulatedDimensionsRef.current. useAsyncFn's internal stale-call tracking discards the aborted call's returned value, but the ref mutation escapes that guard, so a subsequent fetch against the new context could inherit the stale dimension. Wrap the ref write with the same !signal.aborted check already used for telemetry. Adds a regression test that reproduces the leak by ordering a context-B immediate fetch in front of a context-A fetch that resolves afterward, then issues a third fetch within context B and asserts the accumulator still contains only context-B dimensions. Addresses review comment: elastic#263629 (comment)
Move the dimension-accumulator state and reset logic out of useFetchMetricsData into a dedicated hook. The new hook takes a DependencyList of reset keys and returns a stable merge function, which the caller invokes inside its useAsyncFn callback. Why the internals still use a ref rather than useState: useFetchMetricsData writes the merged dimensions into its own useAsyncFn return value, which already triggers a render. A useState-backed accumulator would fire a second, redundant render per fetch. Holding the accumulator in a ref and returning the merged value through the caller's existing state channel keeps it to a single render without exposing the ref to callers. What this replaces: - JSON.stringify(dataContextKey) for change detection — replaced with a cheap Object.is comparison over the caller-supplied resetDeps tuple, run inside a useEffect so there's no render-time mutation. - A pair of hand-managed refs in the fetch hook — now one call. signal.aborted is threaded through `merge(incoming, aborted)` so the prior guard against aborted fetches corrupting the accumulator is preserved.
|
Pinging @elastic/obs-exploration-team (Team:obs-exploration) |
|
Based on the product input in #263309
This behavior seems correct unless I miss something.
The bug in the ticket seems different |
lucaslopezf
left a comment
There was a problem hiding this comment.
I left one comment, the PR it seems solid.
However, we are missing one AC
The list of dimensions is tailored to the grid of metrics when it is loaded (by disable them or by hiding them like we do now, but if we hide them we need to consider the scenario where the user selects multiple before the grid refreshes because that could end up in selections that do not exist)
Also, it’s worth taking a look at our E2E tests. I’m pretty sure we can cover the dimensions dropdown behavior with the tests we already have (or by adding a new one). IMO is important to cover the full behavior of the dropdown to avoid regressions or misunderstandings in the future.
mergeDimensions was doing two things — set-merge and alphabetical sorting — which coupled an incidental caller concern (picker order) with the core utility. Move the sort back to useFetchMetricsData where it sits next to the metricItems sort, keeping mergeDimensions focused on merge semantics so a future sort-policy change has a single home. Addresses lucaslopezf's review: elastic#263629 (comment)
The accumulator was the wrong fix for elastic#263309. The ticket asks the picker to be tailored to the current grid (AC1) and to surface currently-selected dimensions even when the refreshed response drops them (AC2/AC3). The accumulator did the opposite: it preserved every dimension ever seen, including ones not applicable to the current grid. Strip it out so a targeted fix in DimensionsSelector can take its place. - Delete utils/merge_dimensions.{ts,test.ts} - Delete hooks/use_accumulated_dimensions.ts - use_fetch_metrics_data.ts: sort parsed.allDimensions directly, drop mergeAccumulatedDimensions from the useAsyncFn deps - use_fetch_metrics_data.test.ts: remove the "dimension accumulator" suite; the remaining tests exercise the now-direct sort and the existing refetch/race-condition behaviours
… set Fixes ticket elastic#263309 AC2/AC3: when the user breaks down by multiple dimensions and the latest METRICS_INFO response drops one of their selections from the applicable set (because the server-side `WHERE dim IS NOT NULL` filter narrowed the matching metrics), the picker now keeps that "orphan" selection visible at the top of the list so the count badge still matches the rendered ticks and the user can always back out of an invalid combination. - dimensions_selector.tsx: compute orphan selections (localSelected minus applicable names), prepend them to the options list in alphabetical order with `checked: 'on'`, and update handleChange to resolve Dimension records from the union of applicable + locally-selected so deselecting an orphan still produces a well-formed new selection array. - dimensions_selector.test.tsx: extend the ToolbarSelector mock to simulate the real multi-selection toggle contract (click a checked option -> emit the remaining checked options as an array) so the new tests exercise the component's handleChange with the same payload shape it gets at runtime. Add a "Selected dimensions not in applicable set" suite covering visibility, ordering, popover count, and orphan deselection.
Phase 6 of elastic#263309 rework. After the user clicks the first dimension, the picker now narrows its option list client-side to dimensions carried by at least one metric that also carries every current selection — before the debounced fetch fires. This prevents the throttled-network rapid-click race where the second click could land on a dimension disjoint from the first and drop the grid into a "No results" state. The orphan-surfacing path (Phase 2) stays as a safety net for URL-restore and any server edge case where the prediction disagrees with the real response. - DimensionsSelector: new optional metricItems prop; options useMemo filters dimensions against the optimistic applicable-name set when both a selection and metricItems are provided. - useToolbarActions + MetricsExperienceGrid: forward metricItems (the pre-search-filter set from useFetchMetricsData) into the selector. - Tests: 4 new cases covering the filter, the no-metricItems fallback, orphan composition, and the empty-selection no-op.
…/kibana into 263309/fix-dimension-picker
|
@justinkambic I was wondering if you could add a short recording showing the before/after. This really helps understanding the problem quickly and seeing it being solved. I usually use this as guide for my PR review. |
The lens entry bundle was 72-83B over its 86,000B page-load limit after rebasing this branch on main. Nothing in this PR's scope (kbn-unified-chart-section-viewer + one Discover test spec) can reach lens's bundle — a codebase-wide grep shows zero imports of @kbn/unified-chart-section-viewer from x-pack/platform/plugins/shared/lens, and a local dist build's stats.json contains no matching module paths. Cumulative drift from recent Lens-as-code commits that merged into main (e.g., elastic#262871, elastic#264134, elastic#261581, elastic#264147, elastic#263810) pushed the entry bundle just over the line. Raising the limit here is the documented path when no in-PR contribution is found. Limit set via `node scripts/build_kibana_platform_plugins --focus lens --update-limits`.
|
CI flagged the
The likely sources are the recent lens-touching commits that merged into main and were pulled into this branch via the rebase:
No individual PR tripped the limit on its own, but cumulative drift pushed the current size 72–83B over. Per the CI Metrics docs:
I bumped the Tagging the Operations team for the standard |
lucaslopezf
left a comment
There was a problem hiding this comment.
Great! I left some comments, and also I pushed this commit as an example of what I was trying to said (not a final one, just to give you an idea). Let me know what do you think and happy to discuss if we need it
| @@ -0,0 +1,335 @@ | |||
| /* | |||
There was a problem hiding this comment.
Could we add unit tests covering the whole logic of this hook?
| }, [debouncedOnChange]); | ||
|
|
||
| const handleChange = useCallback( | ||
| (chosenOption?: SelectableEntry | SelectableEntry[]) => { |
There was a problem hiding this comment.
In the toOption we are casting from Dimension to SelectableEntry, and here we are doing the viceversa. We could avoid some loops if we define the type
DimensionEntry = SelectableEntry & { dimension: Dimension }| optimisticApplicableNames, | ||
| }); | ||
|
|
||
| const toOption = (dimension: Dimension): SelectableEntry => { |
There was a problem hiding this comment.
I think is worth it create a helper to this function, wdyt?
There was a problem hiding this comment.
c2680f6 (same as previous comment, rolled into the same change)
| onChange([]); | ||
| }, [onChange, debouncedOnChange]); | ||
|
|
||
| const buttonLabel = useMemo<ReactElement>(() => { |
There was a problem hiding this comment.
We could wrap this using components, wdyt?
| ); | ||
| }, [localSelectedDimensions, isLoading]); | ||
|
|
||
| const buttonTooltipContent = useMemo<ReactElement | undefined>(() => { |
There was a problem hiding this comment.
Same here, I think the hook will be much simpler to understand if we split re-use components here instead of defining the jsx in one place
| ); | ||
| }, [localSelectedDimensions]); | ||
|
|
||
| const popoverContentBelowSearch = useMemo<ReactElement>(() => { |
There was a problem hiding this comment.
And same here, we could use components. Even though I like the new approach, the logic feels more complex than everyone expect for a dropdown. Also, I’m pretty sure we’ll need to modify the business logic in the future, that's why I think we should try the cleanest way as possible
There was a problem hiding this comment.
618e8d9 again handled together with the last two
…ldDimensionOption
Addresses review feedback on use_dimensions_selector.tsx:
- Introduce `DimensionEntry = SelectableEntry & { dimension: Dimension }` so
the selected `Dimension` travels with the option instead of being recovered
by a reverse lookup in `handleChange`. Drops the per-change Map build and
removes `dimensions` / `localSelectedDimensions` from the callback's deps.
- Extract `buildDimensionOption` into `dimensions_selector_helpers.ts` so the
hook's `options` memo can focus on partitioning and disabled-state logic.
Comments addressed:
- elastic#263629 (comment) (#2)
- elastic#263629 (comment) (#3)
…nents Split the inline JSX blobs out of `useDimensionsSelector` into dedicated components under `dimensions_selector_components/`: - `DimensionsButtonLabel` — trigger-button content (count badge + spinner). - `MaxDimensionsWarning` — shared `FormattedMessage` used by the popover tooltip overlay and the button-level tooltip (was duplicated before). - `MaxDimensionsTooltipOverlay` — absolute-positioned `EuiToolTip` overlay that activates on disabled (at-max-limit) options. - `DimensionsPopoverFooter` — count + "Clear selection" footer rendered below the search input. With the JSX extracted, the hook drops `euiTheme.levels.menu` from the `options` memo's deps (it now lives in the overlay component) and no longer needs the `EuiButtonEmpty` / `EuiFlexGroup` / `EuiFlexItem` / `EuiText` / `EuiLoadingSpinner` / `EuiNotificationBadge` / `EuiSpacer` / `EuiToolTip` / `useEuiTheme` imports. The hook stays responsible only for state, derivation, and callbacks. Comments addressed: - elastic#263629 (comment) (#4) - elastic#263629 (comment) (#5) - elastic#263629 (comment) (#6)
Covers the hook's full logic surface directly, rather than only through the surrounding `DimensionsSelector` component: - `options`: dimension carried on entry, checked marker, multi-select at-limit disable, tooltip overlay only on disabled-at-limit rows, no disabling in single-select mode, orphan prepending, optimistic metricItems filter. - `selectedValues`: de-duplicated name list. - Local/controlled sync: `selectedDimensions` prop change re-syncs. - `handleChange` (multi): debounces at `DEBOUNCE_TIME`, collapses rapid changes, caps at `MAX_DIMENSIONS_SELECTIONS`, handles `undefined` input. - `handleChange` (single): synchronous, no debounce. - Presentational nodes: `buttonTooltipContent` only when at-max, `buttonLabel`/`popoverContentBelowSearch` always present. - Defensive: options missing a `dimension` field are dropped, not crashed on. Also picks up `eslint --fix` formatting tweaks in the hook file for the now-one-line `DimensionsButtonLabel` / `DimensionsPopoverFooter` JSX that followed the extraction in the previous commit. Comment addressed: - elastic#263629 (comment) (#1)
|
Thanks @lucaslopezf - I think I addressed this feedback. Sorry @cauemarcondes I still haven't taken the time to record a screenshot as I was doing SDH stuff earlier. I'll try to get to it today yet. |
|
@cauemarcondes here's the primary issue we're solving for in this PR: on
|
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
|
lucaslopezf
left a comment
There was a problem hiding this comment.
Hey @justinkambic, much better now, I’ve approved it!
Miguel just created this task: #264957 (I think it’s related to this PR).
Would you prefer merging this one and addressing that in a follow-up PR (It doesn't have to be you, just to synchronize), or handling it here? What do you think?
| // so we can read it straight off the option and skip the reverse lookup | ||
| // against `dimensions` + `localSelectedDimensions` that would otherwise | ||
| // be needed to recover selections no longer present in `dimensions`. | ||
| const newSelection = (opts as DimensionEntry[]) |
There was a problem hiding this comment.
nit: Maybe we could find another cleaner way. But anyway, not blocker at all and we can follow up in this task
There was a problem hiding this comment.
Going to defer to the next patch, don't really have time this week to adjust I think the miss is pretty low impact.
I'll leave a comment on Miguel's new ticket noting that it may be closely aligned with this PR, but given that this is basically ready to go I'd rather not fold additional scope in. I'd gladly tackle the new one but as I'm on SDH this week I don't have capacity to dive into new work just now. If it's still unassigned next week I can take a stab at it! EDIT: I see you've already looped that task into this one, so no need for me to comment. Thank you! |
Agree! From my side let's merge it and we can follow up the changes with the new one |
|
Starting backport for target branches: 9.4 https://github.com/elastic/kibana/actions/runs/24795785671 |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…ch current grid (#263629) (#265127) # Backport This will backport the following commits from `main` to `9.4`: - [[Metrics][Discover] Fix multi-dimension breakdown picker to match current grid (#263629)](#263629) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Justin Kambic","email":"jk@elastic.co"},"sourceCommit":{"committedDate":"2026-04-22T18:34:15Z","message":"[Metrics][Discover] Fix multi-dimension breakdown picker to match current grid (#263629)\n\n## Summary\n\nResolves #263309.\n\nThe Metrics in Discover dimension picker had three related problems:\n\n1. The option list wasn't tailored to the metrics currently in the grid:\nusers could select a dimension that didn't exist in any visible metric.\n2. Selecting a second dimension could cause the first to silently drop\nout of the picker, producing a \"2 selected, 1 tick\" state (see the\nscreenshots on the ticket).\n3. Rapid multi-select across disjoint dimensions could land the user in\nan empty-grid dead state before the fetch returned.\n\nThis PR fixes all three.\n\n- **Tailored list.** The picker's option list is now derived directly\nfrom the dimensions present in the current `METRICS_INFO` response. This\nis the union of `dimensionFields` across the currently visible metrics.\n- **Selected-still-visible.** Any currently-selected dimension that is\nnot in the applicable set (a rare race-condition or URL-restore case) is\nsurfaced at the top of the picker with its checkmark intact, so the\ncount badge and visible ticks always match and the user can deselect to\nget out of the weird state.\n- **Optimistic pre-filter.** When the user clicks a dimension, the\npicker immediately recomputes its option list from the in-memory\n`metricItems` plus the new local selection, hiding any dimension not\ncarried by a metric that already has the selection. This makes\nnon-applicable options disappear before the server round-trip, so rapid\nmulti-select shouldn't reach an empty-grid state via the picker.\n\n## Commit layout\n\n| Commit | Purpose |\n|---|---|\n| `1eb2379e` | Revert the earlier accumulator approach (delete\n`merge_dimensions.ts`, `use_accumulated_dimensions.ts`, related\nplumbing). |\n| `290aaf17` | Surface selected dimensions even when not in the\napplicable set (orphan safety net). |\n| `491456ac` | Optimistically filter picker options by current local\nselection. |\n\n## How this maps to the ticket\n\n| Ticket clause | How it's satisfied |\n|---|---|\n| \"The list of dimensions should be tailored to what you see in the\ngrid\" | `allDimensions` is now the union of\n`metricItems[*].dimensionFields` from the current response. |\n| \"You shouldn't be able to [select a dimension that doesn't exist in\none of the metrics]\" | Optimistic filter removes non-applicable options\nthe moment the first selection is clicked — the race click on a\nsubsequently-invalid dimension cannot hit the DOM. |\n| \"2 dimensions are selected but only one with the tick\" | Selected\ndimensions always render with a checkmark, via the orphan-surfacing path\nwhen they fall outside the applicable set. |\n| \"if we hide them we need to consider the scenario where the user\nselects multiple before the grid refreshes\" | Orphan surfacing +\noptimistic filter together make this scenario unreachable through the\npicker, and visible/recoverable when it is reached via URL or external\nstate. |\n\n## Before / after\n\n_Before:_ Picker lists dimensions regardless of whether the user can\nmeaningfully select them, and silently drops previously-selected\ndimensions if the filtered response no longer carries them.\n\n_After:_ Picker lists only dimensions that apply to the current grid,\nprevents selecting a dimension that would produce an empty grid, and\nnever hides an already-selected dimension.\n\n## Test plan\n\nManual verification focuses on the two scenarios that can't be covered\nby the unit tests below. Applicable-set narrowing, orphan surfacing,\ncount/tick consistency, and max-selection behavior are fully asserted in\n`dimensions_selector.test.tsx`; refetch on context change is asserted in\n`use_fetch_metrics_data.test.ts`.\n\nDataset: TSDB indices with divergent dimension sets: e.g. one metric\nwith dims `environment` + `host.name`, another with `host.name` +\n`region`. The repo's Scout fixture currently puts every dimension on\nevery metric, so for manual verification use a local `metrics-test*`\nwith divergent shapes. A follow-up ticket tracks extending the Scout\nfixture to cover this scenario end-to-end.\n\n- **Ticket repro, end-to-end.** `TS metrics-test*`: open Dimensions\npicker and see all dimensions visible.\n- Select `environment` picker collapses to `environment` + `host.name`,\n`region` is no longer in the list.\n - Deselect: all three return.\n- **Rapid multi-select (throttled network).** Use DevTools to throttle\nthe network, open the picker, click `environment` then attempt to click\n`region` before the first fetch returns. `region` must no longer be in\nthe DOM by the time the second click fires, so the empty-grid state is\nunreachable via the picker. This exercises the optimistic filter's\nrender-race behavior in a real browser, which unit tests can't simulate.\n\n## Follow-ups\n\n- **Scout fixture extension** to reproduce the ticket end-to-end — the\ncurrent generator spreads every dimension onto every metric, which\ndefeats the scenario.\nhttps://github.com/elastic/observability-dev/issues/5481\n- **Orphan styling** — decide whether orphan selections should have a\nvisual distinguisher beyond placement/checkmark. Separate ticket.\n\n---------\n\nCo-authored-by: Lucas Francisco López <lucaslopezf@gmail.com>","sha":"f093d702faced1366926ec27f200f58dcfa453ab","branchLabelMapping":{"^v9.5.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","backport:version","Team:obs-exploration","v9.4.0","v9.5.0"],"title":"[Metrics][Discover] Fix multi-dimension breakdown picker to match current grid","number":263629,"url":"https://github.com/elastic/kibana/pull/263629","mergeCommit":{"message":"[Metrics][Discover] Fix multi-dimension breakdown picker to match current grid (#263629)\n\n## Summary\n\nResolves #263309.\n\nThe Metrics in Discover dimension picker had three related problems:\n\n1. The option list wasn't tailored to the metrics currently in the grid:\nusers could select a dimension that didn't exist in any visible metric.\n2. Selecting a second dimension could cause the first to silently drop\nout of the picker, producing a \"2 selected, 1 tick\" state (see the\nscreenshots on the ticket).\n3. Rapid multi-select across disjoint dimensions could land the user in\nan empty-grid dead state before the fetch returned.\n\nThis PR fixes all three.\n\n- **Tailored list.** The picker's option list is now derived directly\nfrom the dimensions present in the current `METRICS_INFO` response. This\nis the union of `dimensionFields` across the currently visible metrics.\n- **Selected-still-visible.** Any currently-selected dimension that is\nnot in the applicable set (a rare race-condition or URL-restore case) is\nsurfaced at the top of the picker with its checkmark intact, so the\ncount badge and visible ticks always match and the user can deselect to\nget out of the weird state.\n- **Optimistic pre-filter.** When the user clicks a dimension, the\npicker immediately recomputes its option list from the in-memory\n`metricItems` plus the new local selection, hiding any dimension not\ncarried by a metric that already has the selection. This makes\nnon-applicable options disappear before the server round-trip, so rapid\nmulti-select shouldn't reach an empty-grid state via the picker.\n\n## Commit layout\n\n| Commit | Purpose |\n|---|---|\n| `1eb2379e` | Revert the earlier accumulator approach (delete\n`merge_dimensions.ts`, `use_accumulated_dimensions.ts`, related\nplumbing). |\n| `290aaf17` | Surface selected dimensions even when not in the\napplicable set (orphan safety net). |\n| `491456ac` | Optimistically filter picker options by current local\nselection. |\n\n## How this maps to the ticket\n\n| Ticket clause | How it's satisfied |\n|---|---|\n| \"The list of dimensions should be tailored to what you see in the\ngrid\" | `allDimensions` is now the union of\n`metricItems[*].dimensionFields` from the current response. |\n| \"You shouldn't be able to [select a dimension that doesn't exist in\none of the metrics]\" | Optimistic filter removes non-applicable options\nthe moment the first selection is clicked — the race click on a\nsubsequently-invalid dimension cannot hit the DOM. |\n| \"2 dimensions are selected but only one with the tick\" | Selected\ndimensions always render with a checkmark, via the orphan-surfacing path\nwhen they fall outside the applicable set. |\n| \"if we hide them we need to consider the scenario where the user\nselects multiple before the grid refreshes\" | Orphan surfacing +\noptimistic filter together make this scenario unreachable through the\npicker, and visible/recoverable when it is reached via URL or external\nstate. |\n\n## Before / after\n\n_Before:_ Picker lists dimensions regardless of whether the user can\nmeaningfully select them, and silently drops previously-selected\ndimensions if the filtered response no longer carries them.\n\n_After:_ Picker lists only dimensions that apply to the current grid,\nprevents selecting a dimension that would produce an empty grid, and\nnever hides an already-selected dimension.\n\n## Test plan\n\nManual verification focuses on the two scenarios that can't be covered\nby the unit tests below. Applicable-set narrowing, orphan surfacing,\ncount/tick consistency, and max-selection behavior are fully asserted in\n`dimensions_selector.test.tsx`; refetch on context change is asserted in\n`use_fetch_metrics_data.test.ts`.\n\nDataset: TSDB indices with divergent dimension sets: e.g. one metric\nwith dims `environment` + `host.name`, another with `host.name` +\n`region`. The repo's Scout fixture currently puts every dimension on\nevery metric, so for manual verification use a local `metrics-test*`\nwith divergent shapes. A follow-up ticket tracks extending the Scout\nfixture to cover this scenario end-to-end.\n\n- **Ticket repro, end-to-end.** `TS metrics-test*`: open Dimensions\npicker and see all dimensions visible.\n- Select `environment` picker collapses to `environment` + `host.name`,\n`region` is no longer in the list.\n - Deselect: all three return.\n- **Rapid multi-select (throttled network).** Use DevTools to throttle\nthe network, open the picker, click `environment` then attempt to click\n`region` before the first fetch returns. `region` must no longer be in\nthe DOM by the time the second click fires, so the empty-grid state is\nunreachable via the picker. This exercises the optimistic filter's\nrender-race behavior in a real browser, which unit tests can't simulate.\n\n## Follow-ups\n\n- **Scout fixture extension** to reproduce the ticket end-to-end — the\ncurrent generator spreads every dimension onto every metric, which\ndefeats the scenario.\nhttps://github.com/elastic/observability-dev/issues/5481\n- **Orphan styling** — decide whether orphan selections should have a\nvisual distinguisher beyond placement/checkmark. Separate ticket.\n\n---------\n\nCo-authored-by: Lucas Francisco López <lucaslopezf@gmail.com>","sha":"f093d702faced1366926ec27f200f58dcfa453ab"}},"sourceBranch":"main","suggestedTargetBranches":["9.4"],"targetPullRequestStates":[{"branch":"9.4","label":"v9.4.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.5.0","branchLabelMappingKey":"^v9.5.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/263629","number":263629,"mergeCommit":{"message":"[Metrics][Discover] Fix multi-dimension breakdown picker to match current grid (#263629)\n\n## Summary\n\nResolves #263309.\n\nThe Metrics in Discover dimension picker had three related problems:\n\n1. The option list wasn't tailored to the metrics currently in the grid:\nusers could select a dimension that didn't exist in any visible metric.\n2. Selecting a second dimension could cause the first to silently drop\nout of the picker, producing a \"2 selected, 1 tick\" state (see the\nscreenshots on the ticket).\n3. Rapid multi-select across disjoint dimensions could land the user in\nan empty-grid dead state before the fetch returned.\n\nThis PR fixes all three.\n\n- **Tailored list.** The picker's option list is now derived directly\nfrom the dimensions present in the current `METRICS_INFO` response. This\nis the union of `dimensionFields` across the currently visible metrics.\n- **Selected-still-visible.** Any currently-selected dimension that is\nnot in the applicable set (a rare race-condition or URL-restore case) is\nsurfaced at the top of the picker with its checkmark intact, so the\ncount badge and visible ticks always match and the user can deselect to\nget out of the weird state.\n- **Optimistic pre-filter.** When the user clicks a dimension, the\npicker immediately recomputes its option list from the in-memory\n`metricItems` plus the new local selection, hiding any dimension not\ncarried by a metric that already has the selection. This makes\nnon-applicable options disappear before the server round-trip, so rapid\nmulti-select shouldn't reach an empty-grid state via the picker.\n\n## Commit layout\n\n| Commit | Purpose |\n|---|---|\n| `1eb2379e` | Revert the earlier accumulator approach (delete\n`merge_dimensions.ts`, `use_accumulated_dimensions.ts`, related\nplumbing). |\n| `290aaf17` | Surface selected dimensions even when not in the\napplicable set (orphan safety net). |\n| `491456ac` | Optimistically filter picker options by current local\nselection. |\n\n## How this maps to the ticket\n\n| Ticket clause | How it's satisfied |\n|---|---|\n| \"The list of dimensions should be tailored to what you see in the\ngrid\" | `allDimensions` is now the union of\n`metricItems[*].dimensionFields` from the current response. |\n| \"You shouldn't be able to [select a dimension that doesn't exist in\none of the metrics]\" | Optimistic filter removes non-applicable options\nthe moment the first selection is clicked — the race click on a\nsubsequently-invalid dimension cannot hit the DOM. |\n| \"2 dimensions are selected but only one with the tick\" | Selected\ndimensions always render with a checkmark, via the orphan-surfacing path\nwhen they fall outside the applicable set. |\n| \"if we hide them we need to consider the scenario where the user\nselects multiple before the grid refreshes\" | Orphan surfacing +\noptimistic filter together make this scenario unreachable through the\npicker, and visible/recoverable when it is reached via URL or external\nstate. |\n\n## Before / after\n\n_Before:_ Picker lists dimensions regardless of whether the user can\nmeaningfully select them, and silently drops previously-selected\ndimensions if the filtered response no longer carries them.\n\n_After:_ Picker lists only dimensions that apply to the current grid,\nprevents selecting a dimension that would produce an empty grid, and\nnever hides an already-selected dimension.\n\n## Test plan\n\nManual verification focuses on the two scenarios that can't be covered\nby the unit tests below. Applicable-set narrowing, orphan surfacing,\ncount/tick consistency, and max-selection behavior are fully asserted in\n`dimensions_selector.test.tsx`; refetch on context change is asserted in\n`use_fetch_metrics_data.test.ts`.\n\nDataset: TSDB indices with divergent dimension sets: e.g. one metric\nwith dims `environment` + `host.name`, another with `host.name` +\n`region`. The repo's Scout fixture currently puts every dimension on\nevery metric, so for manual verification use a local `metrics-test*`\nwith divergent shapes. A follow-up ticket tracks extending the Scout\nfixture to cover this scenario end-to-end.\n\n- **Ticket repro, end-to-end.** `TS metrics-test*`: open Dimensions\npicker and see all dimensions visible.\n- Select `environment` picker collapses to `environment` + `host.name`,\n`region` is no longer in the list.\n - Deselect: all three return.\n- **Rapid multi-select (throttled network).** Use DevTools to throttle\nthe network, open the picker, click `environment` then attempt to click\n`region` before the first fetch returns. `region` must no longer be in\nthe DOM by the time the second click fires, so the empty-grid state is\nunreachable via the picker. This exercises the optimistic filter's\nrender-race behavior in a real browser, which unit tests can't simulate.\n\n## Follow-ups\n\n- **Scout fixture extension** to reproduce the ticket end-to-end — the\ncurrent generator spreads every dimension onto every metric, which\ndefeats the scenario.\nhttps://github.com/elastic/observability-dev/issues/5481\n- **Orphan styling** — decide whether orphan selections should have a\nvisual distinguisher beyond placement/checkmark. Separate ticket.\n\n---------\n\nCo-authored-by: Lucas Francisco López <lucaslopezf@gmail.com>","sha":"f093d702faced1366926ec27f200f58dcfa453ab"}}]}] BACKPORT--> Co-authored-by: Justin Kambic <jk@elastic.co> Co-authored-by: Lucas Francisco López <lucaslopezf@gmail.com>
…rent grid (elastic#263629) ## Summary Resolves elastic#263309. The Metrics in Discover dimension picker had three related problems: 1. The option list wasn't tailored to the metrics currently in the grid: users could select a dimension that didn't exist in any visible metric. 2. Selecting a second dimension could cause the first to silently drop out of the picker, producing a "2 selected, 1 tick" state (see the screenshots on the ticket). 3. Rapid multi-select across disjoint dimensions could land the user in an empty-grid dead state before the fetch returned. This PR fixes all three. - **Tailored list.** The picker's option list is now derived directly from the dimensions present in the current `METRICS_INFO` response. This is the union of `dimensionFields` across the currently visible metrics. - **Selected-still-visible.** Any currently-selected dimension that is not in the applicable set (a rare race-condition or URL-restore case) is surfaced at the top of the picker with its checkmark intact, so the count badge and visible ticks always match and the user can deselect to get out of the weird state. - **Optimistic pre-filter.** When the user clicks a dimension, the picker immediately recomputes its option list from the in-memory `metricItems` plus the new local selection, hiding any dimension not carried by a metric that already has the selection. This makes non-applicable options disappear before the server round-trip, so rapid multi-select shouldn't reach an empty-grid state via the picker. ## Commit layout | Commit | Purpose | |---|---| | `1eb2379e` | Revert the earlier accumulator approach (delete `merge_dimensions.ts`, `use_accumulated_dimensions.ts`, related plumbing). | | `290aaf17` | Surface selected dimensions even when not in the applicable set (orphan safety net). | | `491456ac` | Optimistically filter picker options by current local selection. | ## How this maps to the ticket | Ticket clause | How it's satisfied | |---|---| | "The list of dimensions should be tailored to what you see in the grid" | `allDimensions` is now the union of `metricItems[*].dimensionFields` from the current response. | | "You shouldn't be able to [select a dimension that doesn't exist in one of the metrics]" | Optimistic filter removes non-applicable options the moment the first selection is clicked — the race click on a subsequently-invalid dimension cannot hit the DOM. | | "2 dimensions are selected but only one with the tick" | Selected dimensions always render with a checkmark, via the orphan-surfacing path when they fall outside the applicable set. | | "if we hide them we need to consider the scenario where the user selects multiple before the grid refreshes" | Orphan surfacing + optimistic filter together make this scenario unreachable through the picker, and visible/recoverable when it is reached via URL or external state. | ## Before / after _Before:_ Picker lists dimensions regardless of whether the user can meaningfully select them, and silently drops previously-selected dimensions if the filtered response no longer carries them. _After:_ Picker lists only dimensions that apply to the current grid, prevents selecting a dimension that would produce an empty grid, and never hides an already-selected dimension. ## Test plan Manual verification focuses on the two scenarios that can't be covered by the unit tests below. Applicable-set narrowing, orphan surfacing, count/tick consistency, and max-selection behavior are fully asserted in `dimensions_selector.test.tsx`; refetch on context change is asserted in `use_fetch_metrics_data.test.ts`. Dataset: TSDB indices with divergent dimension sets: e.g. one metric with dims `environment` + `host.name`, another with `host.name` + `region`. The repo's Scout fixture currently puts every dimension on every metric, so for manual verification use a local `metrics-test*` with divergent shapes. A follow-up ticket tracks extending the Scout fixture to cover this scenario end-to-end. - **Ticket repro, end-to-end.** `TS metrics-test*`: open Dimensions picker and see all dimensions visible. - Select `environment` picker collapses to `environment` + `host.name`, `region` is no longer in the list. - Deselect: all three return. - **Rapid multi-select (throttled network).** Use DevTools to throttle the network, open the picker, click `environment` then attempt to click `region` before the first fetch returns. `region` must no longer be in the DOM by the time the second click fires, so the empty-grid state is unreachable via the picker. This exercises the optimistic filter's render-race behavior in a real browser, which unit tests can't simulate. ## Follow-ups - **Scout fixture extension** to reproduce the ticket end-to-end — the current generator spreads every dimension onto every metric, which defeats the scenario. elastic/observability-dev#5481 - **Orphan styling** — decide whether orphan selections should have a visual distinguisher beyond placement/checkmark. Separate ticket. --------- Co-authored-by: Lucas Francisco López <lucaslopezf@gmail.com>
…rent grid (elastic#263629) ## Summary Resolves elastic#263309. The Metrics in Discover dimension picker had three related problems: 1. The option list wasn't tailored to the metrics currently in the grid: users could select a dimension that didn't exist in any visible metric. 2. Selecting a second dimension could cause the first to silently drop out of the picker, producing a "2 selected, 1 tick" state (see the screenshots on the ticket). 3. Rapid multi-select across disjoint dimensions could land the user in an empty-grid dead state before the fetch returned. This PR fixes all three. - **Tailored list.** The picker's option list is now derived directly from the dimensions present in the current `METRICS_INFO` response. This is the union of `dimensionFields` across the currently visible metrics. - **Selected-still-visible.** Any currently-selected dimension that is not in the applicable set (a rare race-condition or URL-restore case) is surfaced at the top of the picker with its checkmark intact, so the count badge and visible ticks always match and the user can deselect to get out of the weird state. - **Optimistic pre-filter.** When the user clicks a dimension, the picker immediately recomputes its option list from the in-memory `metricItems` plus the new local selection, hiding any dimension not carried by a metric that already has the selection. This makes non-applicable options disappear before the server round-trip, so rapid multi-select shouldn't reach an empty-grid state via the picker. ## Commit layout | Commit | Purpose | |---|---| | `1eb2379e` | Revert the earlier accumulator approach (delete `merge_dimensions.ts`, `use_accumulated_dimensions.ts`, related plumbing). | | `290aaf17` | Surface selected dimensions even when not in the applicable set (orphan safety net). | | `491456ac` | Optimistically filter picker options by current local selection. | ## How this maps to the ticket | Ticket clause | How it's satisfied | |---|---| | "The list of dimensions should be tailored to what you see in the grid" | `allDimensions` is now the union of `metricItems[*].dimensionFields` from the current response. | | "You shouldn't be able to [select a dimension that doesn't exist in one of the metrics]" | Optimistic filter removes non-applicable options the moment the first selection is clicked — the race click on a subsequently-invalid dimension cannot hit the DOM. | | "2 dimensions are selected but only one with the tick" | Selected dimensions always render with a checkmark, via the orphan-surfacing path when they fall outside the applicable set. | | "if we hide them we need to consider the scenario where the user selects multiple before the grid refreshes" | Orphan surfacing + optimistic filter together make this scenario unreachable through the picker, and visible/recoverable when it is reached via URL or external state. | ## Before / after _Before:_ Picker lists dimensions regardless of whether the user can meaningfully select them, and silently drops previously-selected dimensions if the filtered response no longer carries them. _After:_ Picker lists only dimensions that apply to the current grid, prevents selecting a dimension that would produce an empty grid, and never hides an already-selected dimension. ## Test plan Manual verification focuses on the two scenarios that can't be covered by the unit tests below. Applicable-set narrowing, orphan surfacing, count/tick consistency, and max-selection behavior are fully asserted in `dimensions_selector.test.tsx`; refetch on context change is asserted in `use_fetch_metrics_data.test.ts`. Dataset: TSDB indices with divergent dimension sets: e.g. one metric with dims `environment` + `host.name`, another with `host.name` + `region`. The repo's Scout fixture currently puts every dimension on every metric, so for manual verification use a local `metrics-test*` with divergent shapes. A follow-up ticket tracks extending the Scout fixture to cover this scenario end-to-end. - **Ticket repro, end-to-end.** `TS metrics-test*`: open Dimensions picker and see all dimensions visible. - Select `environment` picker collapses to `environment` + `host.name`, `region` is no longer in the list. - Deselect: all three return. - **Rapid multi-select (throttled network).** Use DevTools to throttle the network, open the picker, click `environment` then attempt to click `region` before the first fetch returns. `region` must no longer be in the DOM by the time the second click fires, so the empty-grid state is unreachable via the picker. This exercises the optimistic filter's render-race behavior in a real browser, which unit tests can't simulate. ## Follow-ups - **Scout fixture extension** to reproduce the ticket end-to-end — the current generator spreads every dimension onto every metric, which defeats the scenario. elastic/observability-dev#5481 - **Orphan styling** — decide whether orphan selections should have a visual distinguisher beyond placement/checkmark. Separate ticket. --------- Co-authored-by: Lucas Francisco López <lucaslopezf@gmail.com>
…rent grid (elastic#263629) ## Summary Resolves elastic#263309. The Metrics in Discover dimension picker had three related problems: 1. The option list wasn't tailored to the metrics currently in the grid: users could select a dimension that didn't exist in any visible metric. 2. Selecting a second dimension could cause the first to silently drop out of the picker, producing a "2 selected, 1 tick" state (see the screenshots on the ticket). 3. Rapid multi-select across disjoint dimensions could land the user in an empty-grid dead state before the fetch returned. This PR fixes all three. - **Tailored list.** The picker's option list is now derived directly from the dimensions present in the current `METRICS_INFO` response. This is the union of `dimensionFields` across the currently visible metrics. - **Selected-still-visible.** Any currently-selected dimension that is not in the applicable set (a rare race-condition or URL-restore case) is surfaced at the top of the picker with its checkmark intact, so the count badge and visible ticks always match and the user can deselect to get out of the weird state. - **Optimistic pre-filter.** When the user clicks a dimension, the picker immediately recomputes its option list from the in-memory `metricItems` plus the new local selection, hiding any dimension not carried by a metric that already has the selection. This makes non-applicable options disappear before the server round-trip, so rapid multi-select shouldn't reach an empty-grid state via the picker. ## Commit layout | Commit | Purpose | |---|---| | `1eb2379e` | Revert the earlier accumulator approach (delete `merge_dimensions.ts`, `use_accumulated_dimensions.ts`, related plumbing). | | `290aaf17` | Surface selected dimensions even when not in the applicable set (orphan safety net). | | `491456ac` | Optimistically filter picker options by current local selection. | ## How this maps to the ticket | Ticket clause | How it's satisfied | |---|---| | "The list of dimensions should be tailored to what you see in the grid" | `allDimensions` is now the union of `metricItems[*].dimensionFields` from the current response. | | "You shouldn't be able to [select a dimension that doesn't exist in one of the metrics]" | Optimistic filter removes non-applicable options the moment the first selection is clicked — the race click on a subsequently-invalid dimension cannot hit the DOM. | | "2 dimensions are selected but only one with the tick" | Selected dimensions always render with a checkmark, via the orphan-surfacing path when they fall outside the applicable set. | | "if we hide them we need to consider the scenario where the user selects multiple before the grid refreshes" | Orphan surfacing + optimistic filter together make this scenario unreachable through the picker, and visible/recoverable when it is reached via URL or external state. | ## Before / after _Before:_ Picker lists dimensions regardless of whether the user can meaningfully select them, and silently drops previously-selected dimensions if the filtered response no longer carries them. _After:_ Picker lists only dimensions that apply to the current grid, prevents selecting a dimension that would produce an empty grid, and never hides an already-selected dimension. ## Test plan Manual verification focuses on the two scenarios that can't be covered by the unit tests below. Applicable-set narrowing, orphan surfacing, count/tick consistency, and max-selection behavior are fully asserted in `dimensions_selector.test.tsx`; refetch on context change is asserted in `use_fetch_metrics_data.test.ts`. Dataset: TSDB indices with divergent dimension sets: e.g. one metric with dims `environment` + `host.name`, another with `host.name` + `region`. The repo's Scout fixture currently puts every dimension on every metric, so for manual verification use a local `metrics-test*` with divergent shapes. A follow-up ticket tracks extending the Scout fixture to cover this scenario end-to-end. - **Ticket repro, end-to-end.** `TS metrics-test*`: open Dimensions picker and see all dimensions visible. - Select `environment` picker collapses to `environment` + `host.name`, `region` is no longer in the list. - Deselect: all three return. - **Rapid multi-select (throttled network).** Use DevTools to throttle the network, open the picker, click `environment` then attempt to click `region` before the first fetch returns. `region` must no longer be in the DOM by the time the second click fires, so the empty-grid state is unreachable via the picker. This exercises the optimistic filter's render-race behavior in a real browser, which unit tests can't simulate. ## Follow-ups - **Scout fixture extension** to reproduce the ticket end-to-end — the current generator spreads every dimension onto every metric, which defeats the scenario. elastic/observability-dev#5481 - **Orphan styling** — decide whether orphan selections should have a visual distinguisher beyond placement/checkmark. Separate ticket. --------- Co-authored-by: Lucas Francisco López <lucaslopezf@gmail.com>


Summary
Resolves #263309.
The Metrics in Discover dimension picker had three related problems:
This PR fixes all three.
METRICS_INFOresponse. This is the union ofdimensionFieldsacross the currently visible metrics.metricItemsplus the new local selection, hiding any dimension not carried by a metric that already has the selection. This makes non-applicable options disappear before the server round-trip, so rapid multi-select shouldn't reach an empty-grid state via the picker.Commit layout
1eb2379emerge_dimensions.ts,use_accumulated_dimensions.ts, related plumbing).290aaf17491456acHow this maps to the ticket
allDimensionsis now the union ofmetricItems[*].dimensionFieldsfrom the current response.Before / after
Before: Picker lists dimensions regardless of whether the user can meaningfully select them, and silently drops previously-selected dimensions if the filtered response no longer carries them.
After: Picker lists only dimensions that apply to the current grid, prevents selecting a dimension that would produce an empty grid, and never hides an already-selected dimension.
Test plan
Manual verification focuses on the two scenarios that can't be covered by the unit tests below. Applicable-set narrowing, orphan surfacing, count/tick consistency, and max-selection behavior are fully asserted in
dimensions_selector.test.tsx; refetch on context change is asserted inuse_fetch_metrics_data.test.ts.Dataset: TSDB indices with divergent dimension sets: e.g. one metric with dims
environment+host.name, another withhost.name+region. The repo's Scout fixture currently puts every dimension on every metric, so for manual verification use a localmetrics-test*with divergent shapes. A follow-up ticket tracks extending the Scout fixture to cover this scenario end-to-end.TS metrics-test*: open Dimensions picker and see all dimensions visible.environmentpicker collapses toenvironment+host.name,regionis no longer in the list.environmentthen attempt to clickregionbefore the first fetch returns.regionmust no longer be in the DOM by the time the second click fires, so the empty-grid state is unreachable via the picker. This exercises the optimistic filter's render-race behavior in a real browser, which unit tests can't simulate.Follow-ups