-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[APM] Don't fetch dynamic index pattern in setupRequest #70308
[APM] Don't fetch dynamic index pattern in setupRequest #70308
Conversation
We don't need a dynamic index pattern for parsing the filters from the query bar. Additionally, instead of fetching uiIndices in `getParamsForSearchRequest`, we can use `indices` that we already fetched in `setupRequest`.
Pinging @elastic/apm-ui (Team:apm) |
return; | ||
} | ||
|
||
const ast = esKuery.fromKueryExpression(kuery); | ||
return esKuery.toElasticsearchQuery(ast, indexPattern) as ESFilter; | ||
return esKuery.toElasticsearchQuery(ast) as ESFilter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this not needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it by entering various searches in the query bar, and using the contextual filters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It says:
* IndexPattern isn't required, but if you pass one in, we can be more intelligent
* about how we craft the queries (e.g. scripted fields)
Sounds like it should be okay for our use case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I think I've seen that before and wondered what that actually meant :D
If it's just scripted fields and you did the manual testing it's probably fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm if index pattern is not needed for toElasticsearchQuery
@elasticmachine merge upstream |
…kibana into remove-dynamic-index-pattern
💚 Build SucceededBuild metrics
History
To update your PR or re-run it, just comment with: |
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* master: Update known-plugins.asciidoc (elastic#69370) [ML] Anomaly Explorer swim lane pagination (elastic#70063) [Ingest Manager] Update asset paths to use _ instead of - (elastic#70320) Fix discover, tsvb and Lens chart theming issues (elastic#69695) Allow Saved Object type mappings to set a field's `doc_values` property (elastic#70433) [S&R] Support data streams (elastic#68078) [Maps] Add styling and tooltip support to mapbox mvt vector tile sources (elastic#64488) redirect to default app if hash can not be forwarded (elastic#70417) [APM] Don't fetch dynamic index pattern in setupRequest (elastic#70308) [Security_Solution][Endpoint] Leveraging msearch and ancestry array for resolver (elastic#70134) Update docs for api authentication usage (elastic#66819) chore(NA): disable alerts_detection_rules cypress suites (elastic#70577) add getVisibleTypes API to SO type registry (elastic#70559)
We don't need a dynamic index pattern for parsing the filters from the query bar. Additionally, instead of fetching uiIndices in
getParamsForSearchRequest
, we can useindices
that we already fetched insetupRequest
.