Skip to content

Conversation

@walterra
Copy link
Contributor

@walterra walterra commented Apr 29, 2021

Summary

Fixes #96609.

  • Fixes correctly escaping the characters .[] in field names with double backslashes since Vega treats dots/brackets in field names special to be able to access attributes in object structures. See https://vega.github.io/vega-lite/docs/field.html for details. This replaces the old approach that replaced dots with a similar but different UTF-8 character but missed the brackets.
  • Additionally adds an EuiErrorBoundary component around the VegaChart component so we don't crash the whole page should another issue with Vega bubble up.
  • Note: There seems to be a bug in vega-lite which renders escaping backslashes in axis and legend titles, I created a bug report here.

Checklist

@walterra walterra added bug Fixes for quality problems that affect the customer experience release_note:fix :ml v8.0.0 Feature:Data Frame Analytics ML data frame analytics features v7.14.0 v7.13.0 labels Apr 29, 2021
@walterra walterra requested review from peteharverson and qn895 April 29, 2021 15:17
@walterra walterra self-assigned this Apr 29, 2021
@walterra walterra requested a review from a team as a code owner April 29, 2021 15:17
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@qn895
Copy link
Member

qn895 commented Apr 29, 2021

Tested the Vega charts and code LGTM 🎉 . Unrelated to this PR - I noticed there's an issue when picking fields with special characters in Regression or Classification, but we can probably fix this in a follow up PR in the future.

2021-04-29 at 11 40

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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
ml 5.9MB 5.9MB +5.0B

History

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

cc @walterra

@peteharverson
Copy link
Contributor

Unrelated to this PR - I noticed there's an issue when picking fields with special characters in Regression or Classification, but we can probably fix this in a follow up PR in the future.

This looks like an issue with the _ml/data_frame/analytics/_explain endpoint, when one of the fields in the includes list contains a comma. For example running this query on the special_characters_2019 data set:

POST _ml/data_frame/analytics/_explain
{
   "description":"",
   "source":{
      "index":"special_characters_2019",
      "query":{
         "match_all":{
            
         }
      }
   },
   "analyzed_fields":{
      "includes":[
         "ampersand&name",
         "asterisk*name",
         "at@name",
         "colon:name",
         "comma,name"
      ]
   },
   "analysis":{
      "regression":{
         "dependent_variable":"metric%$£&!{(]field",
         "num_top_feature_importance_values":0,
         "training_percent":80
      }
   },
   "max_num_threads":1
}

Gives the error

{
  "error" : {
    "root_cause" : [
      {
        "type" : "status_exception",
        "reason" : "No field [comma] could be detected"
      }
    ],
    "type" : "status_exception",
    "reason" : "No field [comma] could be detected"
  },
  "status" : 400
}

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. Will follow up the issue around the _explain endpoint separately (raised elastic/elasticsearch#72541)

@walterra walterra added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 30, 2021
@walterra walterra merged commit 98a2840 into elastic:master Apr 30, 2021
@walterra walterra deleted the ml-fix-vega-scatterplot-special-chars branch April 30, 2021 10:40
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 30, 2021
…atterplot matrix. (elastic#98763)

- Fixes correctly escaping the characters .[] in field names with double backslashes since Vega treats dots/brackets in field names special to be able to access attributes in object structures. This replaces the old approach that replaced dots with a similar but different UTF-8 character but missed the brackets.
- Additionally adds an EuiErrorBoundary component around the VegaChart component so we don't crash the whole page should another issue with Vega bubble up.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 30, 2021
…atterplot matrix. (elastic#98763)

- Fixes correctly escaping the characters .[] in field names with double backslashes since Vega treats dots/brackets in field names special to be able to access attributes in object structures. This replaces the old approach that replaced dots with a similar but different UTF-8 character but missed the brackets.
- Additionally adds an EuiErrorBoundary component around the VegaChart component so we don't crash the whole page should another issue with Vega bubble up.
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.13
7.x

The backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 30, 2021
…atterplot matrix. (#98763) (#98891)

- Fixes correctly escaping the characters .[] in field names with double backslashes since Vega treats dots/brackets in field names special to be able to access attributes in object structures. This replaces the old approach that replaced dots with a similar but different UTF-8 character but missed the brackets.
- Additionally adds an EuiErrorBoundary component around the VegaChart component so we don't crash the whole page should another issue with Vega bubble up.

Co-authored-by: Walter Rafelsberger <walter@elastic.co>
kibanamachine added a commit that referenced this pull request Apr 30, 2021
…atterplot matrix. (#98763) (#98893)

- Fixes correctly escaping the characters .[] in field names with double backslashes since Vega treats dots/brackets in field names special to be able to access attributes in object structures. This replaces the old approach that replaced dots with a similar but different UTF-8 character but missed the brackets.
- Additionally adds an EuiErrorBoundary component around the VegaChart component so we don't crash the whole page should another issue with Vega bubble up.

Co-authored-by: Walter Rafelsberger <walter@elastic.co>
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:Data Frame Analytics ML data frame analytics features :ml release_note:fix v7.13.0 v7.14.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ML] DFA creation: blank screen once job type is selected for index with special characters

5 participants