[Logs UI] Use the Super date picker in the log stream by afgomez · Pull Request #54280 · elastic/kibana

afgomez · 2020-01-08T16:58:31Z

Summary

This PR replaces the point-in-time datepicker from the Logs UI with the EuiSuperDatePicker to allow users to specify a range.

Changes

LogSummaryState and LogEntriesState now keep track of the date range. We represent the range with two pairs of values:

startDateExpression, endDateExpression: the datemath expression (i.e. now-10m, now)
startTimestamp, endTimestamp: the resolved datemath expression, in milliseconds.

The endTimestamp is dynamic when endDate is now. This allows the streaming to always fetch the most recent entries.

The minimap now has a fixed range, which corresponds to the startDate and endDate. The minimap is no longer drag-able anymore either.

LogSummaryState has now two elements to consider: the date range and the log position cursor. They can be set with different callbacks and each change might trigger a refetching of entries. That means that if the user has both values (start and end dates, and a position) set in the initial URL, the page will fetch twice. To prevent this, we have added a flag to the LogSummaryState that determines if it has been initialized or not. If it's not initialized, consumers of the API should not do anything.

The GraphQL implementation of the API has been removed.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
(https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2020-02-07T14:17:06Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 6dddc6b8c000e258a0c9b6b7908430c71ac3c3b8

Test Failures

Kibana Pipeline / x-pack-intake-agent / X-Pack Jest Tests.x-pack/legacy/plugins/infra/public/containers/logs/log_summary.useLogSummary hook queries for new summary buckets when the start and end date changes

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

Stack Trace

Error: expect(jest.fn()).toHaveBeenLastCalledWith(...expected)

Expected: ObjectContaining {"endTimestamp": 1581081970183, "startTimestamp": 1581081960182}
Received: {"bucketSize": 100, "endTimestamp": 1581081970183, "query": null, "sourceId": "SOURCE_ID", "startTimestamp": 1581081960183}

Number of calls: 1
    at Object.it (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/legacy/plugins/infra/public/containers/logs/log_summary/log_summary.test.tsx:133:33)

Kibana Pipeline / x-pack-intake-agent / X-Pack Jest Tests.x-pack/legacy/plugins/infra/public/components/logging/log_text_stream.LogEntryFieldColumn should output a

when displaying an Array of values

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

Stack Trace

Error: expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false
    at Object.it (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/legacy/plugins/infra/public/components/logging/log_text_stream/log_entry_field_column.test.tsx:35:36)
    at Object.asyncJestTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:102:37)
    at resolve (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:43:12)
    at new Promise (<anonymous>)
    at mapper (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:26:19)
    at promise.then (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:73:41)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Kibana Pipeline / x-pack-intake-agent / X-Pack Jest Tests.x-pack/legacy/plugins/infra/public/components/logging/log_text_stream.LogEntryFieldColumn should output just text when passed a non-Array

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

Stack Trace

Error: expect(received).toEqual(expected) // deep equality

Expected: "foo"
Received: "\"foo\""
    at Object.it (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/x-pack/legacy/plugins/infra/public/components/logging/log_text_stream/log_entry_field_column.test.tsx:92:30)
    at Object.asyncJestTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:102:37)
    at resolve (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:43:12)
    at new Promise (<anonymous>)
    at mapper (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:26:19)
    at promise.then (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-jasmine2/build/queueRunner.js:73:41)
    at process._tickCallback (internal/process/next_tick.js:68:7)

and 4 more failures, only showing the first 3.

History

💔 Build #24854 failed 6a899558e03dc9434dd7e4cd4cf21002769e8a70
💔 Build #24714 failed c839bf2876ed8f4a1b0f0e13b111e6a83804535f
💔 Build #24707 failed 8c1be37dab266d25a6182e2d3141d521b1b314b9
💔 Build #24605 failed 6ba0eaf62cb27c6c680845787e20e9bf31ac91d6
💔 Build #24561 failed 727bc3022317cf22f7020013d9d3852caa39d993

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

x-pack/legacy/plugins/infra/common/http_api/log_entries/entries.ts

x-pack/legacy/plugins/infra/public/containers/logs/log_entries/index.ts

x-pack/legacy/plugins/infra/public/containers/logs/log_highlights/log_highlights.tsx

x-pack/legacy/plugins/infra/public/containers/logs/log_position/log_position_state.ts

x-pack/legacy/plugins/infra/server/graphql/index.ts

elasticmachine · 2020-02-10T14:26:01Z

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

katrin-freihofner · 2020-02-11T09:38:19Z

I think it looks good in general. A few things that I found confusing:

The timestamps in the minimap seem off

...and...

When extending the timeframe the point in time I'm looking at should not be reset in my opinion.
When selecting a timestamp a year ago the loading seems broken. (I can see log lines appearing and then it is loading again....)
The "this week" time selection does not work for me.

katrin-freihofner · 2020-02-11T12:43:48Z

One more question came to my mind: with this change, the Stream live functionality is more hidden (date picker needs to be set to from X to now and then I have to scroll to the very bottom to click Stream live). @mukeshelastic any thoughts on this?

The UI uses `null` extensively. It's easier to allow `null` as a value in the API than to refactor the UI to use `undefined` instead of `null`, so let's go with that.

The new API uses io-ts so we have removed the assertions that involve type checking. If these would be false the test will still fail when decoding the response.

afgomez · 2020-03-17T10:53:06Z

@katrin-freihofner sorry for the late reply 🙈

For < 1280px screens:

Can we shrink the date picker further?

AFAIK no :(

We could definitely save some space with adjustments of the padding, no icons in the customize and highlights button

I think I'd go for no text instead of no icons, but maybe the UX becomes too bad?

I think the buttons will need to be moved out eventually. There's an issue somewhere about using an existing searchbar+datepicker component that exists in kibana already, and if we use it we won't be able to add the buttons in between the search bar and the date selector, so this might be a good chance to think where they could go

Have we ever discussed moving the date picker in the same row as the tabs?
I don't recall, but could be a solution. However, it will be in conflict with my previous sentence.

weltenwort

We're almost there...

x-pack/plugins/infra/public/pages/logs/stream/page_providers.tsx

Instead of using `useMemo` to update the timestamps we will explicitly set them up when the user selects a new date range or, if the `endTimestamp` is `now`, when the user scrolls down. This still enables infinite scroll and live streaming, but avoids a lot of unnecessary fetching.

weltenwort

I couldn't provoke any unnecessary refetches now. There's just one problem left with the streaming, it seems.

weltenwort · 2020-03-17T19:25:03Z

x-pack/plugins/infra/public/containers/logs/log_position/log_position_state.ts

+    startLiveStreaming: useCallback(() => {
+      setIsStreaming(true);
+      jumpToTargetPosition(null);
+      updateDateRange({ startDateExpression: 'now-1d', endDateExpression: 'now' });


Currently the live streaming doesn't fetch new entries.

I think the updateDateRange has to happen on a an interval (useInterval from react-use, for example), doesn't it? It looks like streamEntriesEffect in the log_entries/index.ts drives the timestamp change right now, which sounds the wrong way around. Shouldn't updating the time range on an interval should drive the loading effect? Then that streamEntriesEffect wouldn't have to await the timeout anymore, but could just react to timestamp changes as long as the isStreaming prop is true.

So I tried this locally and now I understand why you were hesitant to make such changes 🙈

So now my question becomes: If we revert that latest change about the timestamp calculation, can we commit to immediately start cleaning this up so we don't release something that puts a large burden on the Elasticsearch cluster for a simple browser scroll?

If yes, then it might be prudent to roll back to the continuous endTimestamp update and follow this up with a refactoring of the data flow. What do you think?

I think the updateDateRange has to happen on a an interval

I'm not sure. updateDateRange also refreshes the startTimestamp, shifting the range and forcing a reload (...I think, because the effect that triggers a full reload has a isStreaming condition somewhere).

It looks like streamEntriesEffect in the log_entries/index.ts drives the timestamp change right now, which sounds the wrong way around. Shouldn't updating the time range on an interval should drive the loading effect?

Yep, it's not obvious. The state for the streaming is handled in logPosition but the behaviour is handled in logEntries.

I wouldn't change it now. We can address this when we redo this data flow.

x-pack/plugins/infra/public/containers/logs/log_position/log_position_state.ts

kibanamachine · 2020-03-18T22:56:20Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7afe86c

History

💚 Build #34149 succeeded 5269c24
💚 Build #33857 succeeded af41535
💔 Build #33847 failed 780c7fc
💔 Build #33815 failed a146692
💔 Build #33463 failed 2737bb5

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

weltenwort

While I can't say that I feel totally confident into the data fetching logic (due to no fault of your work in this PR!), it seems to work pretty reliably and doesn't perform excessive refetches in my tests. I'm looking forward to the data flow cleanup. ✨

Thank you for sticking with it. ❤️ This was a very difficult change, but you mastered it. 👏

Zacqary

Glad to see this come together!

afgomez force-pushed the 49154-logs-super-date-picker branch 5 times, most recently from b082ee9 to f83254b Compare January 15, 2020 17:54

afgomez force-pushed the 49154-logs-super-date-picker branch from 077aed4 to 2be7fbe Compare January 16, 2020 15:36

afgomez mentioned this pull request Jan 17, 2020

[Logs UI] Fix z-index of logs page toolbar #54469

Merged

afgomez force-pushed the 49154-logs-super-date-picker branch from 2be7fbe to b21a2e9 Compare January 17, 2020 09:42

afgomez force-pushed the 49154-logs-super-date-picker branch from b21a2e9 to 02d8b59 Compare January 29, 2020 15:18

afgomez added v7.7.0 v8.0.0 labels Jan 29, 2020

afgomez force-pushed the 49154-logs-super-date-picker branch 4 times, most recently from c839bf2 to 6a89955 Compare February 6, 2020 09:04

afgomez force-pushed the 49154-logs-super-date-picker branch from 5856911 to 6dddc6b Compare February 7, 2020 12:54

elastic deleted a comment from kibanamachine Feb 7, 2020

afgomez force-pushed the 49154-logs-super-date-picker branch 2 times, most recently from b9e1fde to 4752771 Compare February 10, 2020 13:13

afgomez commented Feb 10, 2020

View reviewed changes

afgomez marked this pull request as ready for review February 10, 2020 14:23

afgomez requested a review from a team as a code owner February 10, 2020 14:23

afgomez added Feature:Logs UI Logs UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Feb 10, 2020

afgomez force-pushed the 49154-logs-super-date-picker branch from 4752771 to a8c7581 Compare February 10, 2020 14:29

Alejandro Fernández Gómez added 10 commits March 11, 2020 15:28

Allow null in some parts of the API

de97635

The UI uses `null` extensively. It's easier to allow `null` as a value in the API than to refactor the UI to use `undefined` instead of `null`, so let's go with that.

Tweak updateDateRange callback

db61f05

Restore test for default columns

81cf057

The new API uses io-ts so we have removed the assertions that involve type checking. If these would be false the test will still fail when decoding the response.

Restore logs_without_millis test

4586cdb

Merge branch 'master' into 49154-logs-super-date-picker

4241e02

Prevent overserializing

01aa571

Fix commit 01aa571

bb75f8f

Fix type errors introduced when merging upstream

2a85f3d

Tweak entry highlight fetching

2737bb5

Remove unused imports

18a9324

weltenwort reviewed Mar 17, 2020

View reviewed changes

x-pack/plugins/infra/public/pages/logs/stream/page_providers.tsx Outdated Show resolved Hide resolved

Alejandro Fernández Gómez added 3 commits March 17, 2020 15:47

Merge branch 'master' into 49154-logs-super-date-picker

780c7fc

Fix typo

af41535

afgomez force-pushed the 49154-logs-super-date-picker branch from d0897d9 to af41535 Compare March 17, 2020 16:08

weltenwort reviewed Mar 17, 2020

View reviewed changes

x-pack/plugins/infra/public/containers/logs/log_position/log_position_state.ts Outdated Show resolved Hide resolved

Alejandro Fernández Gómez added 3 commits March 18, 2020 13:07

Tweak refresh of endTimestamp

a806e3a

Expose centerCursor from logEntriesState

5269c24

Merge branch 'master' into 49154-logs-super-date-picker

7afe86c

weltenwort approved these changes Mar 19, 2020

View reviewed changes

Zacqary approved these changes Mar 19, 2020

View reviewed changes

afgomez merged commit fe4c164 into elastic:master Mar 19, 2020

afgomez deleted the 49154-logs-super-date-picker branch March 19, 2020 16:19

afgomez mentioned this pull request Mar 19, 2020

[7.x] [Logs UI] Use the Super date picker in the log stream (#54280) #60649

Merged

afgomez mentioned this pull request Mar 31, 2020

[DOCS] Show new date picker in logs UI docs #62019

Merged

This was referenced Jul 13, 2020

[Logs UI] Dataset field columns are erroneously being JSON-encoded #71415

Closed

[Logs UI] Fix display of dataset values in anomaly and category example rows #71693

Merged

Conversation

afgomez commented Jan 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Checklist

For maintainers

Uh oh!

elasticmachine commented Feb 7, 2020

💔 Build Failed

Test Failures

Standard Out

Stack Trace

Standard Out

Stack Trace

Standard Out

Stack Trace

History

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Feb 10, 2020

Uh oh!

katrin-freihofner commented Feb 11, 2020

Uh oh!

katrin-freihofner commented Feb 11, 2020

Uh oh!

afgomez commented Mar 17, 2020

Uh oh!

weltenwort left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weltenwort left a comment

Choose a reason for hiding this comment

Uh oh!

weltenwort Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

weltenwort Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

afgomez Mar 18, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Mar 18, 2020

💚 Build Succeeded

History

Uh oh!

weltenwort left a comment

Choose a reason for hiding this comment

Uh oh!

Zacqary left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

afgomez commented Jan 8, 2020 •

edited

Loading