[data views] cache field caps requests #168910

mattkime · 2023-10-15T03:17:17Z

Summary

Field caps request caching - implement stale-while-revalidate cache headers and etags with 304 responses as appropriate.

This PR accomplishes

Adds 304 Not Modified to http server
Adds refresh button to data view management that force refreshes field list
Adds /internal/data_views/fields endpoint which supports caching.
- This is necessary since fields_for_wildcard doesn't support caching due to POST requests
- Adds stale-while-revalidate header directive UNLESS field list is empty.
Uses Vary header with hash of user id to force requests when user has changed.
Unchanged field list responses won't recreate data view field list

How to test

Pop open the dev tools to the network tab and make sure 'Disable cache' is unchecked. filter for 'fields' requests
Load more than one data set (sample data is fine)
Go to discover
Switch selected data views. Notice status code on first load vs subsequent loads
Open a new window, notice loading from cache
Push document that changes field list. You'll need to wait for cache to expire for it to load. The default is 5s.
Notice that field list loads after cache is old properly result in 304 response.
Set data_views:cache_max_age, shift reload. No field requests will be cached.

Note on Safari

Safari doesn't support the stale-while-revalidate directive. Additionally, the Safari dev console shows 304 responses as 200's. I figured this out by adding console statements to the fields endpoint. As best I can tell, Safari won't be performant on slow clusters but its also no slower than it was before this PR. Safari does respect the disabling of cache headers.

Release note

Data View field list requests are now cached with a stale-while-revalidate strategy. This means that after the initial field list request, all subsequent requests return the cached response which is very fast. If the cache is determined to be stale then the cache will update in the background and new data will be available on the next request.

This behavior can be modified via the data_views:cache_max_age Kibana advanced setting. Setting it to zero will disable the cache. All other values (in seconds) will be used to determine whether the cache is stale. The default value is 5 seconds.

The field list can be manually updated via the refresh button in data view management or a hard refresh with your browser.

Note for Safari: The stale-while-revalidate cache directive is unsupported, therefore it makes additional requests. If this is impacting Kibana performance then try Chrome or Firefox.

Data View Management

davismcphee

Thanks for looking into this, and it definitely feels like a step in the right direction. In my local testing, it seems to work reliably in some situations and less so in others. For example, Data View Management seems to fairly reliably cache the requests across page navigations and page reloads, but the same requests in Discover weren't being cached reliably. Even without a page reload, it seemed like the cache wasn't being used reliably for duplicate GET _fields_for_wildcard requests.

HTTP caching is pretty hard to understand and troubleshoot, but after looking into it I suspect this is at least partly related to cache validation requests on page reload. It's possible we can improve this using ETag and/or Last-Modified headers, but I'm not entirely sure.

Some possibly related SO questions I encountered while looking into this (adding here for future reference):

All of this makes me wonder if maybe we should be taking more control over the caching than what HTTP headers provide us. Even if we figure out the reload issue, the Cache-Control headers are still only a hint to the browser about what it's allowed to do, which it may choose to ignore anyway. I think exploring HTTP headers was a good idea and may still be the right approach in the end (and I think the stale-while-revalidate strategy is the way to go regardless of how we implement it), but for completeness I think it's also worth us exploring a custom stale-while-revalidate strategy that interacts directly with the browser cache through the cache API:

src/plugins/data_views/public/data_views/data_views_api_client.ts

src/plugins/data_views/server/rest_api_routes/internal/fields_for.ts

elasticmachine · 2023-10-31T08:27:58Z

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

kertal · 2023-10-31T08:33:54Z

Just did some basic testing with a dashboard containing 4 ES|QL based embeddables using the same index pattern, with Chrome using simulated slow network conditions, the fields fetching in our current iteration would take about 6s in this case

Initial loading, there is just 1 uncached request instead of 5, taking about 2s, the 4 follow up requests are cached and just take 1-2ms

So it already is much faster (about 2s instead of 6s)

When loading the dashboard again with a browser reload, all requests are cached, so instead of 10s it just takes a few milliseconds

…om:mattkime/kibana into fields_for_wildcard_stale_while_revalidate

pgayvallet

LGTM for Core changes, just one remark

packages/core/http/core-http-router-server-internal/src/response.ts

lukasolson

LGTM!

…om:mattkime/kibana into fields_for_wildcard_stale_while_revalidate

davismcphee

Tested locally one last time, and the remaining two issues I reported have been resolved -- thanks for updating the tests too 👍 Great work on this, and I think it's time to get this merged in. Thanks for all the work on it, and LGTM!

davismcphee · 2024-01-15T18:27:26Z

src/plugins/data_views/public/data_views/data_views_api_client.test.ts


-    expect(fetchSpy).toHaveBeenCalledWith(expectedPath, expect.any(Object));
+    expect(fetchSpy).toHaveBeenCalledWith(expectedPath, {
+      // not sure what asResponse is but the rest of the results are useful


I think asResponse just prevents the HTTP service from automatically parsing the response as JSON so we can deal with it in its raw form (which makes sense since we're doing ETags, etc.).

davismcphee · 2024-01-15T18:30:16Z

src/plugins/data_views/public/data_views/data_views_api_client.ts

@@ -104,7 +104,8 @@ export class DataViewsApiClient implements IDataViewsApiClient {
        allow_no_index: allowNoIndex,
        include_unmapped: includeUnmapped,
        fields,
-        allow_hidden: allowHidden,
+        // default to undefined to keep value out of URL params and improve caching
+        allow_hidden: allowHidden || undefined,


This approach is much nicer than addressing each caller separately 👍 no need to try to remember it in future changes.

kibana-ci · 2024-01-15T20:28:08Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 0493689

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #6 / useActionTypes should fetch action types

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/core-http-router-server-internal`	26	27	+1
`@kbn/core-http-server`	179	180	+1
`data`	2569	2579	+10
`dataViews`	257	273	+16
total			+28

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id	before	after	diff
`@kbn/core-http-router-server-internal`	6	7	+1

Async chunks

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

id	before	after	diff
`dataViewManagement`	135.4KB	136.3KB	+916.0B
`discover`	564.1KB	564.1KB	+3.0B
`eventAnnotationListing`	196.9KB	196.9KB	+3.0B
`lens`	1.4MB	1.4MB	+3.0B
total			+925.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`dataViews`	46.9KB	48.3KB	+1.4KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-http-router-server-internal`	26	27	+1
`@kbn/core-http-server`	454	457	+3
`data`	3220	3230	+10
`dataViews`	922	938	+16
total			+30

ESLint disabled line counts

id	before	after	diff
`dataViewManagement`	2	3	+1

Total ESLint disabled count

id	before	after	diff
`dataViewManagement`	2	3	+1

History

💛 Build #187366 was flaky a9315ac
💔 Build #187364 failed 244896e
💔 Build #187363 failed b12b865
💔 Build #187352 failed 194527e
💔 Build #187350 failed 9f2700d
💔 Build #187349 failed 416a785

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @mattkime

tonyghiani

logs-shared changes LGTM!

## Summary tldr; The implements a fallback sha1 method when using the browser in an insecure context. The data views fields requests cache need uniqueness across users. This has been implemented by hashing the user object into a HTTP header using sha1. Typically we can use the browser's built in crypto objects for this HOWEVER its not available in insecure contexts - https://www.chromium.org/blink/webcrypto/#accessing-it - this PR supplies a sha1 function for insecure contexts. How to test - when running kibana locally, it will run in a secure context via 127.0.0.1 or localhost. It will run in an insecure context at 0.0.0.0. Simply load some sample data and load a data view. follow up to #168910 Screenshot of error resolved by this pr - ![Visualize Visualize Reporting Screenshots Print PDF button becomes available whe-c57ca69f29465527c4079569a4548eb10fe0568302776500148260b299fbd5c4](https://github.com/elastic/kibana/assets/216176/d7bcec41-631f-426f-b209-87b2d6403f23) --------- Co-authored-by: kibanamachine <[email protected]>

…#175181) ## Summary tldr; The implements a fallback sha1 method when using the browser in an insecure context. The data views fields requests cache need uniqueness across users. This has been implemented by hashing the user object into a HTTP header using sha1. Typically we can use the browser's built in crypto objects for this HOWEVER its not available in insecure contexts - https://www.chromium.org/blink/webcrypto/#accessing-it - this PR supplies a sha1 function for insecure contexts. How to test - when running kibana locally, it will run in a secure context via 127.0.0.1 or localhost. It will run in an insecure context at 0.0.0.0. Simply load some sample data and load a data view. follow up to elastic#168910 Screenshot of error resolved by this pr - ![Visualize Visualize Reporting Screenshots Print PDF button becomes available whe-c57ca69f29465527c4079569a4548eb10fe0568302776500148260b299fbd5c4](https://github.com/elastic/kibana/assets/216176/d7bcec41-631f-426f-b209-87b2d6403f23) --------- Co-authored-by: kibanamachine <[email protected]>

mattkime changed the title ~~add stale while revalidate~~ [data views] cache field caps requests Oct 20, 2023

mattkime mentioned this pull request Oct 20, 2023

Cross Cluster search causes UI to hang while getting cross cluster field names/types #167706

Closed

kertal marked this pull request as ready for review October 20, 2023 13:16

kertal requested a review from a team as a code owner October 20, 2023 13:16

davismcphee reviewed Oct 23, 2023

View reviewed changes

kertal added the Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. label Oct 31, 2023

kertal assigned mattkime Oct 31, 2023

kertal mentioned this pull request Oct 31, 2023

[DataViews] Add caching for the request of fields #169622

Closed

mattkime added 4 commits October 31, 2023 22:25

Merge branch 'fields_for_wildcard_stale_while_revalidate' of github.c…

5dc9eee

…om:mattkime/kibana into fields_for_wildcard_stale_while_revalidate

Merge branch 'main' into fields_for_wildcard_stale_while_revalidate

797263d

caching fields requests mostly working

691df4f

implement advanced setting for cache length AND etag headers

609a962

mattkime requested a review from a team as a code owner November 2, 2023 03:08

add mock

2aec419

mattkime added Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:enhancement labels Nov 2, 2023

attempt to fix discover alerting functional test

2abf10e

pgayvallet approved these changes Nov 2, 2023

View reviewed changes

packages/core/http/core-http-router-server-internal/src/response.ts Outdated Show resolved Hide resolved

mattkime added 5 commits November 2, 2023 08:55

304 sohuldn't accept body

4de45b5

try setting cache to zero to see if fewer tests fail

8d8fbb4

remove some comments

146ba0b

Merge branch 'main' into fields_for_wildcard_stale_while_revalidate

567e250

fix tests

830b959

mattkime requested review from a team as code owners November 5, 2023 02:41

mattkime requested review from pzl and dasansol92 November 5, 2023 02:41

restore default cache max-age

9638c36

lukasolson approved these changes Jan 12, 2024

View reviewed changes

mattkime added 7 commits January 13, 2024 20:21

better caching, with test

416a785

Merge branch 'main' into fields_for_wildcard_stale_while_revalidate

9f2700d

Merge branch 'main' into fields_for_wildcard_stale_while_revalidate

194527e

Merge branch 'main' into fields_for_wildcard_stale_while_revalidate

b12b865

undo removal of version header for hasUserDataView

99356b3

Merge branch 'fields_for_wildcard_stale_while_revalidate' of github.c…

244896e

…om:mattkime/kibana into fields_for_wildcard_stale_while_revalidate

fix test

a9315ac

davismcphee approved these changes Jan 15, 2024

View reviewed changes

kertal requested a review from tonyghiani January 15, 2024 18:56

mattkime removed request for dej611 and tonyghiani January 15, 2024 19:05

Merge branch 'main' into fields_for_wildcard_stale_while_revalidate

0493689

mattkime requested a review from tonyghiani January 15, 2024 19:50

mattkime requested review from tonyghiani and removed request for tonyghiani January 15, 2024 20:29

tonyghiani approved these changes Jan 16, 2024

View reviewed changes

mattkime merged commit acdaf01 into elastic:main Jan 16, 2024
40 checks passed

kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Jan 16, 2024

mattkime mentioned this pull request Jan 21, 2024

[data views] implement fallback sha1 hash for fields request #175181

Merged

nkhristinin mentioned this pull request Feb 1, 2024

[Security Solution][Sourcerer] Addressing sourcerer performance #172534

Open

davismcphee mentioned this pull request Nov 27, 2024

Discover: Selected field shows with wrong icon when switching from unmapped to mapped #201751

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[data views] cache field caps requests #168910

[data views] cache field caps requests #168910

mattkime commented Oct 15, 2023 •

edited

Loading

davismcphee left a comment •

edited

Loading

elasticmachine commented Oct 31, 2023

kertal commented Oct 31, 2023 •

edited

Loading

pgayvallet left a comment

lukasolson left a comment

davismcphee left a comment

davismcphee Jan 15, 2024

davismcphee Jan 15, 2024

kibana-ci commented Jan 15, 2024

API count

ESLint disabled line counts

Total ESLint disabled count

tonyghiani left a comment

[data views] cache field caps requests #168910

[data views] cache field caps requests #168910

Conversation

mattkime commented Oct 15, 2023 • edited Loading

Summary

How to test

Note on Safari

Release note

davismcphee left a comment • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Oct 31, 2023

kertal commented Oct 31, 2023 • edited Loading

pgayvallet left a comment

Choose a reason for hiding this comment

lukasolson left a comment

Choose a reason for hiding this comment

davismcphee left a comment

Choose a reason for hiding this comment

davismcphee Jan 15, 2024

Choose a reason for hiding this comment

davismcphee Jan 15, 2024

Choose a reason for hiding this comment

kibana-ci commented Jan 15, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Public APIs missing comments

Any counts in public APIs

Async chunks

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

tonyghiani left a comment

Choose a reason for hiding this comment

mattkime commented Oct 15, 2023 •

edited

Loading

davismcphee left a comment •

edited

Loading

kertal commented Oct 31, 2023 •

edited

Loading