Repository navigation
Declare OpenTelemetryKotlin in buildSrc - #808
Merged
Merged
Conversation
`core-jvm` declared this dependency object in its own copy of `buildSrc`, where `config` never had it, and its `server-otel` module imports it from `io.spine.dependency.lib`. Since the `ConfigTester` replaces `buildSrc` wholesale, the declaration disappears under the test build and `server-otel/build.gradle.kts` fails to compile. Upstreaming the object makes `config` its owner: the test build of `core-jvm` compiles, the artifact comes under the `dependency-update` sweep, and `core-jvm`'s next `./config/pull` overwrites its local copy with an identical file instead of leaving two sources of truth. The version stays at `0.4.0` — the one `core-jvm` sources compile against. The project is alpha and publishes no BOM, so moving to the current `0.9.0` is a dependency update to make with the consuming code at hand, not a side effect of this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
🟢 Approved
The declaration matches existing catalogue patterns and consumer usage, with no blocking issues identified; end-to-end verification remains outstanding.
No open issues 🎉
What changed in this PR
Adds OpenTelemetryKotlin to the shared dependency catalogue so replacing consumer buildSrc no longer removes the declaration needed by core-jvm.
Changes:
- Declares five OpenTelemetry Kotlin modules pinned to version
0.4.0. - Documents experimental API requirements and module roles.
| File | Description |
|---|---|
| buildSrc/src/main/kotlin/io/spine/dependency/lib/OpenTelemetryKotlin.kt | Adds shared dependency coordinates and usage documentation. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Declares
io.spine.dependency.lib.OpenTelemetryKotlininbuildSrc—io.opentelemetry.kotlin0.4.0, modulesapi,noop,core,implementation,compat.Why
core-jvmdeclared this object in its own copy ofbuildSrc(added bycore-jvm@e46a9a04, "Draftserver-otelmodule");confighas never had it —git log -S"OpenTelemetryKotlin"over the whole repository is empty. Theserver-otelmodule imports it:ConfigTesterrenames the repository'sbuildSrctobuildSrc-originalandcopies this one in its place, so under
buildDependantsthe declarationdisappears and
server-otel/build.gradle.ktsfails to compile with fourUnresolved reference 'OpenTelemetryKotlin'errors. That is the last repositoryfailing a
buildDependantsrun (baseandbase-typespass, the latter withSpineEventEngine/base-types#56).
Upstreaming makes
configthe owner of the declaration: the test build ofcore-jvmcompiles, the artifact comes under thedependency-updatesweep —which never saw the consumer-local copy — and
core-jvm's next./config/pulloverwrites its copy with an identical file instead of leaving two sources of
truth.
The version stays at 0.4.0, the one
core-jvm's sources compile against,with a comment saying so. Maven Central is at 0.9.0; the project is alpha and
publishes no BOM, so that bump belongs in a
dependency-updatepass made withthe consuming code at hand, not here.
The wider hazard
A consumer that adds a dependency object to its own
buildSrccarries a changeno one upstream knows about. Worth noting that
./config/pulland the testerdiffer here:
migrateinstallsbuildSrcwithcp -R buildSrc .., whichoverlays and therefore leaves consumer-local extras in place, whereas the
tester replaces the folder wholesale. So a file like this survives a pull but
fails the integration test — the test is the stricter of the two, and it is what
surfaced this. Both readings point the same way: a dependency object belongs in
config.Verification
./gradlew detekt— green. This also compilesbuildSrc, where the new filelives; the root project has no
buildordokkaGeneratetask.io.opentelemetry.kotlin.ExperimentalApiand thecreateOpenTelemetry { }entry point in
implementation(notcore) were checked against thepublished 0.4.0 artifacts, so the KDoc claims hold.
clean buildDependantswas not re-run for thischange. It needs Fix the
buildDependantstask #805 (the tester fix) and base-types#56 merged, or a worktreethat stacks all three.
Review note
dependency-auditandkotlin-engineerboth flagged that the versionedconstants (
val api = "$group:api:$version") cannot be looked up throughDependency.artifact { }, whoseartifactsmap is keyed by the versionlessmodulesentries, and asked for versionless constants instead. Deliberately notdone here:
Compiler,Time,Kotlin,Jackson,JacksonV2andJUnitin this sametree already pair versioned convenience properties with a versionless
moduleslist;artifact { }is the idiom of the other group (Ksp,PalantirJavaFormat). Nothing inconfigcalls it on this object.forceArtifactsandartifactsreadmodules, which is versionless andcorrect, so forcing works.
api(OpenTelemetryKotlin.api)incore-jvm— the build this change exists to fix.Picking one of the two idioms for the whole catalogue is a separate change.
🤖 Generated with Claude Code