Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAINT: refactoring existing e2e tests on trace data ingestion #512

Merged
merged 9 commits into from
Nov 3, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ jobs:
- name: Grant execute permission for gradlew
run: chmod +x gradlew
- name: Run raw-span compatibility end-to-end tests with Gradle
run: ./gradlew :data-prepper-core:rawSpanCompatibilityEndToEndTest
run: ./gradlew :e2e:trace:rawSpanCompatibilityEndToEndTest
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ jobs:
- name: Grant execute permission for gradlew
run: chmod +x gradlew
- name: Run raw-span end-to-end tests with Gradle
run: ./gradlew :data-prepper-core:rawSpanEndToEndTest
run: ./gradlew :e2e:trace:rawSpanEndToEndTest
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ jobs:
- name: Grant execute permission for gradlew
run: chmod +x gradlew
- name: Run service-map end-to-end tests with Gradle
run: ./gradlew :data-prepper-core:serviceMapEndToEndTest
run: ./gradlew :e2e:trace:serviceMapEndToEndTest
2 changes: 0 additions & 2 deletions data-prepper-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ sourceSets {
}
}

apply from: "integrationTest.gradle"

dependencies {
implementation project(':data-prepper-api')
implementation project(':data-prepper-plugins')
Expand Down
3 changes: 3 additions & 0 deletions e2e/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Data Prepper End-to-end Tests

This module includes all e2e tests for data-prepper.
40 changes: 40 additions & 0 deletions e2e/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
subprojects {
buildscript {
repositories {
maven {
url "https://plugins.gradle.org/m2/"
}
}
dependencies {
classpath 'com.bmuschko:gradle-docker-plugin:7.0.0'
}
}

sourceSets {
integrationTest {
java {
compileClasspath += main.output + test.output
runtimeClasspath += main.output + test.output
srcDir file('src/integrationTest/java')
}
resources.srcDir file('src/integrationTest/resources')
}
}

configurations {
integrationTestImplementation.extendsFrom testImplementation
integrationTestRuntime.extendsFrom testRuntime
}

def DATA_PREPPER_CORE_JAR = project(':data-prepper-core').jar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try to avoid defs in Gradle builds. Is there a way to clean this up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is not bad practice since it appeared in docs: https://docs.gradle.org/current/userguide/more_about_tasks.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is bad practice, but I do try to avoid them when possible. But, if there isn't a better way, that is fine too.

Also, this isn't a constant, so it should probably also be camelCase: dataPrepperCoreJar.


task copyDataPrepperJar(type: Copy) {
dependsOn DATA_PREPPER_CORE_JAR
from("${project(':data-prepper-core').jar.archivePath}")
into 'build/bin'
}

ext {
data_prepper_jar_filepath = "build/bin/${DATA_PREPPER_CORE_JAR.archiveName}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use Groovy for Gradle build files and should follow the correct coding standards. This should be camelCase: dataPrepperJarFilepath.

Also, this value should be scoped correctly. I'm guessing this should be the root project. In which case it should look more like "${project.rootProject.buildDir}/bin/${DATA_PREPPER_CORE_JAR.archiveName}".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it should be the scope of qa-test project, I will replace it with project(':qa-test').buildDir

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of that, please use ${project.buildDir} which will use the current project.

}
}
36 changes: 36 additions & 0 deletions e2e/trace/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Trace Data Ingestion End-to-end Tests

This module includes e2e tests for trace data ingestion supported by data-prepper.

## Raw Span Ingestion Pipeline End-to-end test

Run from current directory
```
./gradlew :rawSpanEndToEndTest
```
or from project root directory
```
./gradlew :e2e:trace:rawSpanEndToEndTest
```

## Raw Span Ingestion Pipelines Compatibility End-to-end test

Run from current directory
```
./gradlew :rawSpanCompatibilityEndToEndTest
```
or from project root directory
```
./gradlew :e2e:trace:rawSpanCompatibilityEndToEndTest
```

## Service Map Ingestion Pipelines End-to-end test

Run from current directory
```
./gradlew :serviceMapEndToEndTest
```
or from project root directory
```
./gradlew :e2e:trace:serviceMapEndToEndTest
```
74 changes: 19 additions & 55 deletions data-prepper-core/integrationTest.gradle → e2e/trace/build.gradle
Original file line number Diff line number Diff line change
@@ -1,59 +1,11 @@
buildscript {
repositories {
maven {
url "https://plugins.gradle.org/m2/"
}
}
dependencies {
classpath 'com.bmuschko:gradle-docker-plugin:7.0.0'
}
}
apply plugin: DockerRemoteApiPlugin

apply plugin: com.bmuschko.gradle.docker.DockerRemoteApiPlugin

/*
* 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.
*/

import com.bmuschko.gradle.docker.DockerRemoteApiPlugin
import com.bmuschko.gradle.docker.tasks.container.*
import com.bmuschko.gradle.docker.tasks.image.*
import com.bmuschko.gradle.docker.tasks.network.*

sourceSets {
integrationTest {
java {
compileClasspath += main.output + test.output
runtimeClasspath += main.output + test.output
srcDir file('src/integrationTest/java')
}
resources.srcDir file('src/integrationTest/resources')
}
}

configurations {
integrationTestImplementation.extendsFrom testImplementation
integrationTestRuntime.extendsFrom testRuntime
}

dependencies {
integrationTestCompile project(':data-prepper-plugins:opensearch')
integrationTestCompile project(':data-prepper-plugins:otel-trace-group-prepper')
integrationTestImplementation "org.awaitility:awaitility:4.0.3"
integrationTestImplementation "io.opentelemetry:opentelemetry-proto:${versionMap.opentelemetryProto}"
integrationTestImplementation 'com.google.protobuf:protobuf-java-util:3.13.0'
integrationTestImplementation "com.linecorp.armeria:armeria:1.0.0"
integrationTestImplementation "com.linecorp.armeria:armeria-grpc:1.0.0"
integrationTestImplementation "org.opensearch.client:opensearch-rest-high-level-client:${versionMap.opensearchVersion}"
integrationTestImplementation 'com.fasterxml.jackson.core:jackson-databind'
}

/**
* End-to-end test docker network
*/
Expand All @@ -74,13 +26,13 @@ def SERVICE_MAP_PIPELINE_YAML = "service-map-e2e-pipeline.yml"
* DataPrepper Docker tasks
*/
task createDataPrepperDockerFile(type: Dockerfile) {
dependsOn jar
dependsOn copyDataPrepperJar
destFile = project.file('build/docker/Dockerfile')
from("adoptopenjdk/openjdk14:jre-14.0.1_7-alpine")
exposePort(21890)
exposePort(4900)
workingDir("/app")
copyFile("build/libs/${jar.archiveName}", "/app/data-prepper.jar")
copyFile("${data_prepper_jar_filepath}", "/app/data-prepper.jar")
copyFile("src/integrationTest/resources/${RAW_SPAN_PIPELINE_YAML}", "/app/${RAW_SPAN_PIPELINE_YAML}")
copyFile("src/integrationTest/resources/${RAW_SPAN_PIPELINE_LATEST_RELEASE_YAML}", "/app/${RAW_SPAN_PIPELINE_LATEST_RELEASE_YAML}")
copyFile("src/integrationTest/resources/${SERVICE_MAP_PIPELINE_YAML}", "/app/${SERVICE_MAP_PIPELINE_YAML}")
Expand Down Expand Up @@ -209,7 +161,7 @@ task rawSpanEndToEndTest(type: Test) {
classpath = sourceSets.integrationTest.runtimeClasspath

filter {
includeTestsMatching "com.amazon.dataprepper.integration.EndToEndRawSpanTest.testPipelineEndToEnd*"
includeTestsMatching "com.amazon.dataprepper.integration.trace.EndToEndRawSpanTest.testPipelineEndToEnd*"
}

finalizedBy stopOpenSearchDockerContainer
Expand Down Expand Up @@ -246,7 +198,7 @@ task rawSpanCompatibilityEndToEndTest(type: Test) {
classpath = sourceSets.integrationTest.runtimeClasspath

filter {
includeTestsMatching "com.amazon.dataprepper.integration.EndToEndRawSpanTest.testPipelineEndToEnd*"
includeTestsMatching "com.amazon.dataprepper.integration.trace.EndToEndRawSpanTest.testPipelineEndToEnd*"
}

finalizedBy stopOpenSearchDockerContainer
Expand Down Expand Up @@ -283,7 +235,7 @@ task serviceMapEndToEndTest(type: Test) {
classpath = sourceSets.integrationTest.runtimeClasspath

filter {
includeTestsMatching "com.amazon.dataprepper.integration.EndToEndServiceMapTest*"
includeTestsMatching "com.amazon.dataprepper.integration.trace.EndToEndServiceMapTest*"
}

finalizedBy stopOpenSearchDockerContainer
Expand All @@ -295,3 +247,15 @@ task serviceMapEndToEndTest(type: Test) {
finalizedBy removeDataPrepperDockerContainer(stopDataPrepper2Task as DockerStopContainer)
finalizedBy removeDataPrepperNetwork
}

dependencies {
integrationTestCompile project(':data-prepper-plugins:opensearch')
integrationTestCompile project(':data-prepper-plugins:otel-trace-group-prepper')
integrationTestImplementation "org.awaitility:awaitility:4.0.3"
integrationTestImplementation "io.opentelemetry:opentelemetry-proto:${versionMap.opentelemetryProto}"
integrationTestImplementation 'com.google.protobuf:protobuf-java-util:3.13.0'
integrationTestImplementation "com.linecorp.armeria:armeria:1.0.0"
integrationTestImplementation "com.linecorp.armeria:armeria-grpc:1.0.0"
integrationTestImplementation "org.opensearch.client:opensearch-rest-high-level-client:${versionMap.opensearchVersion}"
integrationTestImplementation 'com.fasterxml.jackson.core:jackson-databind'
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* GitHub history for details.
*/

package com.amazon.dataprepper.integration;
package com.amazon.dataprepper.integration.trace;


import com.amazon.dataprepper.plugins.prepper.oteltracegroup.model.TraceGroup;
Expand All @@ -30,15 +30,15 @@
import io.opentelemetry.proto.trace.v1.ResourceSpans;
import io.opentelemetry.proto.trace.v1.Span;
import io.opentelemetry.proto.trace.v1.Status;
import org.junit.Assert;
import org.junit.Test;
import org.opensearch.action.admin.indices.refresh.RefreshRequest;
import org.opensearch.action.search.SearchRequest;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.client.RequestOptions;
import org.opensearch.client.RestHighLevelClient;
import org.opensearch.search.SearchHits;
import org.opensearch.search.builder.SearchSourceBuilder;
import org.junit.Assert;
import org.junit.Test;

import java.io.IOException;
import java.time.Instant;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,11 @@
* GitHub history for details.
*/

package com.amazon.dataprepper.integration;
package com.amazon.dataprepper.integration.trace;


import com.google.protobuf.ByteString;
import com.amazon.dataprepper.plugins.sink.opensearch.ConnectionConfiguration;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import com.google.protobuf.ByteString;
import com.linecorp.armeria.client.Clients;
import com.linecorp.armeria.client.retry.RetryRule;
import com.linecorp.armeria.client.retry.RetryingClient;
Expand All @@ -37,15 +25,27 @@
import io.opentelemetry.proto.trace.v1.InstrumentationLibrarySpans;
import io.opentelemetry.proto.trace.v1.ResourceSpans;
import io.opentelemetry.proto.trace.v1.Span;
import org.junit.Assert;
import org.junit.Test;
import org.opensearch.action.admin.indices.refresh.RefreshRequest;
import org.opensearch.action.search.SearchRequest;
import org.opensearch.action.search.SearchResponse;
import org.opensearch.client.RequestOptions;
import org.opensearch.client.RestHighLevelClient;
import org.opensearch.search.SearchHits;
import org.opensearch.search.builder.SearchSourceBuilder;
import org.junit.Assert;
import org.junit.Test;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.awaitility.Awaitility.await;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.amazon.dataprepper.integration;
package com.amazon.dataprepper.integration.trace;

import io.opentelemetry.proto.trace.v1.Span;

Expand Down
2 changes: 2 additions & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ include 'data-prepper-plugins:blocking-buffer'
include 'data-prepper-plugins:http-source'
include 'data-prepper-plugins:grok-prepper'
include 'data-prepper-logstash-configuration'
include 'e2e'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to raise the question of what name to use here. This uses e2e, but there may be some other options: e2e-test, qa, qa-test, test.

I don't like the name test since it is ambiguous with the other test tasks. I do think I like the idea of a clearer name like e2e-test or qa-test since e2e may not be an immediately obvious convention.

Also, the core OpenSearch project uses qa for its end-to-end tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dlvenable I also went back and forth between qa and e2e. I think qa-test looks good to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmanning09 @graytaylor0 , Any thoughts on the name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually e2e-test might be better since e2e is already populated in the repo 😄

include 'e2e:trace'