-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Debug log the latest saved object migrations per type #89722
Conversation
Pinging @elastic/kibana-core (Team:Core) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just one question/NIT I'd like to be clear about.
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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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; | |
}, {}); |
There was a problem hiding this comment.
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.
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
* Debug log the latest saved object migrations per type * Fix broken test
Summary
Closes: #86320
Adds the following debug logging to make it easier to see which saved object types will be migrated and what the expected migrationVersion should be after the migration was applied:
Checklist
Delete any items that are not applicable to this PR.
For maintainers