Improve handling of invalid data during migrations

[stacey-gammon]

Background

We currently handle invalid data during migrations inconsistently. Sometimes we throw an error, sometimes we just return the invalid data.

This makes it difficult for us to know what type of data we might be dealing with and increases the chances for migrations errors when our assumptions turn out to be wrong.

Goal

Be able to make assumptions about the shape of our data in any given release.

Proposal

If we start just throwing errors when we encounter unknown data, this could prevent users from upgrading, so we should get there slowly. If we increase our knowledge of the shape of data out in the wild, we should be able to account for that in future migrations until we are finally at a spot we can say we are confident we know the shape of the data.

• Log warnings when a saved object is encountered with unexpected data.
• Send telemetry when unexpected data is encountered.
• When telemetry data starts coming in, we can adjust migrations for any unaccounted data that is actually valid.
• We then log a deprecation warning when invalid data is encountered that the user will not be able to upgrade if they won't manually adjust, or delete, the invalid saved objects.

Related

• More resilient error handling for migrations #26143
• Persist migration errors and automatically retry on failure #26144

@spalger - I see we have had similar thoughts!

[elasticmachine]

Pinging @elastic/kibana-platform

[rudolf]

I agree that strictly defining what data we expect and throwing a runtime error if unexpected data appears will improve the quality and readability of the migrations.

The same problems you mentioned with not knowing the shape of the input data also filters through to any part of the code base that uses SavedObjects. This would require a bit more work, but I think it could be very valuable if plugins were required to define runtime types using something like https://github.com/gcanti/io-ts to define the shape of their SavedObjects. That would provide a strong guarantee to all consuming code (including migrations) about the shape of the data.

[stacey-gammon]

Thinking more about this... I think we'd probably want to move to something like this slowly.

It's my understanding that throwing an error will prevent Kibana from starting up. A client who upgrades from 7.x to 7.x.+1 could potentially suddenly see their Kibana failing to start due to invalid data that was previously ignored.

A better plan may be to:

7.x:

• Mark invalid saved objects as such { invalid: true } in es.
• Provide a warning in kibana logs, and possibly the UI. Call out that starting in 8.0 if these objects are not removed, Kibana will fail to start. Perhaps mention contacting your support team if this seems to be in error.

8.0:

• We throw the error and stop migrations on invalid data. We can now be assured that any instances running on 8.0 that pass migrations have validated data.

Benefits:

• In migrations right now, we just return the doc if we hit invalid data. Any additional migrations that need to run are still attempted. This is a waste of time, we can avoid if we mark the saved object as invalid.
• Gives our support team time to react to possible mistakes in our migration code. It will be difficult to know what kibana indices actually exist out in the wild. If we mark the invalid saved objects, and tell users to contact support, we have a chance to see what we are dealing with and how many users it would affect, and adjust our migration scripts to handle some that invalid data, and massage it into a valid shape.

[stacey-gammon]

I had another idea, we could add telemetry to also see how often invalid data would be hit in the wild.

nevermind, trackUiMetric is not available at the time migrations are run.

regardless, I think it'd be great if migrations could throw errors and the migration system itself would catch those errors, track them somehow, log the error to the console/file, and then continue on with the migrations with a best attempt.