Skip to content

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

Closed
aabmass wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
aabmass:otel-fix-ci
Closed

test(otel): re-enable integration tests#8984
aabmass wants to merge 1 commit into
GoogleCloudPlatform:mainfrom
aabmass:otel-fix-ci

Conversation

@aabmass

@aabmass aabmass commented Jan 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 a team and yoshi-approver as code owners January 26, 2024 05:02
@minherz

minherz commented Jan 26, 2024

Copy link
Copy Markdown
Contributor

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

aabmass commented Jan 26, 2024

Copy link
Copy Markdown
Contributor Author

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

minherz commented Jan 29, 2024

Copy link
Copy Markdown
Contributor

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

@aabmass

aabmass commented Jan 29, 2024

Copy link
Copy Markdown
Contributor Author

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

aabmass commented Jan 31, 2024

Copy link
Copy Markdown
Contributor Author

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