Enforce library instrumentation package names matching their module names #12957
Conversation
trask
force-pushed
the
check-package-names
branch
from
1e73636 to
12de48e
Compare
| @@ -3,7 +3,7 @@ | |||
| * SPDX-License-Identifier: Apache-2.0 | |||
| */ | |||
|
|
|||
| package io.opentelemetry.instrumentation.netty.v4.common; | |||
| package io.opentelemetry.instrumentation.netty.common.v4_0; | |||
There was a problem hiding this comment.
I think this is the only public class that is affected
I'm not sure if we can gracefully deprecate since it's used inside of generics in other public classes (that haven't changed)
trask
changed the title
| @@ -5,7 +5,7 @@ plugins { | |||
| dependencies { | |||
| library("org.elasticsearch.client:elasticsearch-rest-client:7.0.0") | |||
| implementation("net.bytebuddy:byte-buddy") | |||
| implementation(project(":instrumentation:elasticsearch:elasticsearch-rest-common:library")) | |||
| implementation(project(":instrumentation:elasticsearch:elasticsearch-rest-common-5.0:library")) | |||
There was a problem hiding this comment.
is the idea with common modules that we would always add a version suffix for the lowest version number that it is used by?
There was a problem hiding this comment.
that was my first idea, but then I thought the more consistent strategy is
the common module version would match the version used in its build.gradle.kts
there are two common modules (lettuce-common and netty-common) that don't have a dependency on the instrumented library, in which case I didn't add any base version
There was a problem hiding this comment.
what do you think of adding a small blurb to https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/contributing/writing-instrumentation.md explaining the naming convention?
sorry didn't get to this, but going to merge to get the CI check in |
Related to #932