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

Debug log the latest saved object migrations per type #89722

Merged
merged 3 commits into from
Feb 2, 2021
Merged

Debug log the latest saved object migrations per type #89722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eeb461e
Debug log the latest saved object migrations per type
rudolf Jan 29, 2021
94e06ce
Merge branch 'master' into debug-log-registered-migrations
rudolf Feb 1, 2021
d5274e1
Fix broken test
rudolf Feb 1, 2021
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
4 changes: 2 additions & 2 deletions src/core/server/saved_objects/migrations/kibana/kibana_migrator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ describe('KibanaMigrator', () => {

const migrator = new KibanaMigrator(options);

expect(() => migrator.runMigrations()).rejects.toThrow(
/Migrations are not ready. Make sure prepareMigrations is called first./i
await expect(() => migrator.runMigrations()).toThrowErrorMatchingInlineSnapshot(
`"Migrations are not ready. Make sure prepareMigrations is called first."`
);
});

Expand Down
10 changes: 10 additions & 0 deletions src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*/

import { BehaviorSubject } from 'rxjs';
import Semver from 'semver';
import { KibanaConfigType } from '../../../kibana_config';
import { ElasticsearchClient } from '../../../elasticsearch';
import { Logger } from '../../../logging';
Expand Down Expand Up @@ -163,6 +164,15 @@ export class KibanaMigrator {
registry: this.typeRegistry,
});

this.log.debug('Applying registered migrations for the following saved object types:');
Object.entries(this.documentMigrator.migrationVersion)
.sort(([t1, v1], [t2, v2]) => {
return Semver.compare(v1, v2);
Copy link
Member

Choose a reason for hiding this comment

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

NIT: are v1 and v2 ensured to always exist? If any don't, can this logic break the rest of the migration logic?

Copy link
Contributor

Choose a reason for hiding this comment

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

The code is hard to follow, but from what I understand, I think that this.documentMigrator.migrationVersion will always have a version, but that types without any migration registered will not be present in the map. Which sounds acceptable @rudolf?

kibana/src/core/server/saved_objects/migrations/core/document_migrator.ts

Lines 388 to 390 in 8534faf

if (!transforms.length) {
return migrations;
}

kibana/src/core/server/saved_objects/migrations/core/document_migrator.ts

Lines 178 to 189 in 8534faf

public get migrationVersion(): SavedObjectsMigrationVersion {
if (!this.migrations) {
throw new Error('Migrations are not ready. Make sure prepareMigrations is called first.');
}
return Object.entries(this.migrations).reduce((acc, [prop, { latestMigrationVersion }]) => {
// some migration objects won't have a latestMigrationVersion (they only contain reference transforms that are applied from other types)
if (latestMigrationVersion) {
return { ...acc, [prop]: latestMigrationVersion };
}
return acc;
}, {});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, these versions come from plugins registering migrations, so it will always be present (and I think we validate that it's a valid semver). If a type doesn't register a migration it won't be in this map.

})
.forEach(([type, migrationVersion]) => {
this.log.debug(`migrationVersion: ${migrationVersion} saved object type: ${type}`);
});

const migrators = Object.keys(indexMap).map((index) => {
// TODO migrationsV2: remove old migrations algorithm
if (this.savedObjectsConfig.enableV2) {
Expand Down