From c66da0e28dd19ca321a7770b11d73ff961840b14 Mon Sep 17 00:00:00 2001 From: Greg Schohn Date: Mon, 13 Nov 2023 18:46:59 -0500 Subject: [PATCH] =?UTF-8?q?Transformation=20improvements.=20I've=20added?= =?UTF-8?q?=20all=20of=20the=20transformer=20jars=20to=20the=20runtime=20c?= =?UTF-8?q?losure=20of=20the=20replayer=20since=20it's=20going=20to=20be?= =?UTF-8?q?=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)); + + } }