build: adopt hypertrace shared BOM, version catalog, and java-convention plugin#120
Conversation
Test Results15 files ±0 15 suites ±0 31s ⏱️ +2s Results for commit 6f6f46a. ± Comparison against base commit 0aa8c44. This pull request removes 5 and adds 5 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
78dc7b6 to
a0e37df
Compare
…ty 12 (CVE-2026-2332) The current build pulls service-framework 0.1.89 (Jetty 11.0.24, vulnerable to CVE-2026-2332 / CVE-2025-5115). service-framework 0.1.94 migrates to Jetty 12.1.9 (ee10), which requires JDK 17+. - Bump source/target compatibility to Java 17 across all subprojects - Bump CI workflows to Java 17 - Bump platform-metrics + platform-service-framework 0.1.89 -> 0.1.94 - Bump grpc-client-utils 0.13.16 -> 0.13.23 (resolution conflict version) - Bump junit-pioneer 2.0.0 -> 2.3.0 and mockito-core 5.2.0 -> 5.15.2 for JDK 17+ reflection compat - Add --add-opens to test task for junit-pioneer @SetEnvironmentVariable on JDK 17+ Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
a0e37df to
43cd165
Compare
The shared hypertrace gradle GitHub action launches Gradle with JDK 11 on the runner. Setting source/target compatibility to 17 caused 'invalid source release: 17' on subprojects in the build job. Switching to a Java toolchain lets Gradle auto-provision and compile with JDK 17 regardless of the launcher JDK the action uses.
| with: | ||
| distribution: 'temurin' | ||
| java-version: '11' | ||
| java-version: '17' |
There was a problem hiding this comment.
It's probably just as easy to standardize this repo to use the BOM and plugins, then the upgrades (And java 17) come for free.
There was a problem hiding this comment.
Migrates the project to the canonical hypertrace pattern: the shared hypertrace-bom catalog drives common dep/plugin versions and the java-convention plugin standardizes JVM toolchain handling. Repo-local deps move to gradle/libs.versions.toml exposed as localLibs. - settings.gradle.kts: apply org.hypertrace.dependency-settings 0.2.0 with catalogVersion 0.3.80 (auto-registers commonLibs and localLibs) - gradle/libs.versions.toml: repo-local catalog (mockito, log4j, junit- pioneer, caffeine, avro, hypertrace-config-partitioner-api, grpc-netty, hypertrace-grpcutils-context, plus the avro plugin which is not in BOM) - build.gradle.kts: replace per-module hand-rolled plugin versions with alias(commonLibs.plugins.*); apply hypertrace.java-convention (toolchain 21 default, releaseCompatibility 11) so dep/JDK upgrades come for free via BOM bumps - Subprojects: rewrite to alias(commonLibs.*)/alias(localLibs.*) and add api(platform(commonLibs.hypertrace.bom)) for managed versions - Generate gradle.lockfile per module (settings-gradle.lockfile not tracked per the BOM doc) - Add .github/workflows/update-locks.yml: weekly schedule keeps locks in sync with newly published BOM versions - Bump pr-build/pr-test java-version 17 to 21 to match toolchain default
Both `hypertrace.framework.metrics` and `hypertrace.framework.metrics.jakarta` (and the same pair for `service`) resolve to the same underlying artifact in hypertrace-bom 0.3.80 — the `-jakarta` suffix is a transitional alias for the in-flight Jakarta EE migration. The unsuffixed names are clearer and self- explanatory; switch to those. No resolved-dependency change; lockfiles unchanged.
aaron-steinfeld
left a comment
There was a problem hiding this comment.
thanks for doing this!
| with: | ||
| distribution: 'temurin' | ||
| java-version: '11' | ||
| java-version: '21' |
There was a problem hiding this comment.
FYI: no issue with changing this, but with the convention plugin the toolchains will govern the java version. At best, changing this may help caching (which is still important)
There was a problem hiding this comment.
also you can keep at 17 if you want, the plugin just defaults to 21
There was a problem hiding this comment.
Acknowledged — kept setup-java at 21 to match the convention plugin's toolchain default for cache hits. Per your follow-up, fine to keep at 17 too; flagged this as a no-op cosmetic if we want to flip back.
There was a problem hiding this comment.
Noted — keeping at 21 for the moment to keep cache hits aligned with the convention default.
| kafka-streams-avro-serde = { module = "io.confluent:kafka-streams-avro-serde" } | ||
| kafka-streams-test-utils = { module = "org.apache.kafka:kafka-streams-test-utils" } | ||
| avro = { module = "org.apache.avro:avro" } | ||
| hypertrace-grpcutils-context = { module = "org.hypertrace.core.grpcutils:grpc-context-utils", version.ref = "hypertrace-grpcutils" } |
There was a problem hiding this comment.
None of these are in the shared catalog? I would expect most would be (at least junit, mockito, grpc, avro and our own libs). If not, can move later. Omit the versions though, assuming they'd be governed by BOM.
There was a problem hiding this comment.
Done in 6f6f46a — moved mockito-core, log4j-slf4j2-impl, grpc-netty, and hypertrace-grpcutils-context to commonLibs. Dropped versions on kafka-bom-managed entries (avro, kafka-streams-avro-serde, kafka-streams-test-utils). Kept in localLibs: junit-pioneer, caffeine, hamcrest-core, partitioner-config-service-api, and the avro plugin — these aren't in the shared catalog yet.
| [libraries] | ||
| junit-pioneer = { module = "org.junit-pioneer:junit-pioneer", version.ref = "junit-pioneer" } | ||
| mockito-core = { module = "org.mockito:mockito-core", version.ref = "mockito" } | ||
| log4j-slf4j-impl = { module = "org.apache.logging.log4j:log4j-slf4j-impl", version.ref = "log4j" } |
There was a problem hiding this comment.
Careful on log4j, that should be the slf4j2 binding.
There was a problem hiding this comment.
Good catch, fixed in 6f6f46a — switched to commonLibs.log4j.slf4j2.impl. Resolves to log4j-slf4j2-impl:2.25.4 (BOM-managed) instead of the slf4j-1.x binding.
- Move shared-catalog deps from localLibs to commonLibs: mockito-core, log4j-slf4j2-impl, grpc-netty, hypertrace-grpcutils-context. The BOM governs the versions there, so the local catalog only carries deps truly outside the shared catalog. - Switch the log4j binding to log4j-slf4j2-impl (the slf4j2 binding) per reviewer note — log4j-slf4j-impl was the slf4j-1.x binding and pulls in the wrong API. - Drop versions for kafka-bom-managed entries (avro, kafka-streams-avro- serde, kafka-streams-test-utils) — the kafka-bom platform constrains them. Lockfiles regenerated; resolved versions now follow the BOM (mockito 5.8.0, log4j-slf4j2-impl 2.25.4) instead of hand-pinned ones.
Summary
Migrate this repo to the canonical hypertrace dependency pattern: the shared
hypertrace-bomcatalog drives common dep/plugin versions, thejava-conventionplugin standardises JVM toolchain handling, and a repo-localgradle/libs.versions.tomlcovers the rest. This addresses aaron-steinfeld's review request to "standardize this repo to use the BOM and plugins" so future CVE / dependency upgrades come for free via BOM bumps instead of one-off PRs.The Jetty 12 / CVE-2026-2332 fix that originally motivated this PR is now picked up transitively through the BOM (which pins
service-framework 0.1.94,grpc-client-utils 0.13.23, etc.). The original hand-rolled JDK 17 / dep bumps have been reverted in favour of letting the BOM andjava-conventionplugin manage them.Reference
Migrating to using the shared BOM — internal best-practices doc that documents this pattern. Used as reference repos:
hypertrace/config-service(settings, root build,update-locks.yml)hypertrace/document-storeChanges
Settings & catalogs
settings.gradle.kts: applyorg.hypertrace.dependency-settings:0.2.0withcatalogVersion = 0.3.80. The plugin auto-registerscommonLibs(from the BOM) andlocalLibs(fromgradle/libs.versions.toml) — no explicitversionCatalogs.create(...)block needed.gradle/libs.versions.toml(new): repo-local catalog for deps not in the shared BOM —mockito,log4j,junit-pioneer,caffeine,hamcrest-core,avro,kafka-streams-test-utils,kafka-streams-avro-serde,hypertrace-grpcutils-context,partitioner-config-service-api,grpc-netty, plus theorg.hypertrace.avro-plugin(not in BOM).Root build
build.gradle.kts: replace hand-rolled plugin versions withalias(commonLibs.plugins.*)(hypertrace.repository,hypertrace.ciutils,hypertrace.publish,hypertrace.codestyle,hypertrace.java.convention,owasp.dependencycheck).hypertrace.java-convention— defaults to toolchain JDK 21 withreleaseCompatibility = 11, so JDK / dep upgrades come for free via BOM bumps and downstream consumers below JDK 21 still work (bytecode targets Java 11).Subprojects
build.gradle.ktsfiles rewritten toalias(commonLibs.*)/alias(localLibs.*)references.api(platform(commonLibs.hypertrace.bom))alongside the existingapi(platform(project(":kafka-bom")))so Hypertrace BOM versions take effect.commonLibs.hypertrace.framework.metrics/servicealiases (rather than the-jakartaaliases that resolve to the same artifact in BOM 0.3.80) for clarity.Lockfiles
gradle.lockfileper module via./gradlew resolveAndLockAll --write-locks(5 lockfiles).settings-gradle.lockfiledeleted per the BOM doc — not tracked.CI workflows
.github/workflows/update-locks.yml(mirrorshypertrace/config-service) — weekly schedule +workflow_dispatchkeepsgradle.lockfiles in sync with newly published BOM versions automatically.pr-build.ymlandpr-test.yml: bumpsetup-javaJava 17 → 21 to match thejava-conventiontoolchain default. Note: the sharedhypertrace/github-actions/gradle@mainaction still launches Gradle on JDK 11 regardless ofsetup-java; the toolchain block then provisions JDK 21 for compile / test.Why this approach (vs. the original JDK 17 + service-framework bump)
Adopting the BOM solves the same CVE problem the original PR aimed at, but also:
hypertrace-bomplus a regenerated lockfile — no per-repo coordination.update-locks.ymlworkflow propagates BOM updates without human action.Test plan
./gradlew clean spotlessApply buildpasses locally withJAVA_HOME=$(/usr/libexec/java_home -v 21)(toolchain auto-provisions JDK 21 for compile / test)../gradlew resolveAndLockAll --write-locks.build & validatejob passes.testjob (jacoco + report upload) passes.dependency-checkjob passes — verify Jetty 12 has displaced Jetty 11 in the resolved graph and no high-CVSS CVEs remain.🤖 Generated with Claude Code