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.

Sign up for GitHub

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

Cleanup for release v1.19.0 #242

Merged
merged 5 commits into from
Oct 20, 2022
Merged

Cleanup for release v1.19.0 #242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2bbf676
Update jackson to latest stable version
rapphil Oct 18, 2022
f817552
Fix approval steps
rapphil Oct 19, 2022
e03aab1
Add missing build script
rapphil Oct 19, 2022
5668985
Add missing inter project dependencies
rapphil Oct 19, 2022
7d532d2
Fix approval to only run integrationt tests in ubuntu
rapphil Oct 19, 2022
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
6 changes: 6 additions & 0 deletions .github/workflows/pr-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@ jobs:
with:
java-version: 17
- uses: gradle/wrapper-validation-action@v1
- name: Build with Gradle with Integration tests
if: ${{ matrix.os == 'ubuntu-latest' }}
uses: gradle/gradle-build-action@v2
with:
arguments: build integrationTests --stacktrace -PenableCoverage=true -PlocalDocker=true
- name: Build with Gradle
uses: gradle/gradle-build-action@v2
if: ${{ matrix.os != 'ubuntu-latest' }}
Comment on lines +23 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are macos and windows tests executed with different gradle inputs than an ubuntu runner?

Copy link
Member Author

Choose a reason for hiding this comment

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

because they don't support docker.

We were skipping the tests for macos and windows silently because of this:
https://github.com/aws-observability/aws-otel-java-instrumentation/blob/main/smoke-tests/runner/src/test/java/io/awsobservability/instrumentation/smoketests/runner/LogInjectionTest.java#L32

@Testcontainers(disabledWithoutDocker = true)
class LogInjectionTest {

Se we are not running smoke tests in windows and macos anyways. Just building and running some unit tets.

Copy link
Contributor

Choose a reason for hiding this comment

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

Noted, there are ways we could run docker on MacOS runners. See here. This could possibly be a follow on item if we feel the need to expand test coverage to other operating systems.

with:
arguments: build --stacktrace -PenableCoverage=true
- uses: codecov/codecov-action@v1
12 changes: 12 additions & 0 deletions buildSrc/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
plugins {
`kotlin-dsl`
}
repositories {
mavenCentral()
gradlePluginPortal()
mavenLocal()
}

dependencies {
implementation("com.google.cloud.tools:jib-gradle-plugin:3.3.0")
}
37 changes: 37 additions & 0 deletions buildSrc/src/main/kotlin/software/amazon/adot/GradleUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package software.amazon.adot

import com.google.cloud.tools.jib.gradle.JibExtension

/**
* Utilitary extension function used to configure jib according to flags. Used to avoid having branching in each build
* script.
*/
fun JibExtension.configureImages(sourceImage:String, destinationImage: String, localDocker: Boolean, multiPlatform: Boolean, tags: Set<String> = setOf<String>()) {
to {
image = destinationImage
if (!tags.isEmpty()) {
this.tags = tags;
}
}

from {
if (localDocker) {
image = "docker://$sourceImage"
} else {
image = sourceImage
}

if (multiPlatform) {
platforms {
platform {
architecture = "amd64"
os = "linux"
}
platform {
architecture = "arm64"
os = "linux"
}
}
}
}
}
4 changes: 2 additions & 2 deletions dependencyManagement/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ val otelSnapshotVersion = "1.20.0"

val DEPENDENCY_BOMS = listOf(
"com.amazonaws:aws-java-sdk-bom:1.12.322",
"com.fasterxml.jackson:jackson-bom:2.14.0-rc2",
"com.fasterxml.jackson:jackson-bom:2.13.4.20221013",
"com.google.guava:guava-bom:31.1-jre",
"com.google.protobuf:protobuf-bom:3.21.7",
"com.linecorp.armeria:armeria-bom:1.20.1",
Expand All @@ -60,7 +60,7 @@ val DEPENDENCY_SETS = listOf(
),
DependencySet(
"org.slf4j",
"2.0.3",
"1.7.36",
Copy link
Contributor

@vasireddy99 vasireddy99 Oct 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The issue with using the latest SLF4J API Module. just wondering if it might have to do with this : https://www.slf4j.org/faq.html#changesInVersion200

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not clear what is the issue a this time, but the entry that you pointed in the faq is related to changes that were introduced in version 2.0 that are new features. I was trusting this statement:

SLF4J 2.0.0 incorporates an optional fluent api. Otherwise, there are no client facing API changes in 2.0.x. For most users, upgrading to version 2.0..x should be a drop-in replacement, as long as the logging provider is updated as well.

listOf(
"slf4j-api",
"slf4j-simple"
Expand Down
3 changes: 3 additions & 0 deletions gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ org.gradle.jvmargs= \
--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED

# Flag used in the jib plugin to inform if we should use multi platform and pull from local docker.
localDocker=false
25 changes: 8 additions & 17 deletions otelagent/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/

import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar

import software.amazon.adot.configureImages
plugins {
java
`maven-publish`
Expand Down Expand Up @@ -131,22 +131,13 @@ tasks {
}

jib {
to {
image = "public.ecr.aws/u0d6r4y4/aws-opentelemetry-java-base:alpha"
}
from {
image = "gcr.io/distroless/java17-debian11:debug"
platforms {
platform {
architecture = "amd64"
os = "linux"
}
platform {
architecture = "arm64"
os = "linux"
}
}
}
configureImages(
"gcr.io/distroless/java17-debian11:debug",
"public.ecr.aws/aws-otel-test/aws-opentelemetry-java-base:alpha",
localDocker = false,
multiPlatform = !rootProject.property("localDocker")!!.equals("true")
)

container {
appRoot = "/aws-observability"
setEntrypoint("INHERIT")
Expand Down
27 changes: 11 additions & 16 deletions sample-apps/spark-awssdkv1/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import software.amazon.adot.configureImages

plugins {
java

Expand All @@ -19,27 +21,20 @@ application {
}

jib {
to {
image = "public.ecr.aws/aws-otel-test/aws-otel-java-spark-awssdkv1"
configureImages(
"public.ecr.aws/aws-otel-test/aws-opentelemetry-java-base:alpha",
"public.ecr.aws/aws-otel-test/aws-otel-java-spark-awssdkv1",
localDocker = rootProject.property("localDocker")!!.equals("true"),
multiPlatform = !rootProject.property("localDocker")!!.equals("true"),
tags = setOf("latest", "${System.getenv("COMMIT_HASH")}")
}
from {
image = "public.ecr.aws/aws-otel-test/aws-opentelemetry-java-base:alpha"
platforms {
platform {
architecture = "amd64"
os = "linux"
}
platform {
architecture = "arm64"
os = "linux"
}
}
}
)
}

tasks {
named("jib") {
dependsOn(":otelagent:jib")
}
named("jibDockerBuild") {
dependsOn(":otelagent:jibDockerBuild")
Copy link
Contributor

Choose a reason for hiding this comment

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

as we are aware about the limitation for jibDockerBuild to use for the multi arch images, it is now able to use it docker daemon with this changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, docker daemon is still not able to support multi arch. This change is just forcing us to use jibDockerBuild in integration tests so that we don't need to rely on the artifacts that are published in the main-build Github CI.

}
}
27 changes: 11 additions & 16 deletions sample-apps/spark/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import software.amazon.adot.configureImages

plugins {
java

Expand All @@ -22,27 +24,20 @@ application {
}

jib {
to {
image = "public.ecr.aws/aws-otel-test/aws-otel-java-spark"
configureImages(
"public.ecr.aws/aws-otel-test/aws-opentelemetry-java-base:alpha",
"public.ecr.aws/aws-otel-test/aws-otel-java-spark",
localDocker = rootProject.property("localDocker")!!.equals("true"),
multiPlatform = !rootProject.property("localDocker")!!.equals("true"),
tags = setOf("latest", "${System.getenv("COMMIT_HASH")}")
}
from {
image = "public.ecr.aws/aws-otel-test/aws-opentelemetry-java-base:alpha"
platforms {
platform {
architecture = "amd64"
os = "linux"
}
platform {
architecture = "arm64"
os = "linux"
}
}
}
)
}

tasks {
named("jib") {
dependsOn(":otelagent:jib")
}
named("jibDockerBuild") {
dependsOn(":otelagent:jibDockerBuild")
}
}
28 changes: 12 additions & 16 deletions sample-apps/springboot/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import software.amazon.adot.configureImages

plugins {
java
id("org.springframework.boot")
Expand All @@ -14,27 +16,21 @@ dependencies {
}

jib {
to {
image = "public.ecr.aws/aws-otel-test/aws-otel-java-springboot"
configureImages(
"public.ecr.aws/aws-otel-test/aws-opentelemetry-java-base:alpha",
"public.ecr.aws/aws-otel-test/aws-otel-java-springboot",
rootProject.property("localDocker")!!.equals("true"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Excuse my gradle ignorance but is there a reason that these don't match the changes in other gradle build files by explicitly stating the parameter they are targetting?

   localDocker = rootProject.property("localDocker")!!.equals("true"),
    multiPlatform = !rootProject.property("localDocker")!!.equals("true"),

Copy link
Member Author

Choose a reason for hiding this comment

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

localDocker and multiPlatform applies to the source image. However there are cases where the we want to use a remote image but no multiPlatform. E.g.: for generating this image: aws-opentelemetry-java-base:alpha

!rootProject.property("localDocker")!!.equals("true"),
tags = setOf("latest", "${System.getenv("COMMIT_HASH")}")
}
from {
image = "public.ecr.aws/aws-otel-test/aws-opentelemetry-java-base:alpha"
platforms {
platform {
architecture = "amd64"
os = "linux"
}
platform {
architecture = "arm64"
os = "linux"
}
}
}
)
}

tasks {
named("jib") {
dependsOn(":otelagent:jib")
}

named("jibDockerBuild") {
dependsOn(":otelagent:jibDockerBuild")
}
}
17 changes: 17 additions & 0 deletions smoke-tests/runner/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,21 @@ tasks {
.getAsFile().absolutePath}"
)
}

register("jibDockerBuildAll")
register("integrationTests")

named("integrationTests") {
dependsOn("test")
dependsOn("jibDockerBuildAll")
findByName("test")?.mustRunAfter("jibDockerBuildAll")
}

named("jibDockerBuildAll") {
// Make sure that images used during tests are available locally.
dependsOn(":sample-apps:spark:jibDockerBuild")
dependsOn(":sample-apps:springboot:jibDockerBuild")
dependsOn(":smoke-tests:spring-boot:jibDockerBuild")
dependsOn(":smoke-tests:fakebackend:jibDockerBuild")
}
}
8 changes: 5 additions & 3 deletions .../test/java/io/awsobservability/instrumentation/smoketests/runner/SpringBootSmokeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public ExportTraceServiceRequest deserialize(

@Container
private static final GenericContainer<?> backend =
new GenericContainer<>("public.ecr.aws/u0d6r4y4/aws-otel-java-test-fakebackend:alpha")
new GenericContainer<>("public.ecr.aws/aws-otel-test/aws-otel-java-test-fakebackend:alpha")
.withExposedPorts(8080)
.waitingFor(Wait.forHttp("/health").forPort(8080))
.withLogConsumer(new Slf4jLogConsumer(backendLogger))
Expand All @@ -107,7 +107,8 @@ public ExportTraceServiceRequest deserialize(

@Container
private static final GenericContainer<?> application =
new GenericContainer<>("public.ecr.aws/u0d6r4y4/aws-otel-java-smoketests-springboot:latest")
new GenericContainer<>(
"public.ecr.aws/aws-otel-test/aws-otel-java-smoketests-springboot:latest")
.dependsOn(backend)
.withExposedPorts(8080)
.withNetwork(network)
Expand All @@ -122,7 +123,8 @@ public ExportTraceServiceRequest deserialize(

@Container
private static final GenericContainer<?> applicationXraySampler =
new GenericContainer<>("public.ecr.aws/u0d6r4y4/aws-otel-java-smoketests-springboot:latest")
new GenericContainer<>(
"public.ecr.aws/aws-otel-test/aws-otel-java-smoketests-springboot:latest")
.dependsOn(backend)
.withExposedPorts(8080)
.withNetwork(network)
Expand Down
32 changes: 26 additions & 6 deletions smoke-tests/spring-boot/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
/*
* Copyright Amazon.com, Inc. or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

import software.amazon.adot.configureImages

/*
* Copyright Amazon.com, Inc. or its affiliates.
*
Expand Down Expand Up @@ -29,16 +46,19 @@ dependencies {
}

jib {
to {
image = "public.ecr.aws/u0d6r4y4/aws-otel-java-smoketests-springboot"
}
from {
image = "public.ecr.aws/u0d6r4y4/aws-opentelemetry-java-base:alpha"
}
configureImages(
"public.ecr.aws/aws-otel-test/aws-opentelemetry-java-base:alpha",
"public.ecr.aws/aws-otel-test/aws-otel-java-smoketests-springboot",
localDocker = rootProject.property("localDocker")!!.equals("true"),
multiPlatform = false
)
}

tasks {
named("jib") {
dependsOn(":otelagent:jib")
}
named("jibDockerBuild") {
dependsOn(":otelagent:jibDockerBuild")
}
}