Skip to content

[9.5] fix: escape service name in APM linked dashboard filters (#291091) - #291180

Merged
kibanamachine merged 1 commit into
elastic:9.5from
kibanamachine:backport/9.5/pr-291091
Sep 15, 2026
Merged

kibanamachine merged 1 commit into
elastic:9.5from
kibanamachine:backport/9.5/pr-291091

Conversation

@kibanamachine

Copy link
Copy Markdown
Contributor

Backport

This will backport the following commits from main to 9.5:

Questions ?

Please refer to the Backport tool documentation

)

## Summary

Closes elastic#245023

Linking a custom dashboard to an APM service whose name contains a colon
(e.g. `unknown_service:java`) broke the Dashboards tab with `Expected
AND, OR, end of input but ":" found. service.name: unknown_service:java
(500)` — and once such a link existed, **no** service could show its
linked dashboards.

Two distinct defects:

1. **Unescaped KQL is persisted.** `save_dashboard_modal.tsx` stored ``
`${SERVICE_NAME}: ${serviceName}` `` — the service name comes straight
off the URL path param, so a colon (and also spaces, `(`, `)`, `"`, `*`,
`and`/`or`/`not`) produced invalid KQL. It's now stored as a quoted
phrase via `escapeQuotes`, matching the existing APM precedents in
`build_alert_filters.ts` and
`service_flyout/footer/hooks/use_alerts_href.ts`.

2. **One bad saved object failed the route for every service.** The read
route loads *all* linked dashboards globally and maps
`kqlQuery(dashboard.kuery)` over them to build a single msearch.
`kqlQuery` calls `fromKueryExpression`, which throws synchronously, so
`GET /internal/apm/services/{serviceName}/dashboards` 500'd regardless
of which service was being viewed. `getServicesWithDashboards` now
guards each dashboard individually: an unparseable stored filter is
skipped and logged instead of failing the request. The positional
response loop was replaced with a paired array so indices stay aligned
when a dashboard is dropped.

The rendering path was never at fault — `getFilters` builds DSL via
`buildPhraseFilter`/`buildExistsFilter` and is already colon-safe. The
stored `kuery` is only ever read server-side.

### Upgrade note

There is deliberately **no saved-object migration**. Unrelated services
recover immediately, but a dashboard linked to a colon-named service
*before* this fix keeps its broken stored filter and stays hidden until
it is re-linked.

## Testing

24 new Jest tests across three files:

- `common/utils/service_name_to_kuery.test.ts` — escaping for colons,
spaces, quotes, backslashes and KQL keywords, each asserted to survive
`fromKueryExpression`
- `server/routes/custom_dashboards/get_services_with_dashboards.test.ts`
— a malformed stored filter is skipped rather than thrown, unrelated
dashboards still returned, and response/dashboard indices stay aligned
when a middle entry is dropped
- `actions/save_dashboard_modal.test.tsx` — drives the modal and asserts
the persisted `kuery`

Reverting the source fix makes exactly 5 of these fail (the regression
cases); the 3 covering pre-existing behaviour still pass.

### Reproducing manually

A new synthtrace scenario generates colon-named services alongside a
plainly-named control service:

```
node scripts/synthtrace trace_with_service_names_with_colons --live
```

Link a dashboard to `synth-node`, then to `unknown_service:java`, and
confirm both still render. To exercise the bad-data path, set an
`apm-custom-dashboards` saved object's `kuery` to the unquoted form and
check the route returns 200 with a warning logged instead of 500.

<img width="1899" height="950" alt="image"
src="https://github.com/user-attachments/assets/03c6738c-7b27-4c0a-9f2d-eba21a7ef449"
/>

https://github.com/user-attachments/assets/d73dfca4-02c7-45d2-b744-88178f0714d5

The scenario emits only transactions, spans and errors. App metrics are
omitted on purpose: they route to
`metrics-apm.app.<service.name>-<namespace>`, and Elasticsearch rejects
a data stream name containing `:`, so those documents would all be
dropped. Aggregated metricsets derived from the transactions still
provide `processor.event: metric` coverage, which is what the dashboards
route needs.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit f4fe32c)
@kibanamachine kibanamachine added the backport This PR is a backport of another PR label Sep 15, 2026
@kibanamachine
kibanamachine enabled auto-merge (squash) September 15, 2026 14:48
@botelastic botelastic Bot added the Team:obs-presentation Focus: APM UI, Infra UI, Hosts UI, Universal Profiling, Obs Overview and left Navigation label Sep 15, 2026
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

Pinging @elastic/obs-presentation-team (Team:obs-presentation)

@kibanamachine kibanamachine added the reviewer:libra PR review with Libra. This disables Claude and Scout reviewers label Sep 15, 2026
@kibanamachine

Copy link
Copy Markdown
Contributor Author

Selected for Libra review

This PR was selected for Libra review as part of the temporary 50% trial.

To opt out permanently, remove the reviewer:libra label. It will not be added again to this PR.

@kibanamachine

Copy link
Copy Markdown
Contributor Author

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 2965 2966 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.3MB 3.3MB +40.0B

cc @jennypavlova

@kibanamachine
kibanamachine merged commit 3cc781c into elastic:9.5 Sep 15, 2026
120 of 121 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR reviewer:libra PR review with Libra. This disables Claude and Scout reviewers Team:obs-presentation Focus: APM UI, Infra UI, Hosts UI, Universal Profiling, Obs Overview and left Navigation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants