From c66da0e28dd19ca321a7770b11d73ff961840b14 Mon Sep 17 00:00:00 2001 From: Greg Schohn Date: Mon, 13 Nov 2023 18:46:59 -0500 Subject: [PATCH 1/2] =?UTF-8?q?Transformation=20improvements.=20I've=20add?= =?UTF-8?q?ed=20all=20of=20the=20transformer=20jars=20to=20the=20runtime?= =?UTF-8?q?=20closure=20of=20the=20replayer=20since=20it's=20going=20to=20?= =?UTF-8?q?be=20a=20challenge=20for=20users=20using=20a=20cloud-deployment?= =?UTF-8?q?=20shrink-wrap=20solution=20to=20add=20addition=20jarfiles.=20C?= =?UTF-8?q?hange=20the=20syntax/rules=20for=20Transformer=20configs.=20*?= =?UTF-8?q?=20If=20no=20argument=20was=20specified=20for=20the=20transform?= =?UTF-8?q?er=20config,=20no=20transformer=20used,=20regardless=20of=20wha?= =?UTF-8?q?t's=20in=20the=20classpath.=20*=20To=20use=20multiple=20transfo?= =?UTF-8?q?rmers,=20you=E2=80=99ll=20need=20to=20specify=20the=20transform?= =?UTF-8?q?er-config=20as=20a=20json=20array=20(of=20json=20transformer=20?= =?UTF-8?q?configs)=20*=20To=20use=20one=20transformer=20with=20default=20?= =?UTF-8?q?settings,=20you=20can=20JUST=20put=20the=20name=20of=20the=20tr?= =?UTF-8?q?ansformer=20in=20the=20arg.=20=20e.g.=20--transformer-config=20?= =?UTF-8?q?JsonTransformerForOpenSearch23PlusTargetTransformerProvider.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I've also updated the key name to only use the short, simple name of a class rather than the fully qualified one with the namespace. Add a NoopTransformer, which makes unit and sanity testing a lot simpler. Signed-off-by: Greg Schohn --- .../replay/MultipleJoltScriptsTest.java | 6 ++--- .../transform/NoopTransformerProvider.java | 17 ++++++++++++++ ...rations.transform.IJsonTransformerProvider | 1 + TrafficCapture/trafficReplayer/build.gradle | 7 +++++- .../replay/TransformationLoader.java | 23 ++++++++++--------- .../replay/HeaderTransformerTest.java | 2 +- .../replay/TransformationLoaderTest.java | 9 ++++++++ 7 files changed, 49 insertions(+), 16 deletions(-) create mode 100644 TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/java/org/opensearch/migrations/transform/NoopTransformerProvider.java create mode 100644 TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF.services/org.opensearch.migrations.transform.IJsonTransformerProvider diff --git a/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJoltMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJoltScriptsTest.java b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJoltMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJoltScriptsTest.java index b83c6f892..7c125cec7 100644 --- a/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJoltMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJoltScriptsTest.java +++ b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJoltMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJoltScriptsTest.java @@ -22,7 +22,7 @@ private static Map parseAsMap(String contents) throws Exception { @Test public void testAddGzip() throws Exception { final var addGzip = - "[{\"org.opensearch.migrations.transform.JsonJoltTransformerProvider\": { \"canned\": \"ADD_GZIP\" }}]"; + "[{\"JsonJoltTransformerProvider\": { \"canned\": \"ADD_GZIP\" }}]"; var toNewHostTransformer = new TransformationLoader().getTransformerFactoryLoader("testhostname", addGzip); var origDocStr = SampleContents.loadSampleJsonRequestAsString(); @@ -37,8 +37,8 @@ public void testAddGzip() throws Exception { @Test public void testAddGzipAndCustom() throws Exception { final var addGzip = "[" + - "{\"org.opensearch.migrations.transform.JsonJoltTransformerProvider\": { \"canned\": \"ADD_GZIP\" }}," + - "{ \"org.opensearch.migrations.transform.JsonJoltTransformerProvider\":" + + "{\"JsonJoltTransformerProvider\": { \"canned\": \"ADD_GZIP\" }}," + + "{ \"JsonJoltTransformerProvider\":" + " {\"script\": \n" + " { \"operation\": \"modify-overwrite-beta\", \"spec\": " + " { \"headers\": {\"newHeader\": \"newValue\"}}}}}" + diff --git a/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/java/org/opensearch/migrations/transform/NoopTransformerProvider.java b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/java/org/opensearch/migrations/transform/NoopTransformerProvider.java new file mode 100644 index 000000000..a8ddfc399 --- /dev/null +++ b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/java/org/opensearch/migrations/transform/NoopTransformerProvider.java @@ -0,0 +1,17 @@ +package org.opensearch.migrations.transform; + +import java.util.Map; + +public class NoopTransformerProvider implements IJsonTransformerProvider { + private static class NoopTransformer implements IJsonTransformer { + @Override + public Map transformJson(Map incomingJson) { + return incomingJson; + } + } + + @Override + public IJsonTransformer createTransformer(Object jsonConfig) { + return new NoopTransformer(); + } +} diff --git a/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF.services/org.opensearch.migrations.transform.IJsonTransformerProvider b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF.services/org.opensearch.migrations.transform.IJsonTransformerProvider new file mode 100644 index 000000000..1439531ad --- /dev/null +++ b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF.services/org.opensearch.migrations.transform.IJsonTransformerProvider @@ -0,0 +1 @@ +org.opensearch.migrations.transform.NoopTransformerProvider \ No newline at end of file diff --git a/TrafficCapture/trafficReplayer/build.gradle b/TrafficCapture/trafficReplayer/build.gradle index 7af608c7c..4cf5c8a65 100644 --- a/TrafficCapture/trafficReplayer/build.gradle +++ b/TrafficCapture/trafficReplayer/build.gradle @@ -40,7 +40,9 @@ dependencies { implementation project(':captureProtobufs') implementation project(':coreUtilities') implementation project(':replayerPlugins:jsonMessageTransformers:jsonMessageTransformerInterface') - implementation project(':replayerPlugins:jsonMessageTransformers:openSearch23PlusTargetTransformerProvider') + runtimeOnly project(':replayerPlugins:jsonMessageTransformers:jsonJMESPathMessageTransformerProvider') + runtimeOnly project(':replayerPlugins:jsonMessageTransformers:jsonJoltMessageTransformerProvider') + runtimeOnly project(':replayerPlugins:jsonMessageTransformers:openSearch23PlusTargetTransformerProvider') implementation group: 'com.beust', name: 'jcommander', version: '1.82' implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.15.0' @@ -73,6 +75,9 @@ dependencies { testImplementation project(':captureOffloader') testImplementation testFixtures(project(path: ':captureOffloader')) testImplementation testFixtures(project(path: ':testUtilities')) + testImplementation project(':replayerPlugins:jsonMessageTransformers:jsonJMESPathMessageTransformerProvider') + testImplementation project(':replayerPlugins:jsonMessageTransformers:jsonJoltMessageTransformerProvider') + testImplementation project(':replayerPlugins:jsonMessageTransformers:openSearch23PlusTargetTransformerProvider') testImplementation group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.2.1' testImplementation group: 'org.junit.jupiter', name:'junit-jupiter-api', version:'5.x.x' diff --git a/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TransformationLoader.java b/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TransformationLoader.java index 3a7b83eb7..1c3d0b8eb 100644 --- a/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TransformationLoader.java +++ b/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TransformationLoader.java @@ -16,6 +16,7 @@ import java.util.Map; import java.util.Optional; import java.util.ServiceLoader; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -23,6 +24,7 @@ public class TransformationLoader { public static final String WRONG_JSON_STRUCTURE_MESSAGE = "Must specify the top-level configuration list with a sequence of " + "maps that have only one key each, where the key is the name of the transformer to be configured."; + public static final Pattern CLASS_NAME_PATTERN = Pattern.compile("^[a-zA-Z_$][a-zA-Z\\d_$]*$"); private final List providers; ObjectMapper objMapper = new ObjectMapper(); @@ -40,23 +42,22 @@ public TransformationLoader() { } List> parseFullConfig(String fullConfig) throws JsonProcessingException { - return objMapper.readValue(fullConfig, new TypeReference<>() {}); + if (CLASS_NAME_PATTERN.matcher(fullConfig).matches()) { + return List.of(Collections.singletonMap(fullConfig, null)); + } else { + return objMapper.readValue(fullConfig, new TypeReference<>() { + }); + } } protected Stream getTransformerFactoryFromServiceLoader(String fullConfig) throws JsonProcessingException { var configList = fullConfig == null ? List.of() : parseFullConfig(fullConfig); - if (providers.size() > 1 && configList.isEmpty()) { - throw new IllegalArgumentException("Must provide a configuration when multiple IJsonTransformerProvider " + - "are loaded (" + providers.stream().map(p -> p.getClass().toString()) - .collect(Collectors.joining(",")) + ")"); - } else if (providers.isEmpty()) { + if (configList.isEmpty() || providers.isEmpty()) { + log.warn("No transformer configuration specified. No custom transformations will be performed"); return Stream.of(); - } else if (!configList.isEmpty()) { + }else { return configList.stream().map(c -> configureTransformerFromConfig((Map) c)); - } else { - // send in Optional.empty because we would have hit the other case in the previous branch - return Stream.of(providers.get(0).createTransformer(Optional.empty())); } } @@ -69,7 +70,7 @@ private IJsonTransformer configureTransformerFromConfig(Map c) { var key = keys.stream().findFirst() .orElseThrow(()->new IllegalArgumentException(WRONG_JSON_STRUCTURE_MESSAGE)); for (var p : providers) { - var className = p.getClass().getName(); + var className = p.getClass().getSimpleName(); if (className.equals(key)) { var configuration = c.get(key); log.atInfo().setMessage(()->"Creating a transformer with configuration="+configuration).log(); diff --git a/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/HeaderTransformerTest.java b/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/HeaderTransformerTest.java index 4d58ad924..ae41e13d2 100644 --- a/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/HeaderTransformerTest.java +++ b/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/HeaderTransformerTest.java @@ -113,7 +113,7 @@ public void testMalformedPayload_andTypeMappingUri_IsPassedThrough() throws Exce var transformingHandler = new HttpJsonTransformingConsumer( new TransformationLoader().getTransformerFactoryLoader(SILLY_TARGET_CLUSTER_NAME, - "[{\"org.opensearch.migrations.transform.JsonTransformerForOpenSearch23PlusTargetTransformerProvider\":\"\"}]"), + "[{\"JsonTransformerForOpenSearch23PlusTargetTransformerProvider\":\"\"}]"), null, testPacketCapture, "TEST", TestRequestKey.getTestConnectionRequestId(0)); Random r = new Random(2); diff --git a/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java b/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java index 5f1e36c45..5ca711bca 100644 --- a/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java +++ b/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java @@ -28,4 +28,13 @@ public void testTransformationLoader() throws Exception { Assertions.assertNotEquals(origDocStr, docWithNewHostnameStr); } + @Test + public void testThatSimpleNoopTransformerLoads() throws Exception { + var noopTransformer = new TransformationLoader() + .getTransformerFactoryLoader("localhost", "NoopTransformerProvider"); + var origDoc = parseAsMap(SampleContents.loadSampleJsonRequestAsString()); + var output = noopTransformer.transformJson(origDoc); + Assertions.assertEquals(mapper.writeValueAsString(origDoc), mapper.writeValueAsString(output)); + + } } From f0dcc0070768cd84f415a3f55284b6e488bf3faf Mon Sep 17 00:00:00 2001 From: Greg Schohn Date: Mon, 13 Nov 2023 23:06:26 -0500 Subject: [PATCH 2/2] =?UTF-8?q?Wrap=20up=20transformation=20improvements?= =?UTF-8?q?=20as=20described=20in=20c66da0e28dd19ca321a7770b11d73ff961840b?= =?UTF-8?q?14=20(repeated)=20I've=20added=20all=20of=20the=20transformer?= =?UTF-8?q?=20jars=20to=20the=20runtime=20closure=20of=20the=20replayer=20?= =?UTF-8?q?since=20it's=20going=20to=20be=20a=20challenge=20for=20users=20?= =?UTF-8?q?using=20a=20cloud-deployment=20shrink-wrap=20solution=20to=20ad?= =?UTF-8?q?d=20addition=20jarfiles.=20Change=20the=20syntax/rules=20for=20?= =?UTF-8?q?Transformer=20configs.=20*=20If=20no=20argument=20was=20specifi?= =?UTF-8?q?ed=20for=20the=20transformer=20config,=20no=20transformer=20use?= =?UTF-8?q?d,=20regardless=20of=20what's=20in=20the=20classpath.=20*=20To?= =?UTF-8?q?=20use=20multiple=20transformers,=20you=E2=80=99ll=20need=20to?= =?UTF-8?q?=20specify=20the=20transformer-config=20as=20a=20json=20array?= =?UTF-8?q?=20(of=20json=20transformer=20configs)=20*=20To=20use=20one=20t?= =?UTF-8?q?ransformer=20with=20default=20settings,=20you=20can=20JUST=20pu?= =?UTF-8?q?t=20the=20name=20of=20the=20transformer=20in=20the=20arg.=20e.g?= =?UTF-8?q?.=20--transformer-config=20JsonTransformerForOpenSearch23PlusTa?= =?UTF-8?q?rgetTransformerProvider.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I've also updated the key name to only use the short, simple name of a class rather than the fully qualified one with the namespace. Add a NoopTransformer, which makes unit and sanity testing a lot simpler. Signed-off-by: Greg Schohn --- .../replay/MultipleJMESPathScriptsTest.java | 2 +- ....migrations.transform.IJsonTransformerProvider | 0 TrafficCapture/trafficReplayer/README.md | 10 +++++++--- .../migrations/replay/TrafficReplayer.java | 7 +++++-- .../replay/TransformationLoaderTest.java | 15 +++++++++++++++ 5 files changed, 28 insertions(+), 6 deletions(-) rename TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/{META-INF.services => META-INF/services}/org.opensearch.migrations.transform.IJsonTransformerProvider (100%) diff --git a/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJMESPathMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJMESPathScriptsTest.java b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJMESPathMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJMESPathScriptsTest.java index 1a7b521f4..bf242ece2 100644 --- a/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJMESPathMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJMESPathScriptsTest.java +++ b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonJMESPathMessageTransformerProvider/src/test/java/org/opensearch/migrations/replay/MultipleJMESPathScriptsTest.java @@ -32,7 +32,7 @@ public void testTwoScripts() throws Exception { var aggregateScriptJoiner = new StringJoiner(",\n", "[", "]"); for (var script : new String[]{EXCISE_SCRIPT, HOSTNAME_SCRIPT}) { aggregateScriptJoiner.add( - "{\"org.opensearch.migrations.transform.JsonJMESPathTransformerProvider\": {" + + "{\"JsonJMESPathTransformerProvider\": {" + " \"script\": \"" + script + "\"}}" ); } diff --git a/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF.services/org.opensearch.migrations.transform.IJsonTransformerProvider b/TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF/services/org.opensearch.migrations.transform.IJsonTransformerProvider similarity index 100% rename from TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF.services/org.opensearch.migrations.transform.IJsonTransformerProvider rename to TrafficCapture/replayerPlugins/jsonMessageTransformers/jsonMessageTransformerInterface/src/main/resources/META-INF/services/org.opensearch.migrations.transform.IJsonTransformerProvider diff --git a/TrafficCapture/trafficReplayer/README.md b/TrafficCapture/trafficReplayer/README.md index c26ad7d5a..21ff7bd13 100644 --- a/TrafficCapture/trafficReplayer/README.md +++ b/TrafficCapture/trafficReplayer/README.md @@ -127,13 +127,17 @@ transform to add GZIP encoding and another to apply a new header would be config ``` [ -{"org.opensearch.migrations.transform.JsonJMESPathTransformerProvider": { "script": +{"JsonJMESPathTransformerProvider": { "script": "{\"method\": method,\"URI\": URI,\"headers\":headers,\"payload\":{\"inlinedJsonBody\":{\"mappings\": payload.inlinedJsonBody.mappings.oldType}}}"}}, -{"org.opensearch.migrations.transform.JsonJoltTransformerProvider": { "canned": "ADD_GZIP" }}, -{ "org.opensearch.migrations.transform.JsonJoltTransformerProvider": {"script": +{"JsonJoltTransformerProvider": { "canned": "ADD_GZIP" }}, +{"JsonJoltTransformerProvider": {"script": { "operation": "modify-overwrite-beta", "spec": { "headers": {"newHeader": "newValue"}}}}}] ``` +To run only one transformer without any configuration, the `--transformer-config` argument can simply +be set to the classname of the transformer (e.g. 'JsonTransformerForOpenSearch23PlusTargetTransformerProvider', +without quotes or any json surrounding it). + Some simple transformations are included to change headers to add compression or to force an HTTP message payload to be chunked. Another transformer, [JsonTypeMappingTransformer.java](../replayerPlugins/jsonMessageTransformers/openSearch23PlusTargetTransformerProvider/src/main/java/org/opensearch/migrations/transform/JsonTypeMappingTransformer.java), is a work-in-progress to excise type mapping references from URIs and message payloads since versions of OpenSource diff --git a/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TrafficReplayer.java b/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TrafficReplayer.java index 0197900d5..649c8e8f9 100644 --- a/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TrafficReplayer.java +++ b/TrafficCapture/trafficReplayer/src/main/java/org/opensearch/migrations/replay/TrafficReplayer.java @@ -300,8 +300,11 @@ static class Parameters { @Parameter(required = false, names = "--transformer-config", arity = 1, - description = "Json configuration of message transformers. Keys are the names of the loaded transformers " + - "(shortname or longname) and values are the configuration passed to each of the transformers.") + description = "Configuration of message transformers. Either as a string that identifies the " + + "transformer that should be run (with default settings) or as json to specify options " + + "as well as multiple transformers to run in sequence. " + + "For json, keys are the (simple) names of the loaded transformers and values are the " + + "configuration passed to each of the transformers.") String transformerConfig; } diff --git a/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java b/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java index 5ca711bca..1e2b263d8 100644 --- a/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java +++ b/TrafficCapture/trafficReplayer/src/test/java/org/opensearch/migrations/replay/TransformationLoaderTest.java @@ -35,6 +35,21 @@ public void testThatSimpleNoopTransformerLoads() throws Exception { var origDoc = parseAsMap(SampleContents.loadSampleJsonRequestAsString()); var output = noopTransformer.transformJson(origDoc); Assertions.assertEquals(mapper.writeValueAsString(origDoc), mapper.writeValueAsString(output)); + } + @Test + public void testMisconfiguration() throws Exception { + Assertions.assertThrows(IllegalArgumentException.class, () -> new TransformationLoader() + .getTransformerFactoryLoader("localhost", "Not right")); } + + @Test + public void testThatNoConfigMeansNoThrow() throws Exception { + var transformer = Assertions.assertDoesNotThrow(()->new TransformationLoader() + .getTransformerFactoryLoader("localhost", null)); + Assertions.assertNotNull(transformer); + var origDoc = parseAsMap(SampleContents.loadSampleJsonRequestAsString()); + Assertions.assertNotNull(transformer.transformJson(origDoc)); + } + }