Use groupBy when groupings is not populated correctly

[kdelemme]

Summary

This PR fallbacks on the groupBy/instanceId fields to build the preview data queries when groupings is empty. It can be empty because the SLO has not been updated for more than its lookback window.
Previously, we would show all events regardless of the groupBy/InstanceId values, bringing confusion to the end user.

Testing

1. Create some SLO (7days) with data forge running, using two groupBy keys.
2. After a while, stop data forge, and delete the previously generated sli documents for one SLO instanceId only.
3. When going to the slo details page, the good vs bad events chart should show the correct number of events

[obltmachine]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

• /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
• run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

[elasticmachine]

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

[simianhacker]

Hum... when the slo.instanceId is created, we sort the values to keep them stable. It's not guaranteed that the first value will match the first field.

kibana/x-pack/plugins/observability_solution/slo/server/assets/ingest_templates/slo_pipeline_template.ts

Line 75 in 0be8295

Collections.sort(sortedValues);

I wonder if we shouldn't change the ingest pipeline to ensure the first value matches first field, the second value matches the second field, and so on... To do this, we could simplify the ingest pipeline by getting rid of the painless script and replace it with:

   {
     dot_expander: {
       path: 'slo.groupings',
       field: '*',
     },
   },
   {
     set: {
       field: 'entity.id',
       value: [slo.groupBy]
         .flat()
         .map((field) => `{{{slo.groupings.${field}}}}`)
         .join(','),
     },
   },

This would ensure that first value in the slo.instanceId would map back to the first field in the group by list. Since this quirk only affects SLOs who's data stopped reporting and were multi-group-by-values (probably a small subset of our users), it would work for most everyone else. Anyone who had a multi-group-by slo.instanceId that had an issue could use the _reset command to get the new ingest pipeline and things would start working as expected.

Thoughts?

[kdelemme]

I think that's a good approach. I like the simplification. We probably need to handle the case where groupBy is * but otherwise looks good to me

[kdelemme]

I came up with this 2 processors to handle no groupings. Local testing seems to work fine without group, with 1 and many groups

 {
      dot_expander: {
        path: 'slo.groupings',
        field: '*',
        ignore_failure: true,
        if: 'ctx.slo.groupings != null',
      },
    },
    {
      set: {
        description: 'Generated the instanceId field based on the groupings field',
        field: 'slo.instanceId',
        value: [slo.groupBy].flat().includes(ALL_VALUE)
          ? ALL_VALUE
          : [slo.groupBy]
              .flat()
              .map((field) => `{{{slo.groupings.${field}}}}`)
              .join(','),
      },
    },

[kdelemme]

@simianhacker can you take a look this week please?

[kdelemme]

@simianhacker Do you have time to review this please?

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 7bd7bc0
• Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-189672-7bd7bc03a206

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
observability	463.6KB	463.6KB	-43.0B
slo	853.3KB	853.3KB	-25.0B
total			-68.0B

History

• 💛 Build #234045 was flaky 4cee9ac
• 💛 Build #233408 was flaky 7633a6c
• 💚 Build #226608 succeeded f626edb
• 💚 Build #225832 succeeded 1401b79
• 💛 Build #225398 was flaky 48dec39

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

[shahzad31]

Did Testing with multiple group-by fields and things looks normal !!

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation