[Metrics UI] Drop partial buckets from ALL Metrics UI queries

[simianhacker]

Summary

This PR fixes #104699 by adding functions to drop partial buckets, including "micro buckets". This PR also changes the resolution of the histogram offset from seconds to milliseconds; in turn this will further eliminate the possibility of "micro buckets".

Checklist

• Unit or functional tests were updated or added to match the most common scenarios

[spalger]

jenkins, test this

(had to abort for Jenkins upgrade)

[elasticmachine]

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

[estermv]

I took a first quick look, I have some small questions, but I think I need a better understanding of the problem we are trying to solve.

I don't really understand how this scenario can be possible. Shouldn't the query limit the documents that are returned? If in the query I filter documents "lt": "2021-07-07T19:37:00.000Z" it seems to me really unintuitive that are returned documents >=2021-07-07T19:37:00.000Z.

Also, from what I understood reviewing the PR, this could be a problem of precision, would it work if, instead of putting the upper limit to 2021-07-07T19:37:00.000Z, we put something like 2021-07-07T19:36:59.999Z?
This came to my mind looking at the example you put in the linked issue. In that case, we get an extra bucket with documents >=2021-07-07T19:37:00.000Z to <=2021-07-07T19:38:00.000Z, because documents like 2021-07-07T19:37:xx.xxxZ goes to this extra bucket. Since we get rid of this data, setting a slightly lower limit, shouldn't return any 2021-07-07T19:37:xx.xxxZ documents. Does this make any sense? I could be missing a million things here 😅

[estermv]

Not sure if this was removed on purpose. If it's on purpose I think the describe that contains it, can also be removed as there is no other test.

[simianhacker]

Whoops... good catch on the describe. This part of the function was moved up a level.

[simianhacker]

@estermv Good questions, I'm not sure changing the time range will help. Here is my attempt at distilling what's happening and what this change is trying to mitigate.

When date_histogram creates the buckets it will round the first bucket to the nearest interval. Let's look at an example:

POST metricbeat-*/_search
{
  "size": 0,
  "query": {
    "bool": {
      "filter": [
        {
          "range": {
            "timestamp": {
              "gte": "2021-07-15T14:14:14.771Z",
              "lte": "2021-07-15T14:19:14.771Z"
            }
          }
        }
      ]
    }
  },
  "aggs": {
    "timeseries": {
      "date_histogram": {
        "field": "timestamp",
        "interval": "1m",
        "extended_bounds": {
          "min": "2021-07-15T14:14:14.771Z",
          "max": "2021-07-15T14:19:14.771Z"
        }
      }
    }
  }
}

This produces this response:

{
  "took" : 4,
  "timed_out" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 0,
      "relation" : "eq"
    },
    "max_score" : null,
    "hits" : [ ]
  },
  "aggregations" : {
    "timeseries" : {
      "buckets" : [
        {
          "key_as_string" : "2021-07-15T14:14:00.000Z",
          "key" : 1626358440000,
          "doc_count" : 0
        },
        {
          "key_as_string" : "2021-07-15T14:15:00.000Z",
          "key" : 1626358500000,
          "doc_count" : 0
        },
        {
          "key_as_string" : "2021-07-15T14:16:00.000Z",
          "key" : 1626358560000,
          "doc_count" : 0
        },
        {
          "key_as_string" : "2021-07-15T14:17:00.000Z",
          "key" : 1626358620000,
          "doc_count" : 0
        },
        {
          "key_as_string" : "2021-07-15T14:18:00.000Z",
          "key" : 1626358680000,
          "doc_count" : 0
        },
        {
          "key_as_string" : "2021-07-15T14:19:00.000Z",
          "key" : 1626358740000,
          "doc_count" : 0
        }
      ]
    }
  }
}

If the from (or gte) timestamp is 2021-07-15T14:14:14.771Z then the first bucket will be 2021-07-15T14:14:00.000Z which actually covers an extra 45229ms of data; we call this a partial bucket because it starts before the actual time range. If we took an average of this bucket, it might only contain 2 events where a normal bucket might contain 6 events, so the average is not representative of the whole bucket so we should toss it.

The same thing happens on the other end, the last bucket's starting timestamp is 2021-07-15T14:19:00.000Z and the ending timestamp would be 2021-07-15T14:20:00.000Z which also overlaps the end of the time range. To try and correct for this we add an offset of -45229ms to the date_histogram to shift the buckets to try and get a "whole" bucket for the last value.

This PR changes the resolution of the offset to milliseconds which should eliminate the need to prune the last bucket. There is still an edge case where sometimes instead of returning 6 buckets for a 5 minute time range, it returns 7 due to how the rounding works (I think).

For most scenarios, the changing the offset to have millisecond resolution fixes the issue with the last bucket being partial. The dropPartialBuckets filter will trim off the first bucket for us and will trim off the last backet when we hit the edge case. The dropPartialBuckets filter is really our safety net for edge cases and weirdness.

[estermv]

Just commenting here, as I think this is totally unrelated to the PR. I would like to understand the reason why we have this logic here (the whole implementation of the mock). This is a place for potential bugs and makes it hard to understand what each test is testing.

[estermv]

LGTM 🚀

@simianhacker thanks for the detailed explanation, it helped a lot 🙌

The only thing I notice is that in the alerts preview charts we display one less column but I don't expect anyone to count them 😅

Please, don't forget to remove the describe mentioned here https://github.com/elastic/kibana/pull/104784/files#r670377393 before merging it!

[simianhacker]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 508a94f
• Storybooks Preview

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
infra	1.7MB	1.7MB	+8.0B

History

• 💚 Build #139107 succeeded 1be7c48
• 💚 Build #138636 succeeded bb6e240
• 💚 Build #138036 succeeded 2e9fee7
• 💔 Build #137995 failed 9624e5a
• 💔 Build #137978 failed b085acf

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