Add metadata transformations with metadata tuples #1148

AndreKurait · 2024-11-19T16:30:38Z

Description

Add metadata transformations with metadata tuples.

Adds SKIPPED migration failure type

Category: New feature
Why these changes are required? Users may require metadata transformations across versions, particularly to support multi-type indices.
What is the old behavior before changes and new behavior after changes? No Metadata tuples, no shown output for skipped indexes/templates, no transformation support in metadata

Issues Resolved

#1115

https://opensearch.atlassian.net/browse/MIGRATIONS-2157

Is this a backport? If so, please add backport PR # and/or commits #

Testing

Unit tests added, aws testing for log format and location

Check List

New functionality includes testing
- All tests pass, including unit test, integration test and doctest
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2024-11-19T17:13:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.94%. Comparing base (1c23ab4) to head (18e83bb).
Report is 6 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1148      +/-   ##
============================================
+ Coverage     80.77%   80.94%   +0.16%     
- Complexity     2952     2995      +43     
============================================
  Files           400      407       +7     
  Lines         15111    15229     +118     
  Branches       1021     1023       +2     
============================================
+ Hits          12206    12327     +121     
+ Misses         2287     2273      -14     
- Partials        618      629      +11

Flag	Coverage Δ
unittests	`80.94% <ø> (+0.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

RFS/src/main/java/org/opensearch/migrations/bulkload/worker/IndexRunner.java

MetadataMigration/src/main/java/org/opensearch/migrations/commands/Evaluate.java

peternied · 2024-11-20T13:32:32Z

MetadataMigration/src/main/java/org/opensearch/migrations/commands/MigratorEvaluatorBase.java


 import lombok.extern.slf4j.Slf4j;

 /** Shared functionality between migration and evaluation commands */
 @Slf4j
 public abstract class MigratorEvaluatorBase {
+    public static final String NOOP_TRANSFORMATION_CONFIG = "[" +


Lets remove this, if there is no transformation config why create and use the factory?

...va/org/opensearch/migrations/bulkload/transformers/TransformerToIJsonTransformerAdapter.java

chelma

Thanks for the work here! Looking forward to seeing some tests to help understand the user experience a bit better. Overall, feels like a good first step as we continue to iterate and evolve our vision for transformations.

chelma · 2024-11-20T14:31:10Z

...va/org/opensearch/migrations/bulkload/transformers/TransformerToIJsonTransformerAdapter.java

+        var transformedTemplates = Stream.concat(Stream.concat(
+                legacyTemplates.stream(),
+                indexTemplates.stream()),
+                componentTemplates.stream()
+                )
+                .map(this::transformMigrationItem).collect(Collectors.toList());


Why combine all the different migration item types into an undifferentiated stream, transform, then split them up again? Wouldn't it be easier and more efficient to just apply the transform to each class individually?

Here, we are supporting the case where someone migrates from legacy templates to index templates

chelma · 2024-11-20T14:33:58Z

...va/org/opensearch/migrations/bulkload/transformers/TransformerToIJsonTransformerAdapter.java

+        log.atInfo().setMessage("BeforeJsonGlobal: {}").addArgument(() -> printMap(inputJson)).log();
+        var afterJson = transformer.transformJson(inputJson);
+        log.atInfo().setMessage("AfterJsonGlobal: {}").addArgument(() -> printMap(afterJson)).log();


It's interesting to have a pre-processor transform for the global metadata but not the index settings. Is this an opinionated stance or just a matter of current necessity? Also - how does this fit into the bigger picture we previously discussed of having user-supplied bulk pre/post processing transforms then line-item transforms for each individual MigrationItem between those two steps?

I've cleaned this up in the latest version, the unit of transformation for index settings is the entire object, whereas for global metadata it's the sub-object (individual template).

.../main/java/org/opensearch/migrations/bulkload/version_es_7_10/IndexMetadataData_ES_7_10.java

RFS/src/main/java/org/opensearch/migrations/bulkload/worker/IndexRunner.java

peternied

Thanks for the tests, looks like maybe there is some kind of refactoring we can do in the future to cleanup the setup / verification that is very similar between the tests, but this gives me a lot of confidence that we will know when we break something - much appreciated.

Seems like there is a one off system index that is causing the new test cases to fail and some style cop changes, but the bones of this good to me once CI is passing.

Signed-off-by: Andre Kurait <[email protected]>

AndreKurait force-pushed the MetadataTransforms branch from fbecea3 to 488c664 Compare November 19, 2024 16:31

AndreKurait marked this pull request as ready for review November 19, 2024 18:00

AndreKurait requested review from chelma, gregschohn, lewijacn, mikaylathompson, peternied and sumobrian as code owners November 19, 2024 18:00

peternied reviewed Nov 20, 2024

View reviewed changes

chelma reviewed Nov 20, 2024

View reviewed changes

peternied approved these changes Nov 22, 2024

View reviewed changes

AndreKurait added 4 commits November 25, 2024 14:48

Add metadata transformations with metadata tuples

2e8ea46

Signed-off-by: Andre Kurait <[email protected]>

Update metadata transformations based on PR feedback

ad1ba91

Signed-off-by: Andre Kurait <[email protected]>

Fix spotless

be777d6

Signed-off-by: Andre Kurait <[email protected]>

Assert on successful migration items

769b107

Signed-off-by: Andre Kurait <[email protected]>

AndreKurait force-pushed the MetadataTransforms branch from fa7896e to 22b509f Compare November 25, 2024 20:48

Correctly assert against ALREADY_EXISTS index

18e83bb

Signed-off-by: Andre Kurait <[email protected]>

AndreKurait force-pushed the MetadataTransforms branch from 22b509f to 18e83bb Compare November 25, 2024 23:14

AndreKurait merged commit 92f0d3f into opensearch-project:main Nov 26, 2024
21 of 22 checks passed

AndreKurait deleted the MetadataTransforms branch November 26, 2024 04:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metadata transformations with metadata tuples #1148

Add metadata transformations with metadata tuples #1148

AndreKurait commented Nov 19, 2024 •

edited

Loading

codecov bot commented Nov 19, 2024 •

edited

Loading

peternied Nov 20, 2024

chelma left a comment

chelma Nov 20, 2024

AndreKurait Nov 25, 2024

chelma Nov 20, 2024

AndreKurait Nov 25, 2024

peternied left a comment

Add metadata transformations with metadata tuples #1148

Add metadata transformations with metadata tuples #1148

Conversation

AndreKurait commented Nov 19, 2024 • edited Loading

Description

Issues Resolved

Testing

Check List

codecov bot commented Nov 19, 2024 • edited Loading

Codecov Report

peternied Nov 20, 2024

Choose a reason for hiding this comment

chelma left a comment

Choose a reason for hiding this comment

chelma Nov 20, 2024

Choose a reason for hiding this comment

AndreKurait Nov 25, 2024

Choose a reason for hiding this comment

chelma Nov 20, 2024

Choose a reason for hiding this comment

AndreKurait Nov 25, 2024

Choose a reason for hiding this comment

peternied left a comment

Choose a reason for hiding this comment

AndreKurait commented Nov 19, 2024 •

edited

Loading

codecov bot commented Nov 19, 2024 •

edited

Loading