[Discover/Reporting] Fix export that does not contain relative time filter #110459
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jloleysens
added
Feature:Discover
We should check with @ppisljar and @lukasolson, who might know this answer. Maybe we still have an opportunity to fix this bug in SearchSource's behavior - it just needs to keep track of the relative time that was given as input. That would mean not changing Reporting code, and making SearchSource more friendly to any sharing service that uses it. However, this is a really bad bug and users have been waiting a long time for a fix. If it's not feasible to work this out in SearchSource then the direction @jloleysens is going with this PR is acceptable to me. |
ppisljar
requested changes
Aug 31, 2021
There was a problem hiding this comment.
We need to make sure that when we serialize searchsource we can restore it on the server and we shouldn't need to do any other tricks to make that work.
We shouldn't be doing any special handling in reporting. That gives reporting way too much internal knowledge of how searches are performed and it will quickly get us back to the csv export v1 days.
|
here is the issue describing our conversion to absolute: #88480 |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
Pinging @elastic/kibana-app-services (Team:AppServices) |
|
Pinging @elastic/kibana-reporting-services (Team:Reporting Services) |
qn895
approved these changes
Sep 13, 2021
|
ML change LGTM 🎉 |
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Sep 14, 2021
…ilter (elastic#110459) * version 1 of fix: we set the time range on the search source at CSV generation time * updated jest tests and updated API for getSharingData * make time range optional for getSharingData * pivot to updating "getTime" functionality by introducing a new flag * update jest snapshots * update comment * refactored coerceToAbsoluteTime -> coerceRelativeTimeToAbsoluteTime and updated behaviour to be more specific * fix jest test * do not change createFilter API, rather create new createRelativeFilter API, also only use this in one place in discover * update jest tests * update mock * update jest test mock Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Sep 14, 2021
…ilter (elastic#110459) * version 1 of fix: we set the time range on the search source at CSV generation time * updated jest tests and updated API for getSharingData * make time range optional for getSharingData * pivot to updating "getTime" functionality by introducing a new flag * update jest snapshots * update comment * refactored coerceToAbsoluteTime -> coerceRelativeTimeToAbsoluteTime and updated behaviour to be more specific * fix jest test * do not change createFilter API, rather create new createRelativeFilter API, also only use this in one place in discover * update jest tests * update mock * update jest test mock Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/data/common/query/timefilter/get_time.test.ts
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Sep 14, 2021
…ilter (elastic#110459) * version 1 of fix: we set the time range on the search source at CSV generation time * updated jest tests and updated API for getSharingData * make time range optional for getSharingData * pivot to updating "getTime" functionality by introducing a new flag * update jest snapshots * update comment * refactored coerceToAbsoluteTime -> coerceRelativeTimeToAbsoluteTime and updated behaviour to be more specific * fix jest test * do not change createFilter API, rather create new createRelativeFilter API, also only use this in one place in discover * update jest tests * update mock * update jest test mock Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/data/common/query/timefilter/get_time.test.ts # src/plugins/data/common/query/timefilter/get_time.ts
jloleysens
added a commit
that referenced
this pull request
Sep 14, 2021
…ilter (#110459) (#112088) * version 1 of fix: we set the time range on the search source at CSV generation time * updated jest tests and updated API for getSharingData * make time range optional for getSharingData * pivot to updating "getTime" functionality by introducing a new flag * update jest snapshots * update comment * refactored coerceToAbsoluteTime -> coerceRelativeTimeToAbsoluteTime and updated behaviour to be more specific * fix jest test * do not change createFilter API, rather create new createRelativeFilter API, also only use this in one place in discover * update jest tests * update mock * update jest test mock Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jloleysens
added a commit
that referenced
this pull request
Sep 14, 2021
…ilter (#110459) (#112089) * version 1 of fix: we set the time range on the search source at CSV generation time * updated jest tests and updated API for getSharingData * make time range optional for getSharingData * pivot to updating "getTime" functionality by introducing a new flag * update jest snapshots * update comment * refactored coerceToAbsoluteTime -> coerceRelativeTimeToAbsoluteTime and updated behaviour to be more specific * fix jest test * do not change createFilter API, rather create new createRelativeFilter API, also only use this in one place in discover * update jest tests * update mock * update jest test mock Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/data/common/query/timefilter/get_time.test.ts
kibanamachine
added
the
backport missing
|
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
2 similar comments
|
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
|
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
kibanamachine
removed
the
backport missing
jloleysens
added a commit
that referenced
this pull request
Sep 20, 2021
… time filter (#110459) (#112090) * [Discover/Reporting] Fix export that does not contain relative time filter (#110459) * version 1 of fix: we set the time range on the search source at CSV generation time * updated jest tests and updated API for getSharingData * make time range optional for getSharingData * pivot to updating "getTime" functionality by introducing a new flag * update jest snapshots * update comment * refactored coerceToAbsoluteTime -> coerceRelativeTimeToAbsoluteTime and updated behaviour to be more specific * fix jest test * do not change createFilter API, rather create new createRelativeFilter API, also only use this in one place in discover * update jest tests * update mock * update jest test mock Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/data/common/query/timefilter/get_time.test.ts # src/plugins/data/common/query/timefilter/get_time.ts * fix types and linting Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
1 task
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/visualize/_tag_cloud·ts.visualize app visualize ciGroup12 tag cloud chart should collapse the sidebarStandard OutStack TraceKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/dashboard/full_screen_mode·ts.dashboard app using current data full screen mode available in view modeStandard OutStack TraceKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/visualize/_data_table·ts.visualize app visualize ciGroup9 data table should allow applying changed paramsStandard OutStack Traceand 21 more failures, only showing the first 3. Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
This was referenced Oct 6, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Closes #22261
This PR updates Discover and Reporting to work together in a way that allows for the use of relative time ranges to be set in reports.
The current implementation adds new functionality to the
get_time.ts. A new functiongetRelativeTimewas added that will allow relative time values, likenow-1m, to be unchanged in the generatedSearchSource. ThegetTimefunction is unchanged to prevent issues with other consumers that may rely on absolute time.How to test
kibana_sample_data_flights)nowtonow-2m. This is a fairly small window for testing purposes.kibana_sample_data_flightsby running:Sharemenu request a new CSV reporttimeRangevalue is present and that there are no absolute time values.Release note
We fixed an issue where the URL for generating discover reports only contained absolute time values despite users inputting a relative time value.
Checklist