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

[Reporting/Discover/Search Sessions] Only use relative time filter when generating share data #112588

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Sep 20, 2021

Summary

Closes #112562

After merging #110459 we introduced a regression into restoring search sessions with relative time on discover. This contribution scopes creating a relative time filter to reporting only. This way we preserve the fix for the Copy URL and scheduled reports and address the regression.

Release note

We fixed a bug where restoring search sessions in discover would not preserve absolute times thus invalidate search session caches.

Checklist

CC @tsullivan @Dosant @lukasolson @ppisljar

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@jloleysens jloleysens requested review from kertal and Dosant September 20, 2021 17:25
@jloleysens
Copy link
Contributor Author

Input regarding tests to add/update would be much appreciated!

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM. This is the right way to solve this. Much appreciation for jumping on this so quickly!

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

kibanamachine and others added 2 commits September 21, 2021 04:12
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked that restoring relative session works.

There is an existing functional test that should have caught this bug:

it('relative timerange works', async () => {
await PageObjects.common.navigateToApp('discover');
await PageObjects.header.waitUntilLoadingHasFinished();
await searchSessions.save();
await searchSessions.expectState('backgroundCompleted');
const searchSessionId = await getSearchSessionId();
expect(await PageObjects.discover.hasNoResults()).to.be(true);
log.info('searchSessionId', searchSessionId);
// load URL to restore a saved session
await PageObjects.searchSessionsManagement.goTo();
const searchSessionList = await PageObjects.searchSessionsManagement.getList();
// navigate to Discover
await searchSessionList[0].view();
await PageObjects.header.waitUntilLoadingHasFinished();
await searchSessions.expectState('restored');
expect(await PageObjects.discover.hasNoResults()).to.be(true);
});
});

I will check separately why it didn't catch it and will try to fix

@jloleysens jloleysens requested a review from a team as a code owner September 21, 2021 08:55
@jloleysens jloleysens requested a review from a team as a code owner September 21, 2021 10:59
Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, code owners review, didn't test

@jloleysens jloleysens added the auto-backport Deprecated - use backport:version if exact versions are needed label Sep 21, 2021
Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

We can remove this TODO: 606d1b8#diff-0154cfbb823111800ea5acf098c44670bcf6035935df851983db9f2c55bfe16cR93 but I'll take care of that later.

@tsullivan tsullivan merged commit 25bf795 into elastic:master Sep 21, 2021
tsullivan added a commit to tsullivan/kibana that referenced this pull request Sep 21, 2021
…en generating share data (elastic#112588)

* only use relative time filter when generating share data

* Added comment on absolute time filter.

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

* improve discover search session relative time range test

* update discover tests and types for passing in data plugin to getSharingData function

* updated reporting to pass in data plugin to getSharingData, also updates jest tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Sep 21, 2021
…en generating share data (elastic#112588)

* only use relative time filter when generating share data

* Added comment on absolute time filter.

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

* improve discover search session relative time range test

* update discover tests and types for passing in data plugin to getSharingData function

* updated reporting to pass in data plugin to getSharingData, also updates jest tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x
7.14 Commit could not be cherrypicked due to conflicts
7.15 Commit could not be cherrypicked due to conflicts

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 112588

tsullivan added a commit to tsullivan/kibana that referenced this pull request Sep 21, 2021
…en generating share data (elastic#112588)

* only use relative time filter when generating share data

* Added comment on absolute time filter.

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

* improve discover search session relative time range test

* update discover tests and types for passing in data plugin to getSharingData function

* updated reporting to pass in data plugin to getSharingData, also updates jest tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
tsullivan added a commit to tsullivan/kibana that referenced this pull request Sep 21, 2021
…en generating share data (elastic#112588)

* only use relative time filter when generating share data

* Added comment on absolute time filter.

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

* improve discover search session relative time range test

* update discover tests and types for passing in data plugin to getSharingData function

* updated reporting to pass in data plugin to getSharingData, also updates jest tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
tsullivan added a commit that referenced this pull request Sep 21, 2021
…en generating share data (#112588) (#112735)

* only use relative time filter when generating share data

* Added comment on absolute time filter.

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

* improve discover search session relative time range test

* update discover tests and types for passing in data plugin to getSharingData function

* updated reporting to pass in data plugin to getSharingData, also updates jest tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>

Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
kibanamachine added a commit that referenced this pull request Sep 21, 2021
…en generating share data (#112588) (#112736)

* only use relative time filter when generating share data

* Added comment on absolute time filter.

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

* improve discover search session relative time range test

* update discover tests and types for passing in data plugin to getSharingData function

* updated reporting to pass in data plugin to getSharingData, also updates jest tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>

Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
tsullivan added a commit that referenced this pull request Sep 22, 2021
…lter when generating share data (#112588) (#112737)

* [Reporting/Discover/Search Sessions] Only use relative time filter when generating share data (#112588)

* only use relative time filter when generating share data

* Added comment on absolute time filter.

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

* improve discover search session relative time range test

* update discover tests and types for passing in data plugin to getSharingData function

* updated reporting to pass in data plugin to getSharingData, also updates jest tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>

* fixes

* fix ts

Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
tsullivan added a commit that referenced this pull request Sep 22, 2021
…lter when generating share data (#112588) (#112740)

* [Reporting/Discover/Search Sessions] Only use relative time filter when generating share data (#112588)

* only use relative time filter when generating share data

* Added comment on absolute time filter.

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>

* improve discover search session relative time range test

* update discover tests and types for passing in data plugin to getSharingData function

* updated reporting to pass in data plugin to getSharingData, also updates jest tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>

* fix bad merge

* fixes

* fix

Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
@jloleysens jloleysens deleted the reporting/discover-fix-relative-time-range-for-search-sessions branch September 22, 2021 08:14
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
discover 378.6KB 378.7KB +177.0B

Page load bundle

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

id before after diff
reporting 40.0KB 40.1KB +109.0B

History

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

@sophiec20 sophiec20 added the (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Feature:Discover Discover Application release_note:fix v7.14.2 v7.15.1 v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Search Sessions][Discover] Search sessions with relative time range won't restore in discover
7 participants