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

[ML] Transforms: Fixes chart histograms for runtime fields. #93028

Merged
merged 5 commits into from
Mar 2, 2021

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Mar 1, 2021

Summary

Part of #71231

Fixes chart histograms for runtime fields. The runtime field configurations were not passed on to the endpoint to fetch the charts data, so charts ended up being empty with a 0 documents legend.

  • Makes sure the wizard's runtime configuration gets passed on to the API endpoint to fetch the chart histogram data
  • Moves histogram charts related types and type guards to ml/common/types/field_histograms.ts for better reuse.
  • Moves runtimeMappingsSchema to api_schemas/common for better reuse.

image

Checklist

@walterra walterra added bug Fixes for quality problems that affect the customer experience :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Transforms ML transforms v7.12.0 v7.13.0 labels Mar 1, 2021
@walterra walterra self-assigned this Mar 1, 2021
@walterra walterra requested a review from a team as a code owner March 1, 2021 13:35
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra walterra requested a review from pheyos March 1, 2021 16:48
@qn895
Copy link
Member

qn895 commented Mar 1, 2021

Tested and the extra validation LGTM. One small thing is I wasn't able to get histogram charts for the numeric and boolean runtime mapping to show up while testing.

Screen Shot 2021-03-01 at 11 22 11

@walterra
Copy link
Contributor Author

walterra commented Mar 2, 2021

@qn895 good catch! I fixed histograms for numeric fields in bfb8654, I missed adding runtime fields to the additional query required to fetch the range of values for numeric fields.

image

@walterra walterra mentioned this pull request Mar 2, 2021
13 tasks
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
ml 1739 1740 +1

Async chunks

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

id before after diff
ml 6.4MB 6.4MB -249.0B
transform 944.2KB 944.9KB +680.0B
total +431.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 68.8KB 69.9KB +1.1KB

History

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

cc @walterra

type: 'numeric';
}

export const isNumericChartData = (arg: any): arg is NumericChartData => {
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we use unknown instead of any for type guards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea, but since we used any so far for all type guards, I'd rather do this in a follow up and change it for all type guards we have if we want to do it. There might be some changes necessary to how we check objects.

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@walterra walterra added the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 2, 2021
@walterra walterra merged commit 8201d4f into elastic:master Mar 2, 2021
@walterra walterra deleted the ml-fix-histogram-runtime-fields branch March 2, 2021 12:37
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 2, 2021
…93028)

Fixes chart histograms for runtime fields. The runtime field configurations were not passed on to the endpoint to fetch the charts data, so charts ended up being empty with a 0 documents legend.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 2, 2021
…93028)

Fixes chart histograms for runtime fields. The runtime field configurations were not passed on to the endpoint to fetch the charts data, so charts ended up being empty with a 0 documents legend.
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.12 / #93202
7.x / #93203

Successful backport PRs will be merged automatically after passing CI.

@peteharverson peteharverson mentioned this pull request Mar 2, 2021
12 tasks
kibanamachine added a commit that referenced this pull request Mar 2, 2021
…93202)

Fixes chart histograms for runtime fields. The runtime field configurations were not passed on to the endpoint to fetch the charts data, so charts ended up being empty with a 0 documents legend.

Co-authored-by: Walter Rafelsberger <walter@elastic.co>
kibanamachine added a commit that referenced this pull request Mar 2, 2021
…93203)

Fixes chart histograms for runtime fields. The runtime field configurations were not passed on to the endpoint to fetch the charts data, so charts ended up being empty with a 0 documents legend.

Co-authored-by: Walter Rafelsberger <walter@elastic.co>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 2, 2021
…bana into task-manager/docs-monitoring

* 'task-manager/docs-monitoring' of github.com:gmmorris/kibana:
  [ILM] Allow multiple searchable snapshot actions (elastic#92789)
  Improve consistency for display of management items (elastic#92694)
  skip flaky suite (elastic#93152)
  skip flaky suite (elastic#93152)
  [ILM] Refactor edit_policy client integration tests into separate feature files (elastic#92826)
  Add developer documentation about the building blocks we offer plugin developers (elastic#92743)
  [Security Solution] Case ui enhancement (elastic#91863)
  [Security Solution] [Detections] Updates warning message when no indices match provided index patterns (elastic#93094)
  Collect agent telemetry even when fleet server is disabled. (elastic#93198)
  [Lens] Fix runtime validation error message (elastic#93195)
  [Lens] Remove warning about ordinal x-domain (elastic#93049)
  [Security Solution] Fixes the Customize Event Renderers modal by removing the EuiOverlayMask (elastic#93150)
  Cleanup Security plugin imports (elastic#93056)
  [Security Solution] - Bug fixes (elastic#92294)
  Updated doc links (elastic#92968)
  [ML] Transforms: Fixes chart histograms for runtime fields. (elastic#93028)
  [chore] Enable core's eslint rule: `@ts-expect-error` (elastic#93086)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience Feature:Transforms ML transforms :ml release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants