TST: trace event migration backward compatibility e2e tests #1264

chenqi0805 · 2022-04-04T17:15:17Z

Description

This PR

refactors existing trace e2e tests
adds additional tests to cover compatibility between event record type and classic record type (ExportTraceServiceRequest)

Issues Resolved

Resolves #1158

Check List

New functionality includes testing.
New functionality has been documented.
- New functionality has javadoc added
Commits are signed with a real name per the DCO

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.

Signed-off-by: Qi Chen <[email protected]>

codecov-commenter · 2022-04-04T17:39:41Z

Codecov Report

Merging #1264 (456fdeb) into main (5d3ff6d) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #1264   +/-   ##
=========================================
  Coverage     94.08%   94.08%           
  Complexity     1156     1156           
=========================================
  Files           162      162           
  Lines          3248     3248           
  Branches        263      263           
=========================================
  Hits           3056     3056           
  Misses          138      138           
  Partials         54       54

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d3ff6d...456fdeb. Read the comment docs.

dlvenable · 2022-04-05T19:19:48Z

.github/workflows/data-prepper-trace-analytics-raw-span-compatibility-e2e-tests.yml

@@ -24,5 +24,8 @@ jobs:
        java-version: ${{ matrix.java }}
    - name: Checkout Data-Prepper
      uses: actions/checkout@v2
-    - name: Run raw-span compatibility end-to-end tests with Gradle
-      run: ./gradlew :e2e-test:trace:rawSpanCompatibilityEndToEndTest


I propose that we keep the rawSpanCompatibilityEndToEndTest. It can just be a task which depends on the other two tests.

This approach allows for developers to have few test commands to run locally.

So the current rawSpanCompatibilityEndToEndTest is renamed as rawSpanOTLPLatestReleaseCompatibilityEndToEndTest. I only added a similar test with event type named rawSpanEventLatestReleaseCompatibilityEndToEndTest. Can you specify what two other tests this should depend on?

You have renamed rawSpanCompatibilityEndToEndTest to rawSpanOTLPLatestReleaseCompatibilityEndToEndTest. So keep that.

But, you can also add a task named rawSpanCompatibilityEndToEndTest. Just make it depend on rawSpanOTLPLatestReleaseCompatibilityEndToEndTest and rawSpanEventLatestReleaseCompatibilityEndToEndTest.

Tried the following:

task rawSpanEndToEndTest(type: Test) { dependsOn rawSpanOTLPEndToEndTest dependsOn rawSpanOTLPAndEventEndToEndTest rawSpanOTLPEndToEndTest.doLast { sleep(10 * 1000) } rawSpanOTLPAndEventEndToEndTest.mustRunAfter(rawSpanOTLPEndToEndTest) }

but unfortunately the container conflict between the two tasks remains:

Status 409: {"message":"Conflict. The container name \"/dataprepper2\" is already in use by container \"d3f18cb07a9ef6625aa830db8dbaf27f85029d5ae7a23967162bec7a3b2c6154\". You have to remove (or rename) that container to be able to reuse that name."}

OK - it appears it isn't quite so easy due to using Docker. Maybe we can improve this later.

dlvenable · 2022-04-05T19:22:39Z

e2e-test/trace/src/integrationTest/resources/raw-span-e2e-pipeline-event-type.yml

+      name: "entry-pipeline"
+  processor:
+    - otel_trace_raw:
+        root_span_flush_delay: 1 # TODO: remove after 1.1 release


Apparently this was never removed after the 1.1 release. Maybe we should remove it now.

Thanks for the catch!

dlvenable · 2022-04-05T19:23:23Z

...e/src/integrationTest/java/com/amazon/dataprepper/integration/trace/EndToEndRawSpanTest.java

@@ -1,15 +1,20 @@
 /*
- * Copyright OpenSearch Contributors
- * SPDX-License-Identifier: Apache-2.0
+ *  SPDX-License-Identifier: Apache-2.0


Please use the shorter license file.

Unless you copied this code from another project, the shorter license file is the correct one.

cmanning09 · 2022-04-05T19:20:03Z

e2e-test/trace/src/integrationTest/resources/raw-span-e2e-pipeline-event-type.yml

+      name: "entry-pipeline"
+  processor:
+    - otel_trace_raw:
+        root_span_flush_delay: 1 # TODO: remove after 1.1 release


This comment is out-dated. Can we delete it?

cmanning09 · 2022-04-05T19:23:35Z

...e/src/integrationTest/java/com/amazon/dataprepper/integration/trace/EndToEndRawSpanTest.java

        // Wait for data to flow through pipeline and be indexed by ES
-        await().atMost(10, TimeUnit.SECONDS).untilAsserted(
+        await().atLeast(6, TimeUnit.SECONDS).atMost(20, TimeUnit.SECONDS).untilAsserted(


Why do we need to increase the time out to 20 seconds?

This is just add some cushion for async assertion due to removal of L116:

Thread.sleep(6000);

cmanning09 · 2022-04-05T19:25:39Z

...e/src/integrationTest/java/com/amazon/dataprepper/integration/trace/EndToEndRawSpanTest.java

+ *  SPDX-License-Identifier: Apache-2.0
+ *
+ *  The OpenSearch Contributors require contributions made to
+ *  this file be licensed under the Apache-2.0 license or a
+ *  compatible open source license.
+ *
+ *  Modifications Copyright OpenSearch Contributors. See
+ *  GitHub history for details.


This license header is out of date. Please keep the original header:

/* * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */

cmanning09 · 2022-04-05T19:26:51Z

e2e-test/trace/build.gradle

+def createServiceMapDataPrepperOTLP2Task = createDataPrepperDockerContainer(
+        "serviceMapDataPrepper2", "dataprepper2", 21891, 4901, "/app/${SERVICE_MAP_PIPELINE_YAML}")
+
+// TODO: replace with Event type in 2.0


Let's provide a link to a Github issue and include a reference to this work in that issue. This way we can easily track the work required when we pick it up.

cmanning09 · 2022-04-05T19:28:26Z

e2e-test/trace/build.gradle

-    // wait for data-preppers to be ready
-    doFirst {
-        sleep(10*1000)
+def createEndToEndTest(final String testName, final String includeTestsMatchPattern,


Nice work removing the repetitive code blocks.

Signed-off-by: Qi Chen <[email protected]>

…lity-e2e-tests Signed-off-by: Qi Chen <[email protected]>

chenqi0805 added 2 commits April 4, 2022 09:58

MAINT: additional 3 e2e tests

9084dad

Signed-off-by: Qi Chen <[email protected]>

REF: e2e tests task definition and README

46869a7

Signed-off-by: Qi Chen <[email protected]>

chenqi0805 requested a review from a team as a code owner April 4, 2022 17:15

chenqi0805 added 3 commits April 4, 2022 12:23

MAINT: github workflow

e355d2a

Signed-off-by: Qi Chen <[email protected]>

STY: spotless

d10649d

Signed-off-by: Qi Chen <[email protected]>

FIX: window_duration

8220b61

Signed-off-by: Qi Chen <[email protected]>

chenqi0805 requested review from cmanning09 and dlvenable April 5, 2022 18:58

dlvenable requested changes Apr 5, 2022

View reviewed changes

cmanning09 reviewed Apr 5, 2022

View reviewed changes

CLN: PR comments

4564080

Signed-off-by: Qi Chen <[email protected]>

chenqi0805 requested review from cmanning09 and dlvenable April 5, 2022 22:30

Merge branch 'main' into tst/trace-event-migration-backward-compatibi…

456fdeb

…lity-e2e-tests Signed-off-by: Qi Chen <[email protected]>

dlvenable approved these changes Apr 6, 2022

View reviewed changes

cmanning09 approved these changes Apr 7, 2022

View reviewed changes

chenqi0805 merged commit de15772 into opensearch-project:main Apr 7, 2022

chenqi0805 deleted the tst/trace-event-migration-backward-compatibility-e2e-tests branch April 7, 2022 14:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: trace event migration backward compatibility e2e tests #1264

TST: trace event migration backward compatibility e2e tests #1264

chenqi0805 commented Apr 4, 2022

codecov-commenter commented Apr 4, 2022 •

edited

Loading

dlvenable Apr 5, 2022

chenqi0805 Apr 5, 2022

dlvenable Apr 6, 2022

chenqi0805 Apr 6, 2022

dlvenable Apr 6, 2022

dlvenable Apr 5, 2022

chenqi0805 Apr 5, 2022

dlvenable Apr 5, 2022

cmanning09 Apr 5, 2022

cmanning09 Apr 5, 2022

chenqi0805 Apr 5, 2022

cmanning09 Apr 5, 2022

cmanning09 Apr 5, 2022

cmanning09 Apr 5, 2022

TST: trace event migration backward compatibility e2e tests #1264

TST: trace event migration backward compatibility e2e tests #1264

Conversation

chenqi0805 commented Apr 4, 2022

Description

Issues Resolved

Check List

codecov-commenter commented Apr 4, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 4, 2022 •

edited

Loading