Skip to content

Make resource-detector compatible with Java 8#275

Merged
psx95 merged 1 commit intoGoogleCloudPlatform:mainfrom
psx95:detector-resources-java-8
Dec 4, 2023
Merged

Make resource-detector compatible with Java 8#275
psx95 merged 1 commit intoGoogleCloudPlatform:mainfrom
psx95:detector-resources-java-8

Conversation

@psx95
Copy link
Copy Markdown
Contributor

@psx95 psx95 commented Nov 30, 2023 •
edited
Loading

Description

This PR makes the resource detector Java 8 compatible. This is to support #266.

How the changes were tested

  • Used the GCP Detector jar built from this change in a Java 8 application (running in a docker container) to successfully detect resources. The Java 8 application was similar to resource-example in the current repo.
  • Used the javap tool to check the major version with which the classes was compiled -
 javap -v javap -v detectors/resources/build/classes/java/main/com/google/cloud/opentelemetry/detectors/*.class | grep major

Output -

  major version: 52
  major version: 52
  major version: 52
  major version: 52

Indicates all classes within resource detector are Java 8 compatible.

Other artifacts are still compiled & targeted for Java 11.

javap -v exporters/*/build/classes/java/main/com/google/cloud/opentelemetry/*/*.class | grep major

Output -

  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55
  major version: 55

@psx95 psx95 requested a review from jsuereth November 30, 2023 23:48
@psx95 psx95 marked this pull request as ready for review December 1, 2023 19:36
@psx95 psx95 requested a review from a team as a code owner December 1, 2023 19:36
@psx95 psx95 requested a review from dashpole December 1, 2023 19:36
@dashpole
Copy link
Copy Markdown
Contributor

dashpole commented Dec 2, 2023

Do we have any test/checks to ensure it continues to support java 8? Or is this enough, as long as we don't change this config?

@psx95
Copy link
Copy Markdown
Contributor Author

psx95 commented Dec 2, 2023 •
edited
Loading

Do we have any test/checks to ensure it continues to support java 8? Or is this enough, as long as we don't change this config?

No, not at the moment, I tested everything manually in a one-off fashion. We currently do not have any tests pertaining to specific Java version.

This was just the first step, checking to see if our build works properly. Once we refactor out the resource-detector-support library, we can add tests there.

However, based on the gradle documentation on release flag used in this config, I think it should give us better assurances than using the Java compatibility options.

Maybe @jsuereth can weigh in here

@psx95 psx95 merged commit fed23bf into GoogleCloudPlatform:main Dec 4, 2023
@psx95 psx95 deleted the detector-resources-java-8 branch December 4, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsuereth jsuereth jsuereth approved these changes

@dashpole dashpole dashpole approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@psx95 @dashpole @jsuereth