Cleanup for release v1.19.0 #242
Conversation
Codecov Report
@@ Coverage Diff @@
## main #242 +/- ##
=========================================
Coverage 85.71% 85.71%
Complexity 19 19
=========================================
Files 3 3
Lines 49 49
Branches 5 5
=========================================
Hits 42 42
Misses 3 3
Partials 4 4 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
rapphil
force-pushed
the
rapphil-fix-jackson-dependency
branch
from
2375d2c to
2bbf676
Compare
rapphil
changed the title
| } | ||
|
|
||
| tasks { | ||
| named("jib") { | ||
| dependsOn(":otelagent:jib") | ||
| } | ||
| named("jibDockerBuild") { | ||
| dependsOn(":otelagent:jibDockerBuild") |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| @@ -60,7 +60,7 @@ val DEPENDENCY_SETS = listOf( | |||
| ), | |||
| DependencySet( | |||
| "org.slf4j", | |||
| "2.0.3", | |||
| "1.7.36", | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| 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"), |
There was a problem hiding this comment.
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"),
There was a problem hiding this comment.
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
| - 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' }} |
There was a problem hiding this comment.
Why are macos and windows tests executed with different gradle inputs than an ubuntu runner?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Description of changes:
aws-otel-testintegrationTeststask that is able to run locally exclusively so that the pr-build gh action can be idempotent. (previously it was depending on artifacts generated during the main-build gh action). This required adding some specific logic in thejibplugin because local docker daemon does not handle multiplatform images.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.