[TSVB/Markdown] Introduce formatted date field label by sulemanof · Pull Request #75555 · elastic/kibana

sulemanof · 2020-08-20T13:29:59Z

Summary

This PR introduces a formatted date field label.
It is built on key_as_string bucket value, which is only inherent for fields with type date (as far as I know).

Release note

TSVB Mardown now handles the case when a field has key_as_string value.
Common case is the value is a date string (e.x. 2020-08-21T20:36:58.000Z) or a boolean stringified value ("true"/"false").
Such a value will be first converted into a moment object and formatted with dateFormat from Kibana UI settings.
If the key_as_string value is not recognized by a known format in Moments.js, a formatted value from elasticsearch will be returned.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
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
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

…_field_label

stratoula · 2020-08-21T07:26:06Z

src/plugins/vis_type_timeseries/public/application/components/lib/convert_series_to_vars.js

+        if (row.labelFormatted) {
+          set(
+            variables,
+            `${_.snakeCase(row.label)}.formatted`,


I was thinking of it more like label.formatted but maybe this is my personal opinion.

Specifying it as:

set(variables, `${_.snakeCase(row.label)}.label.formatted`, value)

will override the previous one since in that case the label would become as object type variable and should have next shape:

set(variables, _.snakeCase(row.label), { raw: row.label, foramtted: moment(row.labelFormatted).format(dateFormat) })

That means existing visualizations could be affected. So I chose this way as a safe one

Yes you are right, I am fine with it ❤️

stratoula · 2020-08-21T07:27:40Z

src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/get_splits.js

      return buckets.map((bucket) => {
        bucket.id = `${series.id}:${bucket.key}`;
        bucket.label = formatKey(bucket.key, series);
+        bucket.labelFormatted = bucket.key_as_string || '';


I have made some tests with various types and it seems that you are right, key_as_string is used for dates. @timroes do you have more input on this?

stratoula · 2020-08-21T07:29:35Z

@sulemanof thank you for that. I agree dates should be human-readable, I will create an issue for this 🍰

stratoula · 2020-08-21T10:03:14Z

And here is the issue #75634

stratoula

Assuming that key_as_string is used only for dates, I am fine with it! I have tested it on Safari, works perfectly and haven't detected any regressions. 🥇 Code LGTM

…_field_label

alexwizp

LGTM Thank you!

elasticmachine · 2020-08-24T10:50:46Z

Pinging @elastic/kibana-app (Team:KibanaApp)

markov00

@sulemanof @stratoula I think we already have this functionality in TSVB
you can access the formatted version of each bucket with something like:

{{# 1595980800000.data.formatted }}
{{[0]}}
{{/ 1595980800000.data.formatted }}

where [0] is the first element of the bucket, in this case the used label/timestamp.
You can also use this technique to in the each method

sulemanof · 2020-08-24T14:33:58Z

@sulemanof @stratoula I think we already have this functionality in TSVB
you can access the formatted version of each bucket with something like:
{{# 1595980800000.data.formatted }}
{{[0]}}
{{/ 1595980800000.data.formatted }}
where [0] is the first element of the bucket, in this case the used label/timestamp.
You can also use this technique to in the each method

The initial request is to have a formatted version of a bucket key, while the data is split by date field and have labels as epoch millis.
In such a case, your data will have an array of all existing time buckets, but the label represent one of top specified.
You're not exactly sure which exact bucket (by index from the data array) is your split one.
For more details, please look at initial issue.

UPD:
Here is an example of desired/first-bucket formatted label:

Notice the agg config is:

markov00

Thanks @sulemanof for the explanation on how to reproduce it, I've checked and this works under the standard use case.
The problem is that the key_as_string, use the format provided in the aggregation as a parameter (as described here), if not provided (as in TSVB) it use the first date format specified in the field mapping.
This means that if the user has a [date format] mappings (https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-date-format.html) that doesn't conform with what moment can digest, this field will be transformed into the following Invalid date string.

You can verify that with this simple example:

PUT my-index-000001
{
  "mappings": {
    "properties": {
      "timestamp": {
        "type":   "date"
      },
      "date": {
        "type":   "date",
         "format": "QQ-yyyy-M-D"
      },
      "value": {
        "type": "long"
      }
    }
  }
}
POST my-index-000001/_doc
{
  "timestamp": "2020-08-20T13:02:45.111Z",
  "date": "01-2020-8-5",
  "value": 100
}

GET my-index-000001/_search
{
  "aggs": {
    "dh": {
      "terms": {
        "field": "date"
      }
    }
  },
  "size": 0
}

We shouldn't probably directly rely on that key_as_string field or at least don't convert it right away, but check the field type used by the terms aggregation and then apply a valid format.
If we know that the key field is a date, we can apply the formatting to the key instead of key_as_string, but we should always check the key field type before doing any conversion

…_field_label

sulemanof · 2020-08-31T10:04:53Z

The PR was updated to format basic label with moment, instead of key_as_string from elasticsearch.
It also handles a case when a label is a boolean value:

src/plugins/vis_type_timeseries/public/application/components/lib/convert_series_to_vars.js

…_field_label

sulemanof · 2020-09-07T07:58:18Z

@elasticmachine merge upstream

kibanamachine · 2020-09-07T09:49:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: ed876c5

Build metrics

async chunks size

id	value	diff	baseline
visTypeTimeseries	1.8MB	+311.0B	1.8MB

History

💚 Build #72194 succeeded 3002691
💚 Build #71206 succeeded 1a10087
💚 Build #69940 succeeded ace52f4
💚 Build #69416 succeeded 3928f46
💔 Build #69386 failed 5ee5e11

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

Out of office for some time and all requested changes have been addressed

* Introduce formatted date field label * Apply changes * Use default format if can't parse, add comments Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

MakoWish · 2022-07-21T20:50:09Z

We just upgraded from 8.2.0 to 8.3.2, and it seems this PR was either removed or broken with the update. We are right back to where we were before this PR. There is no longer an option to display human-readable dates or boolean values again. My visualization that worked perfectly after this merge now gives an error:

Error processing your markdown
{{formatted}} is an unknown variable

Looking at the options for displaying the markdown, we are exactly back to how I described in Issue 75341

Eric

flash1293 · 2022-07-22T08:23:11Z

@MakoWish I can confirm there has been a regression - I opened a new issue to track it here: #136929

We will take care of it

sulemanof added 2 commits August 20, 2020 16:21

Introduce formatted date field label

5ee5e11

Merge remote-tracking branch 'upstream/master' into fix/markdown_date…

3928f46

…_field_label

sulemanof requested review from alexwizp, stratoula and timroes August 20, 2020 14:53

stratoula reviewed Aug 21, 2020

View reviewed changes

stratoula approved these changes Aug 21, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into fix/markdown_date…

ace52f4

…_field_label

sulemanof requested a review from markov00 August 24, 2020 10:43

alexwizp approved these changes Aug 24, 2020

View reviewed changes

sulemanof added Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Aug 24, 2020

sulemanof marked this pull request as ready for review August 24, 2020 10:50

sulemanof requested a review from a team August 24, 2020 10:50

markov00 reviewed Aug 24, 2020

View reviewed changes

markov00 self-requested a review August 25, 2020 12:44

markov00 previously requested changes Aug 25, 2020

View reviewed changes

sulemanof added 3 commits August 27, 2020 13:42

Merge remote-tracking branch 'upstream/master' into fix/markdown_date…

f8b9c7c

…_field_label

Apply changes

e9ebb8a

Merge remote-tracking branch 'upstream/master' into fix/markdown_date…

1a10087

…_field_label

markov00 reviewed Aug 31, 2020

View reviewed changes

src/plugins/vis_type_timeseries/public/application/components/lib/convert_series_to_vars.js Outdated Show resolved Hide resolved

sulemanof added 2 commits September 3, 2020 10:52

Merge remote-tracking branch 'upstream/master' into fix/markdown_date…

56cc9b4

…_field_label

Use default format if can't parse, add comments

3002691

sulemanof added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Sep 3, 2020

Merge branch 'master' into fix/markdown_date_field_label

ed876c5

timroes requested a review from markov00 September 8, 2020 13:50

stratoula approved these changes Sep 8, 2020

View reviewed changes

stratoula merged commit caa4e0c into elastic:master Sep 8, 2020

sulemanof deleted the fix/markdown_date_field_label branch September 8, 2020 14:44

sulemanof mentioned this pull request Sep 8, 2020

[7.x] [TSVB/Markdown] Introduce formatted date field label (#75555) #76951

Merged

wylieconlon mentioned this pull request Oct 15, 2020

[data.search.aggs] Aggs should exclude the key_as_string parameter from date histogram responses #80720

Closed

flash1293 mentioned this pull request Jul 22, 2022

[TSVB] formatted date field label in markdown missing #136929

Closed

Conversation

sulemanof commented Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Release note

Checklist

For maintainers

Uh oh!

stratoula Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

sulemanof Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

stratoula Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

stratoula Aug 21, 2020

Choose a reason for hiding this comment

Uh oh!

stratoula commented Aug 21, 2020

Uh oh!

stratoula commented Aug 21, 2020

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

alexwizp left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Aug 24, 2020

Uh oh!

markov00 left a comment

Choose a reason for hiding this comment

Uh oh!

sulemanof commented Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markov00 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sulemanof commented Aug 31, 2020

Uh oh!

Uh oh!

sulemanof commented Sep 7, 2020

Uh oh!

kibanamachine commented Sep 7, 2020

💚 Build Succeeded

Build metrics

async chunks size

History

Uh oh!

MakoWish commented Jul 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flash1293 commented Jul 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

sulemanof commented Aug 20, 2020 •

edited

Loading

sulemanof commented Aug 24, 2020 •

edited

Loading

markov00 left a comment •

edited

Loading

MakoWish commented Jul 21, 2022 •

edited

Loading