[Lens] Allow non-numeric metrics for metric vis

[mbondyra]

Summary

Fixes #137756
Allows to set non numeric metrics for metric visualization for primary and secondary metric.

Doesn't include coloring by terms.
When primary metric is non-numeric:

1. when maximum value is empty, we hide maximum value group
2. when maximum value has a value we set an error message on dimension
3. we don’t allow to use a palette for coloring
4. we don’t allow to set a trendline

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
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[elasticmachine]

Pinging @elastic/kibana-visualizations (Team:Visualizations)

[stratoula]

This looks great! I found some small bugs:

• When the collapse by is on then I see the coloring options in the metric dimension for non numeric fields

• I have a configuration like this

I change the metric to last value of @timestamp and my chart is colored blue for some reason, shouldn't this default to the neutral color? If I remove the dimension it works fine. Also with trendlines it works as expected. The problem is when I have the bars set.

[dej611]

Maybe this does not strictly belong here, but I would like to raise some awareness about long text fields for Metric:

While the secondary metric can go multiline, the primary metric has a no-wrap policy for the CSS white-space property.
CC @gvnmagni

I've tried to quickly check what would happen without that nowrap thing but it introduces other issues:

[gvnmagni]

Thank you @dej611, given the nature of the secondary metric it was easier to expect that it could go on multiple lines, it is also way smaller and then way more easy to deal with. At the same time this is an interesting conversation to have because introducing text might make worth to talk about multi-lines for values/strings as well. Thank you for pointing this out!

[mbondyra]

@dej611 @gvnmagni I'll create an issue about the pointed out problem, we should definitely find a solution but I think it's out of scope for this PR.

[drewdaemon]

Nice! Just did a code review.

[drewdaemon]

If I remove the dimension it works fine. Also with trendlines it works as expected. The problem is when I have the bars set.

Yeah, IIRC when no max is provided, elastic-charts interprets the color property as the background color instead of the bar color.

[mbondyra]

When the collapse by is on then I see the coloring options in the metric dimension for non numeric fields

@stratoula this is an extreme edgecase and also not an easy problem to fix (we already have it in datatable). I could create a bug or we could also just ignore it – in the end, seeing these options don't really break anything.

[stratoula]

Well this doesn't look good

in datatable also doesn't make sense but feels a bit better

I agree that is an edge case but can you explain why is it difficult to hide this in that case?

[stratoula]

I see that the problem is that the last metric column reports that is a number but is wrong. Can we fix it? If we fix it on the aggs level then it will be solved in both places

[gchaps]

How about making this text more positive.

Primary metric must be numeric to set a maximum value.

[stratoula]

I see that the problem is that the last metric column reports that is a number but is wrong. Can we fix it? If we fix it on the aggs level then it will be solved in both places

If the fix on the aggs level is difficult you can do it here possibly (I dont think that this will cause any bugs tbh) x-pack/plugins/lens/common/expressions/datatable/utils.ts

We are already doing this for min and max. You can add a check for last value too. If you check the column.meta.params.id the value is string (not numeric) which is the correct type. So you could extend it possibly for other types.

// min and max aggs are reporting as number but are actually dates - work around this by checking for the date formatter until this is fixed at the source

const isNumeric = column?.meta.type === 'number' && column?.meta.params?.id !== 'date';

[mbondyra]

@stratoula all the bugs were fixed (by rebasing off the aggs PR)
@gchaps copy replaced with your suggestion, thank you!
@drewdaemon I will start working on replacing the test file with RTL, but I will submit it in the separate PR to not make this one bulky and dependant on my speed.

Please recheck 🙏🏼 😊

[gchaps]

The text re: the primary metric LGTM.

What does this message mean: At least one color range contains the wrong value or color

[mbondyra]

@gchaps this doesn't come from this PR, but I am happy to revisit. This error appears when the input value for the color ranges is not correct (see the screenshot attached to Stratoula's message)

[stratoula]

Ok we are very close!

• If you set a collapse by on a numerc field and then change the metric to last value of a keyword the color remains. Let's default to the default one

[mbondyra]

Fixed!

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: d06095e

Failed CI Steps

• Defend Workflows Cypress Tests on Serverless #1

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
expressionMetricVis	4.6KB	4.9KB	+283.0B
lens	1.4MB	1.4MB	+1.2KB
total			+1.5KB

History

• 💚 Build #171225 succeeded 088ff3e
• 💔 Build #171182 failed 8b4738e
• 💔 Build #170366 failed 5690aaa
• 💛 Build #169798 was flaky baad606
• 💛 Build #169651 was flaky 0e495ac
• 💔 Build #169490 failed 7c86ba3

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

[drewdaemon]

Tested the broken color cases! Thank you!