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

[Lens] Save valuesInLegend:true as legendStats:['value'] in preparation for legend statistics #181953

Merged
merged 12 commits into from
May 3, 2024

Conversation

mbondyra
Copy link
Contributor

@mbondyra mbondyra commented Apr 29, 2024

Summary

To the code owners,

This PR updates the saved object of Lens with the following changes: the property valuesInLegend: boolean is converted to the new property legendStats: ["currentAndLastValue"] (cartesian) or legendStats: ["value"](partition) to accommodate displaying additional legend statistics in the future. There are no user-facing changes introduced. I have updated all saved object shapes in the code to adhere to the new structure. While the old structure will continue to function properly, this update ensures cleaner code.

Fixes #181035 (phase 2). It's is a continuation of #180917.

In comparison to the phase 1, we've removed the conversion of legendStats to valuesInLegend which was initially added in the previous PR to ensure backward compatibility for serverless applications.

Screenshot 2024-04-25 at 10 51 50

I won't merge till the end of the week as it has to be released separately from phase 1.

@mbondyra mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v8.15.0 labels Apr 29, 2024
@mbondyra mbondyra marked this pull request as ready for review April 29, 2024 10:24
@mbondyra mbondyra requested review from a team as code owners April 29, 2024 10:24
@mbondyra mbondyra requested a review from machadoum April 29, 2024 10:24
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Apr 29, 2024
@elasticmachine
Copy link
Contributor

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

@mbondyra mbondyra changed the title [Lens] Save valuesInLegend:true as legendStats:['values'] in preparation for legend statistics [Lens] Save valuesInLegend:true as legendStats:['values'] in preparation for legend statistics Apr 29, 2024
@mbondyra mbondyra changed the title [Lens] Save valuesInLegend:true as legendStats:['values'] in preparation for legend statistics [Lens] Save valuesInLegend:true as legendStats:['value'] in preparation for legend statistics Apr 29, 2024
mbondyra and others added 9 commits April 30, 2024 11:37
…es (elastic#182001)

Related to elastic#180677
closes elastic#181886

There's still a small difference on the CO2 values, but that's due to
different formats returned by ES apis:
```
TopN Functions
"self_annual_co2_tons": 0.0068555964801996295

Flamegraph
"AnnualCO2TonsInclusive": [
    0.0069,
```

After:
<img width="1765" alt="Screenshot 2024-04-29 at 16 34 57"
src="https://github.com/elastic/kibana/assets/55978943/092a704f-69fe-4dd0-99d5-9ac9bce77188">
<img width="1788" alt="Screenshot 2024-04-29 at 16 35 03"
src="https://github.com/elastic/kibana/assets/55978943/da4a1406-fad7-48de-81ac-e8aae64cba67">
## Summary
The PR elastic#182001 was verified at a
point where typechecking was missing for a bit, this allowed a missing
field to slip in. This PR tries to fix it.
## Summary
As usual, the diff through the migration revealed some drifting between
our assumed defaults and the defaults on the new infra.

This PR re-adjusts the settings to how it was before the migration.
@mbondyra mbondyra force-pushed the lens/migrate_show_values_stage_2 branch from 00b0cb6 to 4369808 Compare April 30, 2024 15:10
Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

Obs Services LGTM

Copy link
Member

@machadoum machadoum left a comment

Choose a reason for hiding this comment

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

Entity Analytics changes LGTM!

Copy link
Contributor

@nikitaindik nikitaindik left a comment

Choose a reason for hiding this comment

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

Hey @mbondyra! I have checked the Rule Management changes. LGTM.

@nikitaindik
Copy link
Contributor

Actually, I just noticed one thing: in dashboard_rule_monitoring.json.

For "Executions by rule type" and "Executions by status" visualisations legendStats is equal to ["values"] (with an "s" at the end, not a singular "value").
Also, since it's an XY visualisation, shouldn't it be "currentAndLastValue"?

The dashboard works just fine, though. And it looks same as in main.

This branch
branch

vs main
main

Is changing legendStats in JSONs supposed to have an effect of visualisations?

…a/kibana into lens/migrate_show_values_stage_2

# Conflicts:
#	x-pack/plugins/lens/public/visualizations/partition/visualization.test.ts
#	x-pack/plugins/lens/public/visualizations/xy/visualization.test.tsx
#	x-pack/plugins/security_solution/server/lib/dashboards/__mocks__/index.ts
#	x-pack/plugins/security_solution/server/lib/detection_engine/rule_monitoring/logic/detection_engine_health/assets/dashboard_rule_monitoring.json
@mbondyra
Copy link
Contributor Author

mbondyra commented May 2, 2024

Hi @nikitaindik thanks for pointing it out. The legendStats: ['currentAndLastValue'] setting shows you the value of the last bucket if it exists or the current value in the legend when you hover over the series.
Screenshot 2024-05-02 at 12 01 16

I fixed it already 🙏🏼

Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Tested locally with different scenarios ( a dashboard from 8.12 and one from the phase 1 state) and all worked fine 👍

@kibana-ci
Copy link
Collaborator

kibana-ci commented May 2, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

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
apm 3.2MB 3.2MB -81.0B
lens 1.4MB 1.4MB -524.0B
securitySolution 13.7MB 13.7MB +324.0B
total -281.0B

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5407 +5407
total size - 9.1MB +9.1MB

History

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

@mbondyra mbondyra merged commit ab065ad into elastic:main May 3, 2024
39 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 3, 2024
@mbondyra mbondyra deleted the lens/migrate_show_values_stage_2 branch May 3, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:review backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Support new legendStats and old valuesInLegend saved object properties