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

[Graph] Fix graph saved object references #85295

Merged
merged 3 commits into from
Dec 10, 2020

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Dec 8, 2020

Fixes #34989
Fixes #34997

This PR fixes saved object references for Graph and makes them importable/exportable.

The problem

Right now, graph does use the reference system, but it's not storing the proper id, but the title of the index pattern instead:

[
  { "type": "index-pattern", "name": "indexPattern_0", "id": "titleOfThePattern*" }
]

In the serialized state, an indexPatternRefName is stored:

{
  "vertices": [...],
  "indexPatternRefName": "indexPattern_0"
}

In x-pack/plugins/graph/public/services/persistence/saved_workspace_references.ts the reference is placed inside the state.

This part of the code wasn't touched in this PR, as it's already functioning as it should, the only issue being it contains the title instead of the id of the index pattern.

If a user would try to export/import a workspace saved object, they would get errors because the referenced index pattern can't be found.

The solution

This PR resolves this by doing the following changes:

  • Migrate all existing saved objects to remove the broken reference and add a legacyIndexPatternRef property (x-pack/plugins/graph/server/saved_objects/migrations.ts)
  • If a saved graph workspace is loaded, migrateLegacyIndexPatternRef (x-pack/plugins/graph/public/services/persistence/deserialize.ts) is called, which tries to map the index pattern title to the id by searching for it in a list of all index pattern which is anyway fetched for the Graph UI. If it is successful, legacyIndexPatternRef is removed, and the index pattern id is stored in the deserialized workspace state
  • If the workspace is saved again, the id is placed in the references array instead of the title via the existing logic in (see x-pack/plugins/graph/public/services/persistence/serialize.ts and x-pack/plugins/graph/public/services/persistence/saved_workspace_references.ts)
  • New workspaces and legacy workspaces which are opened and saved at least once have consistent references, while unopened workspaces don't have any references at all. If they are exported and imported without a matching index pattern by name, an error message is shown

Other changes

  • xpack.graph.loadWorkspace.missingIndexPatternErrorMessage got improved by including the name of the index pattern causing the problem (either if loading it fails for some reason or if a legacy workspace is opened and the legacyIndexPatternRef can't be mapped)
  • In x-pack/plugins/graph/server/saved_objects/graph_workspace.ts workspace saved objects are whitelisted for importing/exporting
  • In x-pack/plugins/graph/server/plugin.ts a new UI capability "show" is added to make it possible to enable the edit link in the saved object management page for graph workspaces if the user has at least "read" permissions for the Graph app

@flash1293 flash1293 added Feature:Graph Graph application feature release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0 labels Dec 8, 2020
@flash1293 flash1293 marked this pull request as ready for review December 9, 2020 08:07
@flash1293 flash1293 requested a review from a team December 9, 2020 08:07
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

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.

Code reviewed 👍 . Only minor comments

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@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
graph 1.3MB 1.3MB +856.0B

Distributable file count

id before after diff
default 46981 47741 +760

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
graph-workspace 9 10 +1

History

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

@flash1293 flash1293 merged commit 2038582 into elastic:master Dec 10, 2020
flash1293 added a commit to flash1293/kibana that referenced this pull request Dec 10, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 10, 2020
* master: (53 commits)
  Fixing recovered instance reference bug (elastic#85412)
  Switch to new elasticsearch client for Visualizations (elastic#85245)
  Switch to new elasticsearch client for TSVB (elastic#85275)
  Switch to new elasticsearch client for Vega (elastic#85280)
  [ILM] Add shrink field to hot phase (elastic#84087)
  Add rolling-file appender to core logging (elastic#84735)
  [APM] Service overview: Dependencies table (elastic#83416)
  [Uptime ]Update empty message for certs list (elastic#78575)
  [Graph] Fix graph saved object references (elastic#85295)
  [APM] Create new API's to return Latency and Throughput charts (elastic#85242)
  [Advanced settings] Reset to default for empty strings (elastic#85137)
  [SECURITY SOLUTION] Bundles _source -> Fields + able to sort on multiple fields in Timeline (elastic#83761)
  [Fleet] Update agent listing for better status reporting (elastic#84798)
  [APM] enable 'sanitize_field_names' for Go (elastic#85373)
  Update dependency @elastic/charts to v24.4.0 (elastic#85452)
  Introduce external url service (elastic#81234)
  Deprecate disabling the security plugin (elastic#85159)
  [FLEET] New Integration Policy Details page for use in Integrations section (elastic#85355)
  [Security Solutions][Detection Engine] Fixes one liner access control with find_rules REST API
  chore: 🤖 remove extraPublicDirs (elastic#85454)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Graph Graph application feature release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0
Projects
None yet
4 participants