Skip to content

[Exploratory view] use percentages in distribution chart#103080

Merged
shahzad31 merged 23 commits intoelastic:masterfrom
shahzad31:percentages-over-all
Jun 29, 2021
Merged

[Exploratory view] use percentages in distribution chart#103080
shahzad31 merged 23 commits intoelastic:masterfrom
shahzad31:percentages-over-all

Conversation

@shahzad31
Copy link
Copy Markdown
Contributor

@shahzad31 shahzad31 commented Jun 23, 2021 •
edited
Loading

Summary

fix #103469

should be merged/reviwed after #103214

Using lens formula overall count implemented to display percentages in Distribution charts on Y axis in exploratory view.

image

@shahzad31 shahzad31 changed the title use percentages overall [Exploratory view] use percentages in distribution chart Jun 28, 2021
@shahzad31 shahzad31 self-assigned this Jun 28, 2021
@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Jun 28, 2021
@shahzad31 shahzad31 marked this pull request as ready for review June 28, 2021 12:21
@shahzad31 shahzad31 requested a review from a team June 28, 2021 12:21
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jun 28, 2021

Pinging @elastic/uptime (Team:uptime)

@shahzad31 shahzad31 requested a review from justinkambic June 28, 2021 12:22
flash1293
flash1293 approved these changes Jun 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Kibana app changes LGTM

dominiqueclarke
dominiqueclarke reviewed Jun 28, 2021
View reviewed changes
...ns/observability/public/components/shared/exploratory_view/configurations/lens_attributes.ts
}

getMainYAxis(layerConfig: LayerConfig) {
getMainYAxis(layerConfig: LayerConfig, layerId: string, columnFilter: string) {
Copy link
Copy Markdown
Contributor

@dominiqueclarke dominiqueclarke Jun 28, 2021

Choose a reason for hiding this comment

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

I don't think I fully understand the idea of a mainYAxis and a childYAxis of yet. Can you explain to me how they relate to one another?

Copy link
Copy Markdown
Contributor Author

@shahzad31 shahzad31 Jun 28, 2021

Choose a reason for hiding this comment

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

Sorry again for confusing naming here. Seems like i need to clean up this part as well.

So as far as Lens is concerned there are no main/child axis. I coined these because i thought we will have a main series concept in lens.

Essentially child columns here means there are more than one column in the layer.

...ns/observability/public/components/shared/exploratory_view/configurations/lens_attributes.ts Outdated
accessors: [
`y-axis-column-layer${index}`,
...Object.keys(this.getChildYAxises(layerConfig)),
// ...Object.keys(this.getChildYAxises(layerConfig)),
Copy link
Copy Markdown
Contributor

@dominiqueclarke dominiqueclarke Jun 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Did you intend to comment this out?

Copy link
Copy Markdown
Contributor Author

@shahzad31 shahzad31 Jun 28, 2021

Choose a reason for hiding this comment

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

no, i removed it now.

...ity/public/components/shared/exploratory_view/configurations/lens_columns/overall_column.tsx
string,
CountIndexPatternColumn | MathIndexPatternColumn | OverallSumIndexPatternColumn
> = {
[`${yAxisColId}X0`]: countColumn,
Copy link
Copy Markdown
Contributor

@dominiqueclarke dominiqueclarke Jun 28, 2021

Choose a reason for hiding this comment

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

the name column is still confusing to me, but I know it maps back to lens nicely.

Copy link
Copy Markdown
Contributor Author

@shahzad31 shahzad31 Jun 28, 2021

Choose a reason for hiding this comment

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

i will probably spend more time in tech sync or dedicated session to go over this concept.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 28, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
observability 246 247 +1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
lens 162 169 +7

Async chunks

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

id before after diff
observability 472.2KB 474.4KB +2.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
lens 16 14 -2
Unknown metric groups

API count

id before after diff
lens 174 181 +7

History

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

cc @shahzad31

@shahzad31 shahzad31 added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 28, 2021
justinkambic
justinkambic approved these changes Jun 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

Smoke test went well; I'm ++ to @dominique in thinking we should continue sessions on this codebase as a lot of it seems arcane to me.

@shahzad31 shahzad31 merged commit 409a0f2 into elastic:master Jun 29, 2021
@shahzad31 shahzad31 deleted the percentages-over-all branch June 29, 2021 09:21
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 29, 2021
@shahzad31 @kibanamachine
[Exploratory view] use percentages in distribution chart (elastic#103080
f63841d 
)
@kibanamachine kibanamachine mentioned this pull request Jun 29, 2021
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jun 29, 2021

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 29, 2021
@kibanamachine @shahzad31
[Exploratory view] use percentages in distribution chart (#103080) (#…
1339f85 
…103625)

Co-authored-by: Shahzad <shahzad.muhammad@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flash1293 flash1293 flash1293 approved these changes

@dominiqueclarke dominiqueclarke dominiqueclarke approved these changes

@justinkambic justinkambic justinkambic approved these changes

Assignees

@shahzad31 shahzad31

Labels

auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.14.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Exploratory view] Displays distribution as percentages in charts

6 participants

@shahzad31 @elasticmachine @kibanamachine @flash1293 @dominiqueclarke @justinkambic