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

Use TS to discourage SO mappings with dynamic: false / dynamic: true #69927

Merged
merged 3 commits into from
Jun 25, 2020

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented Jun 25, 2020

Summary

When Saved Object types are registered with a mapping of dynamic: true there's a big risk that indexing will add hundreds of fields and eventually push the .kibana index over the default ES field limit of 1000. Although dynamic: false won't cause a mappings explosion it's preferable to simply use type: "object", enabled: false so that no fields are being indexed.

We should probably add runtime validation as well, but this provides a quick stop gap for #69985

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@rudolf rudolf added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Saved Objects v8.0.0 v7.8.1 labels Jun 25, 2020
@@ -37,8 +37,8 @@ export const mapsTelemetrySavedObjects: SavedObjectsType = {
avg: { type: 'long' },
},
},
layerTypesCount: { dynamic: 'true', properties: {} },
emsVectorLayersCount: { dynamic: 'true', properties: {} },
layerTypesCount: { type: 'object', enabled: false },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will prevent any queries on fields inside layerTypesCount. If we do need to be able to query this data we will have to change the data that's being indexed so that it conforms to a fixed schema instead of dynamically adding new fields.

@rudolf rudolf marked this pull request as ready for review June 25, 2020 17:53
@rudolf rudolf requested a review from a team June 25, 2020 17:53
@rudolf rudolf requested review from a team as code owners June 25, 2020 17:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 25, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@rudolf rudolf added the release_note:skip Skip the PR/issue when compiling release notes label Jun 25, 2020
@rudolf rudolf removed the request for review from a team June 25, 2020 17:59
Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

@kibanamachine
Copy link
Contributor

💔 Build Failed

Failed CI Steps

Build metrics

✅ unchanged

History

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

@rudolf
Copy link
Contributor Author

rudolf commented Jun 25, 2020

Merging because the typescript failure happens on master too.

@rudolf rudolf merged commit 61a69f3 into elastic:master Jun 25, 2020
@rudolf rudolf deleted the no-dynamic-so-mappings branch June 25, 2020 19:45
rudolf added a commit to rudolf/kibana that referenced this pull request Jun 26, 2020
…lastic#69927)

* Use TS to discourage SO mappings with dynamic

* Some unrelated docs changes
rudolf added a commit to rudolf/kibana that referenced this pull request Jun 26, 2020
…lastic#69927)

* Use TS to discourage SO mappings with dynamic

* Some unrelated docs changes
# Conflicts:
#	x-pack/plugins/ingest_manager/server/saved_objects/index.ts
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 26, 2020
…t-pipelines/editor-error-messages

* ingest-pipelines/editor-dropzone-refinement: (122 commits)
  Fixes for monaco XJSON grammar parser and update form copy
  Added cancel move button
  Refactor SCSS values to variables
  use classNames as it is intended to be used
  Remove box shadow on all nested tree items
  Rename variables and prefix booleans with "is"
  Fixes bug on color picker defaults on TSVB (elastic#69889)
  [DOCS] Fixes wording in Upload a CSV section (elastic#69969)
  [Discover] Validate timerange before submitting query to ES (elastic#69363)
  [Maps] avoid using MAP_SAVED_OBJECT_TYPE constant when defining URL paths (elastic#69723)
  [Maps] Fix icon palettes are not working (elastic#69937)
  [Ingest Manager] Fix typo in constant name (elastic#69919)
  [test] skip status.allowAnonymous tests on cloud (elastic#69017)
  Fix backport (elastic#70003)
  [Reporting] ReportingStore module (elastic#69426)
  Add reporting assets to the eslint ignore file (elastic#69968)
  [Discover] set minBarHeight for high cardinality data (elastic#69875)
  Add featureUsage API to licensing context provider (elastic#69838)
  Fix uncaught typecheck merge conflict (elastic#70001)
  Use TS to discourage SO mappings with dynamic: false / dynamic: true (elastic#69927)
  ...
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 26, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

rudolf added a commit that referenced this pull request Jun 30, 2020
…69927) (#70060)

* Use TS to discourage SO mappings with dynamic

* Some unrelated docs changes

Co-authored-by: Elastic Machine <[email protected]>
rudolf added a commit that referenced this pull request Jun 30, 2020
… true (#69927) (#70062)

* Use TS to discourage SO mappings with dynamic: false / dynamic: true (#69927)

* Use TS to discourage SO mappings with dynamic

* Some unrelated docs changes
# Conflicts:
#	x-pack/plugins/ingest_manager/server/saved_objects/index.ts

* @ts-ignore instead of @ts-expect-error

* ingest_manager PACKAGES_SAVED_OBJECT_TYPE enabled: false

Co-authored-by: Elastic Machine <[email protected]>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Saved Objects release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Fleet Team label for Observability Data Collection Fleet team v7.8.1 v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants