SO migrations: Improves transformation error creation and testing #100297

TinaHeiligers · 2021-05-18T20:38:30Z

Summary

Enhances the work on saved object migrations done in #96986.
Resolves #99685

Changes/improvements handled:

Use the originalError's message as the top level TransformSavedObjectDocumentError message.
Refactors extractTransformFailuresReason to construct the full, final migration failure reason.
Integration test improvements:
- Uses a 7.13.0 index to test the changes introduced to REINDEX_SOURCE_TO_TEMP_*.
- Ensures we get all the failed document ids in the logs along with the full failure reason.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

…an up transform failure message

…ns fail

…hance

…ransforms to throw

…hance

TinaHeiligers · 2021-05-18T20:40:25Z

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

-        failedDoc,
-        error
-      );
+      throw new TransformSavedObjectDocumentError(failedTransform, failedDoc, error);


We already have the raw doc id in migrateRawDocsSafely and don't need the extra properties.

TinaHeiligers · 2021-05-18T20:43:00Z

src/core/server/saved_objects/migrations/core/transform_saved_object_document_error.test.ts

-      Doc: failedDoc"
-    `
-    );
+    expect(err.message).toMatchInlineSnapshot(`"Dang diggity!"`);


We use the original transformation error message and don't construct one anymore.

TinaHeiligers · 2021-05-18T22:57:25Z

@elasticmachine merge upstream

…v2/transform-failures-test-enhance

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

mshustov · 2021-05-19T15:15:46Z

src/core/server/saved_objects/migrationsv2/integration_tests/corrupt_outdated_docs.test.ts

-        ].length
-      ).toEqual(16);
-    }
+    await expect(root.start()).rejects.toThrowError();


Why don't use the old approach? Reading from the logs adds flakiness (logging nondeterminism, the test depends on the logging format, etc.)

FWIW, I was trying to avoid parsing strings to get the details.
I'll change it back to read from the error message itself if that's more reliable.

…hance

TinaHeiligers · 2021-05-19T20:58:41Z

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

-          "Failed to transform document smelly. Transform: dog:1.2.3
-          Doc: {\\"id\\":\\"smelly\\",\\"type\\":\\"dog\\",\\"attributes\\":{},\\"migrationVersion\\":{}}"
-        `);
+        expect(error.message).toMatchInlineSnapshot(`"Dang diggity!"`);
        expect(error).toBeInstanceOf(TransformSavedObjectDocumentError);


@mshustov after we've aded in the raw SO doc id in migrateRawDocsSafely we push the original error and the rawId on to the array of transformErrors within migrateRawDocsSafely. We assert this in migrate_raw_docs.test.

We also have the rawId for failed transformations in the final migration error (as part of the reason) that's shown to the end user when a migration fails because of document transformation issues.

I could specifically add an assertion to the integration test to show that we do get the raw id. It would require a bit of string parsing though, because each transform error also contains the stack trace. I'll add one but can't think of a way to make the parsing elegant.

We also have the rawId for failed transformations in the final migration error (as part of the reason) that's shown to the end user when a migration fails because of document transformation issues.

Do we know rawId value? If so, we can check that a message contains rawId value.

Do we know the rawId value?

We don't know the rawId value in document_migrator, only the uuid part of it.

…ing from the logs

…v2/transform-failures-test-enhance

...ver/saved_objects/migrationsv2/integration_tests/migration_7_13_0_transform_failures.test.ts

TinaHeiligers · 2021-05-19T23:20:21Z

src/core/server/saved_objects/migrationsv2/model.test.ts

@@ -1184,7 +1177,7 @@ describe('migrations v2 model', () => {
        expect(newState.reason.includes('Migrations failed. Reason:')).toBe(true);
        expect(newState.reason.includes('Corrupt saved object documents: ')).toBe(true);
        expect(newState.reason.includes('Transformation errors: ')).toBe(true);
-        expect(newState.reason.includes('randomvis: 7.12.0')).toBe(true);
+        expect(newState.reason.includes('bob:tail')).toBe(true);


Asserts that we have the raw document id in the error message

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

src/core/server/saved_objects/migrationsv2/integration_tests/corrupt_outdated_docs.test.ts

...ver/saved_objects/migrationsv2/integration_tests/migration_7_13_0_transform_failures.test.ts

…hance

kibanamachine · 2021-05-22T00:53:42Z

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`canvas`	29	25	-4
`maps`	286	208	-78
total			-82

History

💚 Build #126652 succeeded 2c37746
💚 Build #126484 succeeded 2f4b729

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

…astic#100297)

kibanamachine · 2021-05-22T00:56:37Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…00297) (#100435) Co-authored-by: Christiane (Tina) Heiligers <[email protected]>

…astic#100297)

TinaHeiligers added 13 commits May 14, 2021 12:26

Uses the original error as the TransformSavedObjectDocumentError, cle…

cf0d27a

…an up transform failure message

Uses the original error as the TransformSavedObjectDocumentError, cle…

f228398

…an up transform failure message

WIP1

160d255

Adds 7_13 index with expected transform issues and corrupt documents

fdefc63

Ensures transformation failures are caught and reported when migratio…

3be8767

…ns fail

Adds comment about do that fails transformation

7e4c4ed

Merge branch 'master' into so-migrationsv2/transform-failures-test-en…

607e21e

…hance

Adds fixture that contains transformation failure documents

8386508

Uses new 7.13 test fixture with corrupt SO docs and docs that cause t…

993f892

…ransforms to throw

Merge branch 'master' into so-migrationsv2/transform-failures-test-en…

fec170c

…hance

Cleans up integration test

01ce1b4

Adds dev comments

9654d18

Improves corrupt_outdated_docs test

0c57cfa

TinaHeiligers commented May 18, 2021

View reviewed changes

TinaHeiligers added 2 commits May 18, 2021 14:09

Removes unused archives

e812436

Type fix

d03ad29

elastic deleted a comment from kibanamachine May 18, 2021

TinaHeiligers added 2 commits May 18, 2021 16:02

Cleans up corrupt_outdated_docs test

a894c1d

Merge branch 'master' of github.com:elastic/kibana into so-migrations…

8d81a96

…v2/transform-failures-test-enhance

TinaHeiligers force-pushed the so-migrationsv2/transform-failures-test-enhance branch from f056e34 to 8d81a96 Compare May 18, 2021 23:17

Removes unused properties from TransformSavedObjectDocumentError

4bae603

elastic deleted a comment from kibanamachine May 19, 2021

mshustov reviewed May 19, 2021

View reviewed changes

src/core/server/saved_objects/migrations/core/document_migrator.test.ts Outdated Show resolved Hide resolved

mshustov reviewed May 19, 2021

View reviewed changes

TinaHeiligers added 2 commits May 19, 2021 08:29

Merge branch 'master' into so-migrationsv2/transform-failures-test-en…

88667cb

…hance

reverts back to reading the error message from the error itself

9751288

TinaHeiligers commented May 19, 2021

View reviewed changes

TinaHeiligers added 4 commits May 19, 2021 15:19

Asserts directly against the migration error message rather than read…

647bbf9

…ing from the logs

Merge branch 'master' of github.com:elastic/kibana into so-migrations…

1a3eb52

…v2/transform-failures-test-enhance

Removes unused imports

c3a7eba

Updates snapshot

2c37746

TinaHeiligers commented May 19, 2021

View reviewed changes

...ver/saved_objects/migrationsv2/integration_tests/migration_7_13_0_transform_failures.test.ts Outdated Show resolved Hide resolved

TinaHeiligers commented May 19, 2021

View reviewed changes

TinaHeiligers requested review from mshustov and a team May 20, 2021 13:05

mshustov approved these changes May 21, 2021

View reviewed changes

mshustov reviewed May 21, 2021

View reviewed changes

...ver/saved_objects/migrationsv2/integration_tests/migration_7_13_0_transform_failures.test.ts Outdated Show resolved Hide resolved

TinaHeiligers added 3 commits May 21, 2021 10:38

Merge branch 'master' into so-migrationsv2/transform-failures-test-en…

3b2c797

…hance

Improves test assertions

006b5df

Updates so migrations integration test

c3efd50

TinaHeiligers added the auto-backport Deprecated - use backport:version if exact versions are needed label May 21, 2021

TinaHeiligers enabled auto-merge (squash) May 21, 2021 23:07

TinaHeiligers merged commit 26272fa into elastic:master May 22, 2021

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 22, 2021

SO migrations: Improves transformation error creation and testing (el…

cd0276c

…astic#100297)

kibanamachine mentioned this pull request May 22, 2021

[7.x] SO migrations: Improves transformation error creation and testing (#100297) #100435

Merged

TinaHeiligers deleted the so-migrationsv2/transform-failures-test-enhance branch May 22, 2021 01:24

kibanamachine added a commit that referenced this pull request May 22, 2021

SO migrations: Improves transformation error creation and testing (#1…

b2297d8

…00297) (#100435) Co-authored-by: Christiane (Tina) Heiligers <[email protected]>

yctercero pushed a commit to yctercero/kibana that referenced this pull request May 25, 2021

SO migrations: Improves transformation error creation and testing (el…

01ab915

…astic#100297)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SO migrations: Improves transformation error creation and testing #100297

SO migrations: Improves transformation error creation and testing #100297

TinaHeiligers commented May 18, 2021 •

edited

Loading

TinaHeiligers May 18, 2021

TinaHeiligers May 18, 2021

TinaHeiligers commented May 18, 2021

mshustov May 19, 2021

TinaHeiligers May 19, 2021 •

edited

Loading

TinaHeiligers May 19, 2021 •

edited

Loading

mshustov May 21, 2021

TinaHeiligers May 21, 2021

TinaHeiligers May 19, 2021

kibanamachine commented May 22, 2021

References to deprecated APIs

kibanamachine commented May 22, 2021

SO migrations: Improves transformation error creation and testing #100297

SO migrations: Improves transformation error creation and testing #100297

Conversation

TinaHeiligers commented May 18, 2021 • edited Loading

Summary

Checklist

For maintainers

TinaHeiligers May 18, 2021

Choose a reason for hiding this comment

TinaHeiligers May 18, 2021

Choose a reason for hiding this comment

TinaHeiligers commented May 18, 2021

mshustov May 19, 2021

Choose a reason for hiding this comment

TinaHeiligers May 19, 2021 • edited Loading

Choose a reason for hiding this comment

TinaHeiligers May 19, 2021 • edited Loading

Choose a reason for hiding this comment

mshustov May 21, 2021

Choose a reason for hiding this comment

TinaHeiligers May 21, 2021

Choose a reason for hiding this comment

TinaHeiligers May 19, 2021

Choose a reason for hiding this comment

kibanamachine commented May 22, 2021

💚 Build Succeeded

Metrics [docs]

References to deprecated APIs

History

kibanamachine commented May 22, 2021

💚 Backport successful

TinaHeiligers commented May 18, 2021 •

edited

Loading

TinaHeiligers May 19, 2021 •

edited

Loading

TinaHeiligers May 19, 2021 •

edited

Loading