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.

Sign up for GitHub

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

Cleanup enabled: false saved objects mappings #149102

Merged
merged 15 commits into from
Feb 9, 2023
Merged

Cleanup enabled: false saved objects mappings #149102

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
19e63eb
Remove unused type maps-telemetry
rudolf Jan 18, 2023
46d8c13
Remove enabled:false mappings
rudolf Jan 18, 2023
e2b1316
Merge branch 'main' into cleanup-so-mappings
rudolf Jan 19, 2023
fa6339a
Remove nested type from ingest-package-policies mappings
rudolf Jan 19, 2023
1aa1e94
Remove type: object, enabled: false mappings pass 2
rudolf Jan 19, 2023
5767058
Update registered SO types snapshot
rudolf Jan 24, 2023
0f1fcd5
Merge branch 'main' into cleanup-so-mappings
rudolf Jan 24, 2023
d43a0a5
Revert changes to non-saved object reporting mappings
rudolf Jan 24, 2023
798de73
Merge branch 'main' into cleanup-so-mappings
rudolf Jan 24, 2023
c8335bf
Merge branch 'main' into cleanup-so-mappings
rudolf Jan 27, 2023
647c639
Merge branch 'main' into cleanup-so-mappings
rudolf Jan 30, 2023
d165f61
Merge branch 'main' into cleanup-so-mappings
rudolf Jan 30, 2023
3dd40d7
Merge branch 'main' into cleanup-so-mappings
rudolf Jan 30, 2023
8d158be
Merge branch 'main' into cleanup-so-mappings
rudolf Feb 8, 2023
a977e92
Merge branch 'main' into cleanup-so-mappings
rudolf Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions ...-migration-server-internal/src/__snapshots__/migrations_state_action_machine.test.ts.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions .../core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export const REMOVED_TYPES: string[] = [
'ui-counter',
// Deprecated, no longer used since 7.13 https://github.com/elastic/kibana/pull/94923/files
'application_usage_transactional',
// Removed in 7.8.1 / 7.9.0 https://github.com/elastic/kibana/pull/69871
'maps-telemetry',
].sort();

// When migrating from the outdated index we use a read query which excludes
Expand Down
5 changes: 5 additions & 0 deletions ...core/saved-objects/core-saved-objects-migration-server-internal/src/initial_state.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ describe('createInitialState', () => {
"type": "guided-setup-state",
},
},
Object {
"term": Object {
"type": "maps-telemetry",
},
},
Object {
"term": Object {
"type": "ml-telemetry",
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/integration_tests/saved_objects/migrations/7.7.2_xpack_100k.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ async function removeLogFile() {
}

/** Number of SO documents dropped during the migration because they belong to an unused type */
const UNUSED_SO_COUNT = 4;
const UNUSED_SO_COUNT = 5;

describe('migration from 7.7.2-xpack with 100k objects', () => {
let esServer: TestElasticsearchUtils;
Expand Down
15 changes: 7 additions & 8 deletions src/core/server/integration_tests/saved_objects/migrations/check_registered_types.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
Object {
"action": "7858e6d5a9f231bf23f6f2e57328eb0095b26735",
"action_task_params": "bbd38cbfd74bf6713586fe078e3fa92db2234299",
"alert": "f2e81863be0b50966b876b88b906c962e30b8c9c",
"alert": "f4767f8699442d563c2975830f740906d62cffbd",
"api_key_pending_invalidation": "9b4bc1235337da9a87ef05a1d1f4858b2a3b77c6",
"apm-indices": "ceb0870f3a74e2ffc3a1cd3a3c73af76baca0999",
"apm-server-schema": "2bfd2998d3873872e1366458ce553def85418f91",
Expand All @@ -70,7 +70,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"canvas-workpad": "4b05f7829bc805bbaa07eb9fc0d2a2bbbd6bbf39",
"canvas-workpad-template": "d4bb65aa9c4a2b25029d3272fd9c715d8e4247d7",
"cases": "a27d57e75e358349a6ba835152fd4de0033a7bff",
"cases-comments": "d7c4c1d24e97620cd415e27e5eb7d5b5f2c5b461",
"cases-comments": "6c7a572e75db9336274a6b7d372667cb44f4aaad",
"cases-configure": "1afc414f5563a36e4612fa269193d3ed7277c7bd",
"cases-connector-mappings": "4b16d440af966e5d6e0fa33368bfa15d987a4b69",
"cases-telemetry": "16e261e7378a72acd0806f18df92525dd1da4f37",
Expand All @@ -85,7 +85,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"endpoint:user-artifact": "f94c250a52b30d0a2d32635f8b4c5bdabd1e25c0",
"endpoint:user-artifact-manifest": "8c14d49a385d5d1307d956aa743ec78de0b2be88",
"enterprise_search_telemetry": "fafcc8318528d34f721c42d1270787c52565bad5",
"epm-packages": "7d80ba3f1fcd80316aa0b112657272034b66d5a8",
"epm-packages": "61c34aed078aac04f5cd572162d4f761d6e62a66",
"epm-packages-assets": "9fd3d6726ac77369249e9a973902c2cd615fc771",
"event_loop_delays_daily": "d2ed39cf669577d90921c176499908b4943fb7bd",
"exception-list": "fe8cc004fd2742177cdb9300f4a67689463faf9c",
Expand All @@ -105,15 +105,14 @@ describe('checking migration metadata changes on all registered SO types', () =>
"ingest-agent-policies": "54d586fdafae83ba326e47d1a3727b0d9c910a12",
"ingest-download-sources": "1e69dabd6db5e320fe08c5bda8f35f29bafc6b54",
"ingest-outputs": "29181ecfdc7723f544325ecef7266bccbc691a54",
"ingest-package-policies": "d93048bf153f9043946e8965065a88014f7ccb41",
"ingest-package-policies": "598cb46d731b4447bdee94659b4821c0c37f84a9",
"ingest_manager_settings": "6f36714825cc15ea8d7cda06fde7851611a532b4",
"inventory-view": "bc2bd1e7ec7c186159447ab228d269f22bd39056",
"kql-telemetry": "29544cd7d3b767c5399878efae6bd724d24c03fd",
"legacy-url-alias": "7172dfd54f2e0c89fe263fd7095519b2d826a930",
"lens": "236ecd358ed3a4ecfc03ed676d958b64acf0b697",
"lens-ui-telemetry": "df2844565c9e18fed2bdb1f6cc3aadd58cf1e45b",
"map": "00ca6c4cf46ae59f70f1436262eb9f457b45eb14",
"maps-telemetry": "5adbde35bd50ec2b8e9ea5b96d4d9f886e31ecfb",
"metrics-explorer-view": "09e56993352b8ee678e88f71e4410d9aeee72f3a",
"ml-job": "2836da98a81bd220db61c0549e8e28da7a876cb2",
"ml-module": "95055522c8406afa67a554690a43506f6c040744",
Expand All @@ -123,11 +122,11 @@ describe('checking migration metadata changes on all registered SO types', () =>
"osquery-pack": "a2d675c7af4208e54a5b28d23d324d7c599a5491",
"osquery-pack-asset": "de8783298eb33a577bf1fa0caacd42121dcfae91",
"osquery-saved-query": "7b213b4b7a3e59350e99c50e8df9948662ed493a",
"query": "4640ef356321500a678869f24117b7091a911cb6",
"query": "e9d18422d3df74e366e68d5e6fd977219309d14d",
"rules-settings": "1af4c9abd4b40a154e233c2af4867df7aab7ac24",
"sample-data-telemetry": "8b10336d9efae6f3d5593c4cc89fb4abcdf84e04",
"search": "c48f5ab5d94545780ea98de1bff9e39f17f3606b",
"search-session": "ba383309da68a15be3765977f7a44c84f0ec7964",
"search": "a8d45880d9ce712e8e9a4aa46f5dc5963731d526",
"search-session": "012cf49851285c03c89bfd2c6535449282bc785d",
"search-telemetry": "beb3fc25488c753f2a6dcff1845d667558712b66",
"security-rule": "e0dfdba5d66139d0300723b2e6672993cd4a11f3",
"security-solution-signals-migration": "e65933e32926e0ca385415bd44fc6da0b6d3d419",
Expand Down
7 changes: 5 additions & 2 deletions src/plugins/data/server/saved_objects/query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ export const querySavedObjectType: SavedObjectsType = {
query: {
properties: { language: { type: 'keyword' }, query: { type: 'keyword', index: false } },
},
filters: { type: 'object', enabled: false },
timefilter: { type: 'object', enabled: false },
filters: {
dynamic: false,
properties: {},
},
timefilter: { dynamic: false, properties: {} },
},
},
migrations: savedQueryMigrations,
Expand Down
12 changes: 6 additions & 6 deletions src/plugins/data/server/search/saved_objects/search_session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ export const searchSessionSavedObjectType: SavedObjectsType = {
type: 'keyword',
},
initialState: {
type: 'object',
enabled: false,
dynamic: false,
properties: {},
},
restoreState: {
type: 'object',
enabled: false,
dynamic: false,
properties: {},
},
idMapping: {
type: 'object',
enabled: false,
dynamic: false,
properties: {},
},
realmType: {
type: 'keyword',
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/saved_search/server/saved_objects/search.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function getSavedSearchObjectType(
},
sort: { type: 'keyword', index: false, doc_values: false },
title: { type: 'text' },
grid: { type: 'object', enabled: false },
grid: { dynamic: false, properties: {} },
version: { type: 'integer' },
rowHeight: { type: 'text' },
timeRestore: { type: 'boolean', index: false, doc_values: false },
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/alerting/server/saved_objects/mappings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ export const alertMappings: SavedObjectsTypeMappingDefinition = {
type: 'keyword',
},
params: {
enabled: false,
type: 'object',
dynamic: false,
properties: {},
},
frequency: {
properties: {
Expand Down
6 changes: 2 additions & 4 deletions x-pack/plugins/cases/server/saved_object_types/comments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,14 @@ export const createCaseCommentSavedObjectType = ({
},
externalReferenceMetadata: {
dynamic: false,
type: 'object',
enabled: false,
properties: {},
},
persistableStateAttachmentTypeId: {
type: 'keyword',
},
persistableStateAttachmentState: {
dynamic: false,
type: 'object',
enabled: false,
properties: {},
},
pushed_at: {
type: 'date',
Expand Down
56 changes: 11 additions & 45 deletions x-pack/plugins/fleet/server/saved_objects/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,48 +177,14 @@ const getSavedObjectTypes = (
},
},
elasticsearch: {
enabled: false,
properties: {
privileges: {
properties: {
cluster: { type: 'keyword' },
},
},
},
dynamic: false,
properties: {},
},
vars: { type: 'flattened' },
inputs: {
type: 'nested',
enabled: false,
properties: {
type: { type: 'keyword' },
policy_template: { type: 'keyword' },
enabled: { type: 'boolean' },
vars: { type: 'flattened' },
config: { type: 'flattened' },
compiled_input: { type: 'flattened' },
streams: {
type: 'nested',
properties: {
id: { type: 'keyword' },
enabled: { type: 'boolean' },
data_stream: {
properties: {
dataset: { type: 'keyword' },
type: { type: 'keyword' },
elasticsearch: {
properties: {
privileges: { type: 'flattened' },
},
},
},
},
vars: { type: 'flattened' },
config: { type: 'flattened' },
compiled_stream: { type: 'flattened' },
},
},
},
type: 'nested', // TODO Review: If we're not searching `nested` isn't necessary but it's not possible to change the type later
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elastic/fleet

Nested mappings is rather "expensive" because it creates a document for each object in the array. It also enforces other limits like 50 nested fields per index and only 10000 objects per array.
https://www.elastic.co/guide/en/elasticsearch/reference/current/nested.html#_limits_on_nested_mappings_and_objects

Given that we previously had enabled: false we were never querying these fields. By extension we wouldn't need the advantages of the nested type either. Which makes me lean towards removing it.

BUT it's not possible to change the type later on (it's an incompatible mapping change) so if we have any reason to believe we'd need to run nested queries on the inputs in the future it would be better keeping it.

Copy link
Member

Choose a reason for hiding this comment

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

@rudolf - Thanks for the ping. I can't imagine we'd need to run nested queries on this inputs field, so I'd be in favor of just biting the bullet on this mapping change to just dynamic: false + properties: {} like the others now.

dynamic: false,
properties: {},
},
revision: { type: 'integer' },
updated_at: { type: 'date' },
Expand Down Expand Up @@ -256,8 +222,8 @@ const getSavedObjectTypes = (
internal: { type: 'boolean' },
keep_policies_up_to_date: { type: 'boolean', index: false },
es_index_patterns: {
enabled: false,
type: 'object',
dynamic: false,
properties: {},
},
verification_status: { type: 'keyword' },
verification_key_id: { type: 'keyword' },
Expand All @@ -270,13 +236,13 @@ const getSavedObjectTypes = (
},
},
installed_kibana: {
type: 'object',
enabled: false,
dynamic: false,
properties: {},
},
installed_kibana_space_id: { type: 'keyword' },
package_assets: {
type: 'object',
enabled: false,
dynamic: false,
properties: {},
},
install_started_at: { type: 'date' },
install_version: { type: 'keyword' },
Expand Down
17 changes: 0 additions & 17 deletions x-pack/plugins/maps/server/saved_objects/setup_saved_objects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,6 @@ export function setupSavedObjects(
);
},
});

Copy link
Contributor

@nreese nreese Jan 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, this resolves #139347 for maps.

Question, will this impact upgrades? If a cluster has set this value in previous versions, will the cluster be able to upgrade? We have seen some SDHs regarding upgrade failures for maps-telemetry and clusters that have previously set this value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By adding it to the REMOVED_TYPES list in packages/core/saved-objects/core-saved-objects-migration-server-internal/src/core/unused_types.ts any remaining maps-telemetry documents will be deleted upon the next upgrade. So it won't impact upgrades.

/*
* The maps-telemetry saved object type isn't used, but in order to remove these fields from
* the mappings we register this type with `type: 'object', enabled: true` to remove all
* previous fields from the mappings until https://github.com/elastic/kibana/issues/67086 is
* solved.
*/
core.savedObjects.registerType({
name: 'maps-telemetry',
hidden: false,
namespaceType: 'agnostic',
mappings: {
// @ts-ignore Core types don't support this since it's only really valid when removing a previously registered type
type: 'object',
enabled: false,
},
});
}

/**
Expand Down