Skip to content
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

Reduce requests for all fields #184441

Closed
7 of 10 tasks
mattkime opened this issue May 29, 2024 · 13 comments
Closed
7 of 10 tasks

Reduce requests for all fields #184441

mattkime opened this issue May 29, 2024 · 13 comments
Assignees
Labels
impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.

Comments

@mattkime
Copy link
Contributor

mattkime commented May 29, 2024

Field caps requests have historically been treated as free but with large datasets that can be slow, large, and increase the cost of operations. There are a number of places in the Kibana codebase where all fields are loaded - these should be reviewed.

  1. How often is a given call made?

For instance, alerts may run every couple of minutes and are therefore highest priority for improvement. Next, it could be called by a common dashboard like view - still being run many times a day and might be slow for the user. Last, they might only be called when creating something. These would be lowest priority and could be treated like ordinary tech debt.

  1. Is it necessary to load all fields? If so, why?

Might be nice to add a comment in the codebase.

Low priority for optimization

  • x-pack/plugins/enterprise_search/server/lib/search_applications/field_capabilities.ts:39 @elastic/enterprise-search-frontend
  • x-pack/plugins/security/server/routes/indices/get_fields.ts:24 - this is hitting getFieldMapping but its probably still worth the conversation @elastic/kibana-security
  • x-pack/plugins/rollup/server/routes/api/indices/register_validate_index_pattern_route.ts:74 @elastic/kibana-management
  • x-pack/plugins/search_playground/server/lib/fetch_query_source_fields.ts:58 @elastic/enterprise-search-frontend
  • x-pack/packages/ml/aiops_log_rate_analysis/queries/fetch_index_info.ts:46 @elastic/ml-ui
  • x-pack/plugins/observability_solution/apm/server/routes/correlations/queries/fetch_duration_field_candidates.ts:58 @elastic/obs-ux-infra_services-team
  • x-pack/plugins/ml/server/models/job_service/new_job_caps/field_service.ts:74 @elastic/ml-ui
  • x-pack/plugins/transform/server/routes/api/transforms_preview/route_handler.ts:37 @elastic/ml-ui

Higher priority for optimization

  • x-pack/plugins/observability_solution/infra/server/lib/alerting/common/utils.ts:199 @elastic/obs-ux-logs-team @elastic/obs-ux-infra_services-team
  • x-pack/plugins/observability_solution/observability/server/lib/rules/custom_threshold/utils.ts:133 @elastic/obs-ux-management-team
@mattkime mattkime added the Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. label May 29, 2024
@mattkime mattkime self-assigned this May 29, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@legrego
Copy link
Member

legrego commented May 29, 2024

x-pack/plugins/security/server/routes/indices/get_fields.ts:24 - this is hitting getFieldMapping but its probably still worth the conversation @elastic/kibana-security

We would love to use field caps here, but we need the ability to detect runtime fields, which I don't think fieldcaps supports.
Edit: I've opened elastic/elasticsearch#109182 to track this request.

How often is a given call made?

This call is made when configuring Field-Level Security ("FLS") within a role definition. Creating/updating roles is an administrative operation, and shouldn't occur too frequently.

Is it necessary to load all fields? If so, why?

We provide the list of fields within a combobox to allow administrators to select which fields to grant/restrict access to. If there's a way for us to page/search these results, then we'd be happy to consider doing so.

We would love a more performant option. We've had issues in the past with this feature (#47378), to which we've applied band-aids.

@yuliacech
Copy link
Contributor

x-pack/plugins/rollup/server/routes/api/indices/register_validate_index_pattern_route.ts:74 is called to validate an index pattern when creating a rollup job via Kibana UI. Since Rollups have been deprecated in 8.11, I think this UI will be deprecated/removed from Kibana some time soon too. We don't have concrete plans for this work yet, here is a placeholder issue while we are working with design on that.

@joemcelroy
Copy link
Member

x-pack/plugins/search_playground/server/lib/fetch_query_source_fields.ts:58 @elastic/enterprise-search-frontend is called when the user chooses an index to determine what type of fields are in the indices chosen (the user can choose one or more indices to search on).

Its called on change of index and shouldn't be called too frequently. Its cached on client side and the call is only made when the indices change (user action).

The indices chosen are typically indices for search application data and tend to have smaller number of fields.

@sphilipse
Copy link
Member

x-pack/plugins/enterprise_search/server/lib/search_applications/field_capabilities.ts:39:

Fetched when viewing a Search Application or its documents in the Search Application Documents Explorer. We use it to show the user which fields are present in the index (which we should just use mappings for), but we also use it to show the user which fields have conflicts. Search Applications are effectively index aliases with multiple backing indices, so having fields with the same name but different types in both indices are a common source of issues preventing Search Applications from working correctly. This call shouldn't happen very often as we don't see users going back to their search application after creation a lot.

@joemcelroy AFAICT we don't actually do anything with the capabilities in the creation step for Search Applications, just the list of fields. We can just use mappings for that instead if it's cheaper, but probably not worth the effort.

@mattkime
Copy link
Contributor Author

Thank you @legrego @yuliacech @joemcelroy @sphilipse - exactly what I was asking for!

@benakansara
Copy link
Contributor

x-pack/plugins/observability_solution/infra/server/lib/alerting/common/utils.ts:199 @elastic/obs-ux-logs-team @elastic/obs-ux-infra_services-team
x-pack/plugins/observability_solution/observability/server/lib/rules/custom_threshold/utils.ts:133 @elastic/obs-ux-management-team

We are using fieldCaps to check if a particular field is present before using it in terms aggregation to get additional context from source documents for the alert. Right now, we are using it only to check container.id field, but the idea was to be able to use more fields in future in aggregation to get more context about alert. If we can specify the fields directly instead of using * to check if those fields are present in index, and if that's more efficient, we can do that change.

@mattkime
Copy link
Contributor Author

If we can specify the fields directly instead of using * to check if those fields are present in index, and if that's more efficient, we can do that change.

@benakansara That would be much appreciated.

@kertal kertal added the impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. label Jun 3, 2024
@jgowdyelastic
Copy link
Member

jgowdyelastic commented Jun 3, 2024

Regarding x-pack/plugins/ml/server/models/job_service/new_job_caps/field_service.ts

How often is a given call made?

It called once when the user starts the wizard to create an new anomaly detection or data frame analytics job, so pretty rare.

Is it necessary to load all fields? If so, why?

We match up fields with supported ML functions (like aggregations) to allow the user to select one for a AD job detector or DFA job.
e.g.
image

Which creates the detector:

"detectors": [
  {
    "function": "mean",
    "field_name": "responsetime"
   }
]

Regarding x-pack/plugins/transform/server/routes/api/transforms_preview/route_handler.ts

How often is a given call made?

It is called a few times during the use of the new transform wizard creating a Latest transform. I would guess on average 2 or 3 times while the user is making changes to the configuration.

Is it necessary to load all fields? If so, why?

We need to preview the destination index which will have the same mappings as the source index.

@smith
Copy link
Contributor

smith commented Jun 4, 2024

@mattkime my team owns x-pack/plugins/observability_solution/apm/server/routes/correlations/queries/fetch_duration_field_candidates.ts:58, but that code was written by the ML UI team, so they might be able to update it along with the other items.

x-pack/plugins/observability_solution/infra/server/lib/alerting/common/utils.ts:199 can probably also be handled by @elastic/obs-ux-management-team

@walterra
Copy link
Contributor

walterra commented Jun 10, 2024

x-pack/plugins/observability_solution/apm/server/routes/correlations/queries/fetch_duration_field_candidates.ts

This code is used as part of the correlation analysis feature for Latency Correlations and Failed Transaction Correlations.

How often is a given call made?

It's not used in any kind of automation. The call will be made only once when a user runs correlation analysis. We need to run field caps to identify fields suitable for the analysis.

image

Is it necessary to load all fields? If so, why?

We need to fetch all fields because it's necessary as part of the analysis to identify which fields are suitable for analysis. However, I noticed when we originally implemented this, filtering of field types was not available. It was introduced in 8.2 (https://www.elastic.co/guide/en/elasticsearch/reference/current/release-notes-8.2.0.html) and we could update the existing code to make use of this. Create an issue to track this: #185875

Update 8.15: We merged some optimizations in #186182

@walterra
Copy link
Contributor

x-pack/packages/ml/aiops_log_rate_analysis/queries/fetch_index_info.ts:46

This code is used as part of the AIOps Log Rate Analysis feature.

How often is a given call made?

It's not used in any kind of automation. The call will be made only once when a user runs log rate analysis. We need to run field caps to identify fields suitable for the analysis.

Is it necessary to load all fields? If so, why?

We need to fetch all fields because it's necessary as part of the analysis to identify which fields are suitable for analysis. In 8.14 we started to make use of filters to limit the scope of the query: #181109

The following parameters were added to improve the call:

  • index_filter: Adds a range filter to only get field caps from indices spanning the deviation time range.
  • filters: -metadata was added to not return fields like _id and esp. _tier. We previously had a manually check for _tier which is now unnecessary using this option.
  • types: Previously we fetched all field types and then filtered out the ones we don't support. This option allows us to pass in the supported fields right away and not return unsupported ones in the first place.

@mattkime
Copy link
Contributor Author

mattkime commented Jul 9, 2024

I'm closing this issue since all the high priority items have been completed and the low priority items can be treated like normal tech debt if any change is appropriate.

@mattkime mattkime closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
Projects
None yet
Development

No branches or pull requests