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

[Discover] Fix context view for date_nanos format with custom timestamps #54089

Merged

Conversation

kertal
Copy link
Member

@kertal kertal commented Jan 7, 2020

Summary

Discover's context view currently doesn't work when you're using a custom formatted date_nanos timestamp like this: { "type" : "date_nanos", "format" : "yyyy-MM-dd HH:mm:ss.SSSSSS" }.

We're currently using the timestamp stored in _source to provide the context functionality with date_nanos timestamps, which is due to the limited support of BigInt support of Browsers. If the _source value is formatted in a non standard way, the conversion to a Date Object may fail. This fix switches from the value stored in _source to the value provided by fields of the search result.

Bildschirmfoto 2020-01-07 um 10 00 21

This contains a strict_date_time formatted string, so there are no more problems with custom timestamp formats

Bildschirmfoto 2020-01-07 um 10 04 02

Fixes #48064

Testing

You can use the following data to test the fix:

PUT date_nanos_test-ms
{
  "mappings": {
    "properties": {
      "timestamp": {
        "type": "date_nanos",
        "format" : "yyyy-MM-dd HH:mm:ss.SSSSSS"
      }
    }
  }
}

PUT date_nanos_test-ms/_doc/1
{
  "timestamp": "2019-10-12 00:30:04.828740",
  "test": "1"
}

PUT date_nanos_test-ms/_doc/2
{
  "timestamp": "2019-10-21 08:30:04.828733",
  "test": "1"
}


PUT date_nanos_test-ms/_doc/3
{
  "timestamp": "2019-10-21 00:30:04.828723",
  "test": "1"
}

Before the fix:

Bildschirmfoto 2020-01-07 um 10 20 43

After the fix:
Bildschirmfoto 2020-01-07 um 10 11 32

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
- [ ] 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

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

@kertal kertal force-pushed the kertal-pr-2019-12-20-context-custom-timestamp-fix branch from 1937102 to fc0f2f2 Compare January 7, 2020 08:40
@kertal kertal self-assigned this Jan 7, 2020
@kertal kertal marked this pull request as ready for review January 10, 2020 16:34
@kertal kertal requested a review from a team January 10, 2020 16:34
@kertal
Copy link
Member Author

kertal commented Jan 11, 2020

@elasticmachine merge upstream

@kertal
Copy link
Member Author

kertal commented Jan 11, 2020

@elasticmachine merge upstream

@kertal kertal requested a review from Bargs January 13, 2020 17:07
@kertal
Copy link
Member Author

kertal commented Jan 13, 2020

@elasticmachine merge upstream

Copy link
Contributor

@Bargs Bargs left a comment

Choose a reason for hiding this comment

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

Code change looks good to me and I tested the functionality in Chrome. Had one question about the tests that were changed.

…ithub.com:kertal/kibana into kertal-pr-2019-12-20-context-custom-timestamp-fix
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@kertal kertal added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Jan 14, 2020
@elasticmachine
Copy link
Contributor

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

kertal added a commit to kertal/kibana that referenced this pull request Jan 14, 2020
…mps (elastic#54089)

* Switch from _source to fields when fetching anchor records with date_nanos timestamps

* Add testdata

* Add functional test
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 17, 2020
…mps (elastic#54089)

* Switch from _source to fields when fetching anchor records with date_nanos timestamps

* Add testdata

* Add functional test
@kertal kertal added v7.7.0 and removed v7.6.0 labels Jan 20, 2020
flash1293 pushed a commit to flash1293/kibana that referenced this pull request Feb 19, 2021
…mps (elastic#54089)

* Switch from _source to fields when fetching anchor records with date_nanos timestamps

* Add testdata

* Add functional test
flash1293 added a commit that referenced this pull request Feb 19, 2021
* Discover: Add handling for source column (#91815)

* enable by default

* [Discover] Fix context view for date_nanos format with custom timestamps (#54089)

* Switch from _source to fields when fetching anchor records with date_nanos timestamps

* Add testdata

* Add functional test

* fix search_after

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>
@flash1293 flash1293 added v7.12.0 and removed v7.7.0 labels Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover]view surrounding documents on date_nanos field, the result incorrect
5 participants