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

test(otel): re-enable integration tests #8984

Closed
wants to merge 1 commit into from
Closed

test(otel): re-enable integration tests #8984

wants to merge 1 commit into from

Conversation

aabmass
Copy link
Contributor

@aabmass aabmass commented Jan 26, 2024 •
edited
Loading

Description

Fixes #8979

The breakage was actually caused by upgrading logstash-encoder to 7.4 because it does not support logback versions prior to 1.3.0. Spring Boot 2 uses an older logback version.

Checklist

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Jan 26, 2024
@aabmass aabmass marked this pull request as ready for review January 26, 2024 05:02
@aabmass aabmass requested review from yoshi-approver and a team as code owners January 26, 2024 05:02
@minherz
Copy link
Contributor

minherz commented Jan 26, 2024

We would like to avoid pinning versions for multiple packages in the situations like this. AFAIK it should be possible to pack two different versions of the logback.
Additionally, what is necessity of using logstash-encoder? Is it possible to remove it and reduce the footprint of dependencies?

@aabmass
Copy link
Contributor Author

aabmass commented Jan 26, 2024

All of this pinning is ultimately to support java 11 because Spring Boot 3 requires java 17. Otherwise it would not be necessary

Additionally, what is necessity of using logstash-encoder?

It is used in logback.xml to support structured JSON logs

@minherz
Copy link
Contributor

minherz commented Jan 29, 2024

Is it possible that this PR and #8987 have overlapped changes?

@aabmass
Copy link
Contributor Author

aabmass commented Jan 29, 2024

Is it possible that this PR and #8987 have overlapped changes?

Yes #8987 is a draft still, I was using this as base branch to test the changes. I will unstack once this change is in

@aabmass
Copy link
Contributor Author

aabmass commented Jan 31, 2024

Closing in favor of moving this into opentelemetry-operations-java. I will send a PR to delete this sample once region tags are moved over to the new repo

@aabmass aabmass closed this Jan 31, 2024
aabmass added a commit to aabmass/opentelemetry-operations-java that referenced this pull request Jan 31, 2024
@aabmass
Use logstash-logback-encoder version that works with spring boot 2
52f1680 
This is a port over of
GoogleCloudPlatform/java-docs-samples#8984 to
this repo.
@aabmass aabmass mentioned this pull request Jan 31, 2024
Merged
aabmass added a commit to GoogleCloudPlatform/opentelemetry-operations-java that referenced this pull request Feb 1, 2024
@aabmass @psx95
Use logstash-logback-encoder version that works with spring boot 2 (#285
2b4f807 
)

This is a port over of
GoogleCloudPlatform/java-docs-samples#8984 to
this repo.

Co-authored-by: Pranav Sharma <sharmapranav@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yoshi-approver yoshi-approver Awaiting requested review from yoshi-approver yoshi-approver is a code owner

Assignees

@minherz minherz

Labels
samples Issues that are directly related to samples.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

opentelemetry/spring-boot-instrumentation test(s) are failing
2 participants
@aabmass @minherz