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

[Logs UI] The UI throws error when @timestamp format is date_nanos #88290

Closed
simianhacker opened this issue Jan 13, 2021 · 13 comments · Fixed by #170308
Closed

[Logs UI] The UI throws error when @timestamp format is date_nanos #88290

simianhacker opened this issue Jan 13, 2021 · 13 comments · Fixed by #170308
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Logs UI Logs UI feature Team:obs-ux-logs Observability Logs User Experience Team

Comments

@simianhacker
Copy link
Member

Kibana version:
master

Elasticsearch version:
master

Describe the bug:
When @timestamp is set to date_nanos the date formatter throws an error.

Steps to reproduce:

  1. Index some logs setting @timestamp to date_nanos (contact @simianhacker for help)
  2. Open Logs UI
  3. :(

Expected behavior:
It should render properly without errors.

Screenshots (if relevant):
image

@simianhacker simianhacker added bug Fixes for quality problems that affect the customer experience Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Jan 13, 2021
@elasticmachine
Copy link
Contributor

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

@weltenwort
Copy link
Member

related: #40183

@jasonrhodes
Copy link
Member

Note: from @weltenwort -- Discover and some other places in Kibana have solved how to handle the nanosecond format for bigint, we should look into those implementations for guidance on how to fix this in Logs UI.

@joshdover
Copy link
Contributor

@Kerry350 Has there been any progress on this? We are exploring supporting nanoseconds in Beats/Agent and would need to be sure Kibana fully supports this.

@Kerry350
Copy link
Contributor

Hi @joshdover, there hasn't been any progress on this to date. Do you know what the timeframe would be for adding the support in Beats / Agent? This would allow us to try and reprioritise the support.

@jughosta
Copy link
Contributor

jughosta commented May 10, 2023

If it's of any help here, there is a way of getting date nanos as a string instead of a timestamp so it can be safely used for search_after. Example #157241

@ruflin
Copy link
Member

ruflin commented Sep 11, 2023

Adding some more links for context here:

To reproduce it, here some Dev Tools command:

DELETE _index_template/foo

PUT _index_template/foo
{
  "index_patterns": ["logs-foo-bar"],
  "template": {
    "mappings": {
      "properties": {
        "host_name": {
          "type": "keyword"
        },
        "@timestamp": {
          "type": "date_nanos"
        }
      }
    }
  },
  "priority": 500,
  "version": 3,
  "data_stream": { }
}

DELETE _data_stream/logs-foo-bar
POST logs-foo-bar/_doc
{
  "@timestamp": "2023-09-07T15:04:05.000000002Z",
  "message": "Second message"
}

POST logs-foo-bar/_doc
{
  "@timestamp": "2023-09-07T15:04:05.000000001Z",
  "message": "First message"
}

POST logs-foo-bar/_doc
{
  "@timestamp": "2023-09-07T15:04:05.000000003Z",
  "message": "Third message"
}

POST logs-foo-bar/_doc
{
  "@timestamp": "2023-09-07T15:04:05.000000000Z",
  "message": "Third message"
}

GET logs-foo-bar/_search
{
  "sort" : ["@timestamp"]
}

Going to Discover and include the time range, the sorting works as expected. Going to Logs UI and including the time range with the timestamp above shows the error

@weltenwort
Copy link
Member

Some thoughts and hints for anyone picking this up in the future:

As @jughosta pointed out we need to fetch the timestamp as a string because a numeric nanosecond timestamp exceeds the value range of normal ECMAScript numbers. In contrast to Discover, the Logs UI interprets the timestamp in several places, though, and even performs date math on it.

So while switching the format parameter in queries to strict_date_optional_time_nanos and the TypeScript type from number to string should work in many places where we just pass the value back to Elasticsearch, some usage sites might have to be rewritten to keep working. A quick list off the top of my head would be:

  • The "fetch around" logic, in which we add 1 ms as the smallest step to not skip any documents with search_after.
  • How we pass the timestamp to the minimap to mark the visible window.
  • How we pass the timestamp to ML during job creation.
  • How we sync the position to the URL.

@mohamedhamed-ahmed mohamedhamed-ahmed self-assigned this Sep 29, 2023
@mohamedhamed-ahmed
Copy link
Contributor

I timeboxed this ticket and quickly tried an approach similar to the one discover applied using strict_date_optional_time_nanos. The solution works and the UI was working fine as in the video below.

Adding to what @weltenwort mentioned and the places that need to be modified we also need to take care of:

  1. The logs flyout as it is also using the @timestamp value.
  2. The test files
  3. Modifying the types as they are propagating in many places in the code.

I wasn't able to spend much time to be more precise on exact expected time that we need to get this fixed but the change doesn't seem to be a quick one.

One question though, do we just need the UI to not break with date_nanos, or do we also need to display and make use of the date_nanos in the Stream page?

Because one option we might have is that we retrieve the timestamp as date_nanos but remove the precision of the nano_seconds when we get the response and continue using it in number as before. Sounds a bit hacky and not sure about the consequences of this in the code until we spend more time into it but could be an option here.

date_nanos.mov

@ruflin
Copy link
Member

ruflin commented Oct 9, 2023

One question though, do we just need the UI to not break with date_nanos, or do we also need to display and make use of the date_nanos in the Stream page?

I think step one should be to get the Logs UI not working when there is a nano timestamp in the data. If there is a quick fix to this through rounding, I suggest we do it and follow up with the more detailed fix in step 2.

@mohamedhamed-ahmed
Copy link
Contributor

After discussing this further with @weltenwort we concluded that it would be best to solve this in a proper way and address all places where the timestamp is being used. Otherwise, it would be hacky to do a quick fix as it might introduce inconsistent behaviors where for example documents are retrieved twice or even not retrieved at all because of the rounding issues.

Given that, the ET for getting this fixed would be around a week to address the places that were clear during the timeboxed investigation and also other places that came up during the discussion with @weltenwort.

@ruflin wdyt? should we proceed with implementing the fix?

@ruflin
Copy link
Member

ruflin commented Oct 18, 2023

++ on moving forward with this.

@gbamparop gbamparop added Team:obs-ux-logs Observability Logs User Experience Team and removed Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Nov 9, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

@botelastic botelastic bot added needs-team Issues missing a team label and removed needs-team Issues missing a team label labels Nov 9, 2023
mohamedhamed-ahmed added a commit that referenced this issue Nov 13, 2023
closes #88290 

## 📝  Summary

As described in #88290 we need to add `date_nanos` support to the Stream
UI page. In this PR the necessary changes have been made all over the
Stream UI and the usages of it.

## ✅  Testing

⚠️ Testing the Stream UI with old timestamp indices is important to make
sure that the behavior is still as before and not affected at all. This
can be done by running local env from the PR and simulating all
interactions on edge-lit cluster for example, to make sure that the
behavior is not changed.

For testing the new changes with support of `date_nano`:

1. You can use [the steps
here](#88290 (comment))
to create and ingest documents with nano precision.
2. Navigate to the stream UI and the documents should be displayed
properly.
3. Sync with the URL state should be changed from timestamp to ISO
string date.
4. Changing time ranges should behave as before, as well as Text
Highlights.
5. Open the logs flyout and you should see the timestamp in nano
seconds.
6. Play around with the minimap, it should behave exactly as before.

### Stream UI:

<img width="2556" alt="Screenshot 2023-11-02 at 14 15 49"
src="https://github.com/elastic/kibana/assets/11225826/596966cd-0ee0-44ee-ba15-f387f3725f66">

- The stream UI has been affected in many areas:
- The logPosition key in the URL should now be in ISO string, but still
backward compatible incase the user has bookmarks with timestamps.
- The minimap should still behave as before in terms of navigation
onClick and also highlighting displayed areas

### Stream UI Flyout:

<img width="2556" alt="Screenshot 2023-11-02 at 14 15 23"
src="https://github.com/elastic/kibana/assets/11225826/6081533c-3bed-43e1-872d-c83fe78ab436">

- The logs flyout should now display the date in nanos format if the
document is ingested using a nano format.

### Anomalies:

<img width="1717" alt="Screenshot 2023-11-01 at 10 37 22"
src="https://github.com/elastic/kibana/assets/11225826/b6170d76-40a4-44db-85de-d8ae852bc17e">

-Anomalies tab under logs as a navigation to stream UI which should
still behave as before passing the filtration and time.

### Categories:

<img width="1705" alt="Screenshot 2023-11-01 at 10 38 19"
src="https://github.com/elastic/kibana/assets/11225826/c4c19202-d27f-410f-b94d-80507542c775">

-Categories tab under logs as a navigation to stream UI which should
still behave as before passing the filtration and time.

### External Links To Stream:
- All links to the Stream UI should still work as before:
   - APM Links for traces, containers, hosts
   - Infra links in Inventory and Hosts views

## 🎥 Demo


https://github.com/elastic/kibana/assets/11225826/9a39bc5a-ba37-49e0-b7f2-e73260fb01f0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Logs UI Logs UI feature Team:obs-ux-logs Observability Logs User Experience Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants