open-telemetry / opentelemetry-java-contrib

https://opentelemetry.io
Apache License 2.0
144 stars 118 forks source link

Update dependency org.openjdk.jmc:flightrecorder to v9 - abandoned #1287

Closed renovate[bot] closed 1 month ago

renovate[bot] commented 2 months ago

Mend Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
org.openjdk.jmc:flightrecorder (source) 8.3.1 -> 9.0.0 age adoption passing confidence

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.



This PR has been generated by Mend Renovate. View repository job log here.

renovate[bot] commented 2 months ago

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠️ Warning: custom changes will be lost.

breedx-splk commented 2 months ago

@jeanbisutti or @open-telemetry/java-contrib-approvers We need to disable the tests on anything < 17 now, due to jmc being 17+. Please have a look. This also impacts #1286, which we can rebase after this gets merged.

trask commented 2 months ago

I think Java 8+ support would be nice to keep for this module since we may want to use it as part of profiling pipeline in the future

breedx-splk commented 2 months ago

I think Java 8+ support would be nice to keep for this module since we may want to use it as part of profiling pipeline in the future

I agree, but the tests no longer pass with java 8 or 11 after this dep upgrade.

Do you know of a way then to read JFR files with java 8? We could rework the tests, but I'm not sure that the classes to read are there, are they? All of the stuff in jdk.jfr.* is since 9 or later.

Heck, I can't even run the tests locally using java 8, hrmph.

$ java -version
openjdk version "1.8.0_412"
OpenJDK Runtime Environment (Zulu 8.78.0.19-CA-macos-aarch64) (build 1.8.0_412-b08)
OpenJDK 64-Bit Server VM (Zulu 8.78.0.19-CA-macos-aarch64) (build 25.412-b08, mixed mode)
[jplumb@splunk/Users/jplumb/code/opentelemetry-java-contrib] [05/07 10:21 AM]
$ ./gradlew :jfr-connection:test

FAILURE: Build failed with an exception.

* What went wrong:
A problem occurred configuring project ':buildSrc'.
> Could not resolve all artifacts for configuration ':buildSrc:classpath'.
   > Could not resolve com.diffplug.spotless:spotless-plugin-gradle:6.25.0.
     Required by:
         project :buildSrc > com.diffplug.spotless:com.diffplug.spotless.gradle.plugin:6.25.0
      > No matching variant of com.diffplug.spotless:spotless-plugin-gradle:6.25.0 was found. The consumer was configured to find a library for use during runtime, compatible with Java 8, packaged as a jar, and its dependencies declared externally, as well as attribute 'org.gradle.plugin.api-version' with value '8.7' but:
breedx-splk commented 2 months ago

(that pr mentioned above isn't relevant, I just accidentally mentioned the wrong issue).

breedx-splk commented 1 month ago

Ok, so with ./gradlew clean :jfr-connection:test -PtestJavaVersion=8 the build runs but fails just like this one used to:

Caused by: java.lang.UnsupportedClassVersionError: org/openjdk/jmc/flightrecorder/CouldNotLoadRecordingException has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 52.0

@trask can you think of a way to continue testing with 8 when literally the classes provided in the dependency are incompatible?

trask commented 1 month ago

@trask can you think of a way to continue testing with 8 when literally the classes provided in the dependency are incompatible?

pin the dependency version?

laurit commented 1 month ago

This dependency is used only for testing. I'd pin it. An alternative would be to rewrite the tests to use the jfr parser from the jdk.

breedx-splk commented 1 month ago

Yeah ok, pinning it will be.

An alternative would be to rewrite the tests to use the jfr parser from the jdk.

I don't think those exist in 8, do they?

laurit commented 1 month ago

I don't think those exist in 8, do they?

It doesn't exist in all versions of 8, should be available since openjdk 8u262.

renovate[bot] commented 1 month ago

Autoclosing Skipped

This PR has been flagged for autoclosing. However, it is being skipped due to the branch being already modified. Please close/delete it manually or report a bug if you think this is in error.