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

Jump to bottom

add kafka Spring starter smoke test #11262

Merged
merged 13 commits into from
May 14, 2024
Merged

add kafka Spring starter smoke test #11262

merged 13 commits into from
May 14, 2024

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Apr 30, 2024 •
edited
Loading

Fixes #11261

@github-actions github-actions bot added the test native This label can be applied to PRs to trigger them to run native tests label Apr 30, 2024
@zeitlinger zeitlinger self-assigned this Apr 30, 2024
@zeitlinger zeitlinger mentioned this pull request May 2, 2024
Merged
@zeitlinger zeitlinger marked this pull request as ready for review May 2, 2024 16:35
@zeitlinger zeitlinger requested a review from a team May 2, 2024 16:35
@zeitlinger zeitlinger closed this May 2, 2024
@zeitlinger zeitlinger reopened this May 2, 2024
@zeitlinger zeitlinger force-pushed the spring-kafka-test branch 3 times, most recently from b6a05d3 to 0d3dbc7 Compare May 6, 2024 11:15
@zeitlinger zeitlinger force-pushed the spring-kafka-test branch from 57fd731 to cbd57b0 Compare May 10, 2024 10:48
@zeitlinger zeitlinger force-pushed the spring-kafka-test branch from 2819d22 to c6cf76a Compare May 10, 2024 13:22
@trask trask added this to the v2.4.0 milestone May 10, 2024
laurit
laurit reviewed May 13, 2024
View reviewed changes
Copy link
Contributor

@laurit laurit left a comment

Choose a reason for hiding this comment

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

Previously we tested with the earliest supported version + latest. Now we test with latest 2.x and latest 3.x. Are you sure you wish to remove testing from the earliest supported version?

@zeitlinger
Copy link
Member Author

Previously we tested with the earliest supported version + latest. Now we test with latest 2.x and latest 3.x. Are you sure you wish to remove testing from the earliest supported version?

I don't get it

  • previously we tested against
    • 2.7.18 in autoconfigure (latest 2.x)
    • 3.2.5 in smoke test (latest 3.x)
  • now we're testing against the same versions - just in smoke test

Regardless - I'd like to tackle this comment in an upcoming PR 😄

@zeitlinger @jeanbisutti
Update smoke-tests-otel-starter/spring-smoke-testing/src/main/java/io…
7c0b50e 
…/opentelemetry/spring/smoketest/RequiresDockerCompose.java

Co-authored-by: Jean Bisutti <[email protected]>
@trask
Copy link
Member

trask commented May 14, 2024

Previously we tested with the earliest supported version + latest. Now we test with latest 2.x and latest 3.x. Are you sure you wish to remove testing from the earliest supported version?

we have the capability to test with earliest supported boot version, by using library dependencies, but looks like we aren't taking advantage of that capability and are just pinning the spring boot library dependencies to the latest 2.x and 3.x versions

opentelemetry-java-instrumentation/instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts

Lines 47 to 52 in 68cf431

library("org.springframework.kafka:spring-kafka:2.9.0")
library("org.springframework.boot:spring-boot-starter-actuator:$springBootVersion")
library("org.springframework.boot:spring-boot-starter-aop:$springBootVersion")
library("org.springframework.boot:spring-boot-starter-web:$springBootVersion")
library("org.springframework.boot:spring-boot-starter-webflux:$springBootVersion")
library("org.springframework.boot:spring-boot-starter-data-r2dbc:$springBootVersion")

and

opentelemetry-java-instrumentation/instrumentation/spring/spring-boot-autoconfigure-3/build.gradle.kts

Lines 14 to 15 in 68cf431

val springBootVersion = "3.2.4"
library("org.springframework.boot:spring-boot-starter-web:$springBootVersion")

it's a good question though, as typically we test all the instrumentation against an "earliest supported version" (and document that as the earliest supported version).

we should decide if/how we want to test/support earlier Spring Boot minor versions, @zeitlinger can you open an issue? I'll go ahead and merge this PR

@trask trask merged commit d18ccc5 into open-telemetry:main May 14, 2024
53 checks passed
@zeitlinger zeitlinger deleted the spring-kafka-test branch May 15, 2024 11:40
@zeitlinger zeitlinger mentioned this pull request May 15, 2024
Closed
@zeitlinger
Copy link
Member Author

we should decide if/how we want to test/support earlier Spring Boot minor versions, @zeitlinger can you open an issue? I'll go ahead and merge this PR

#11357

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurit laurit laurit left review comments

@jeanbisutti jeanbisutti jeanbisutti approved these changes

@trask trask trask approved these changes

Assignees

@zeitlinger zeitlinger

Labels
test native This label can be applied to PRs to trigger them to run native tests
Projects
None yet
Milestone
v2.4.0
Development

Successfully merging this pull request may close these issues.

move kafka spring starter test to spring smoke test project
4 participants
@zeitlinger @trask @laurit @jeanbisutti