Skip to content

Commit 2a651ff

Browse files
committed
Add testLatestDeps() helper method
1 parent d9fba53 commit 2a651ff

68 files changed

Lines changed: 297 additions & 135 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/agents/knowledge/general-rules.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
2727
| Semconv | Dual semconv testing | `SemconvStability`, `maybeStable`, semconv Gradle tasks | `testing-semconv-stability.md` |
2828
| Testing | General test patterns | Test files in scope — assertion style, test method signatures and throws clauses, resource cleanup, attribute assertions | `testing-general-patterns.md` |
2929
| Testing | Experimental flag tests | `testExperimental`, experimental attribute assertions, `experimental` flags in JVM args or system properties | `testing-experimental-flags.md` |
30+
| Testing | Flag-gated / mode-dependent assertion shape (experimental, `testLatestDeps`, semconv) — hoist flag into per-class constant, inline ternary, `if` for structural differences | Test classes branching on `EXPERIMENTAL_ATTRIBUTES`, `Boolean.getBoolean("testLatestDeps")`, or `emitOld*`/`emitStable*` | `testing-general-patterns.md` |
3031
| Library | TelemetryBuilder/getter/setter patterns | Library instrumentation classes | `library-patterns.md` |
3132
| API | Deprecation and breaking-change policy | Public API changes | `api-deprecation-policy.md` |
3233
| Config | Config property stability/renames/removals | `otel.instrumentation.*` property changes, `DeclarativeConfigUtil` or `ConfigProperties` usage | `config-property-stability.md` |

.github/agents/knowledge/testing-experimental-flags.md

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,30 +51,44 @@ val testExperimental by registering(Test::class) {
5151

5252
## Java Test Patterns
5353

54-
Read the flag once into a `private static final boolean` at class level:
54+
For the cross-cutting shape — inline ternary with `null` for "absent", when to use top-level
55+
`if` blocks, and `assumeTrue(...)` guidance — see
56+
[testing-general-patterns.md](testing-general-patterns.md#flag-gated--mode-dependent-assertions).
57+
The experimental-specific patterns below build on that shape.
58+
59+
### Hoist the flag into a per-class `EXPERIMENTAL_ATTRIBUTES` constant
60+
61+
Unlike `testLatestDeps()` / `emitStable*Semconv()`, there is no shared helper for
62+
experimental flags — the property name is module-specific. Read it once into a
63+
`private static final boolean` near the top of the class and reference the constant
64+
everywhere in the file:
5565

5666
```java
5767
private static final boolean EXPERIMENTAL_ATTRIBUTES =
5868
Boolean.getBoolean("otel.instrumentation.<module>.experimental-span-attributes");
5969
```
6070

61-
Use inline ternary in assertions — `null` means attribute expected absent:
71+
When multiple experimental flags coexist in one class, use a more specific name per flag.
72+
Replace every in-file occurrence of the raw `Boolean.getBoolean("…")` call with the
73+
constant — including plain `if`, `&&`, and `assumeTrue(...)`, not just inside ternaries.
6274

63-
```java
64-
equalTo(ExperimentalAttributes.SOME_ATTR, EXPERIMENTAL_ATTRIBUTES ? "value" : null)
65-
```
75+
### Single-arg `experimental(value)` helper
6676

67-
When many assertions share the flag, extract an `experimental()` helper:
77+
Experimental attributes are by definition absent when the flag is off, so the off-branch is
78+
always `null`. When several assertions in the same class gate attributes on
79+
`EXPERIMENTAL_ATTRIBUTES`, extract a tiny helper:
6880

6981
```java
7082
@Nullable
7183
private static <T> T experimental(T value) {
7284
return EXPERIMENTAL_ATTRIBUTES ? value : null;
7385
}
86+
87+
equalTo(SOME_KEY, experimental("value"))
7488
```
7589

7690
For multiple test classes sharing the same flag, move the helper into a shared
7791
`ExperimentalTestHelper` class and static-import it.
7892

79-
Use `assumeTrue(EXPERIMENTAL_ATTRIBUTES)` only when an entire test is meaningful in
80-
experimental mode only — prefer the ternary/helper pattern so both modes are exercised.
93+
Do **not** add a two-argument `experimental(on, off)` helper — for the rare case where the
94+
off-branch is non-null, an inline ternary is more readable.

.github/agents/knowledge/testing-general-patterns.md

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,58 @@ unless the span ordering within a trace is genuinely non-deterministic (e.g., co
130130
producers/consumers, thread-pool fan-out, or channel interleaving). Sequential operations
131131
like `repeat {}` loops, single-child traces, and `flux` sequential emission produce spans
132132
in deterministic order — use `hasSpansSatisfyingExactly` for those.
133+
134+
## Flag-Gated / Mode-Dependent Assertions
135+
136+
Several test modes change which attributes, span names, status codes, or span indexes are
137+
expected:
138+
139+
- Experimental attributes (`-Dotel.instrumentation.<module>.experimental-*=true`) — see
140+
[testing-experimental-flags.md](testing-experimental-flags.md).
141+
- Semconv stability (`-Dotel.semconv-stability.opt-in=...`) — see
142+
[testing-semconv-stability.md](testing-semconv-stability.md).
143+
- `testLatestDeps` Gradle property — runs against the newest supported library versions
144+
instead of the pinned earliest-supported ones.
145+
146+
### Read the flag through a shared static helper, not a per-class field
147+
148+
Each flag has a shared static accessor; static-import it and call it directly. Never call
149+
`Boolean.getBoolean("…")` inline, never duplicate the property-name string at the call
150+
site, and replace any per-class `TEST_LATEST_DEPS` constant with the shared accessor —
151+
including in plain `if`, `&&`, and `assumeTrue(...)`, not just inside ternaries.
152+
153+
| Flag | Shared accessor | Where it lives |
154+
| --- | --- | --- |
155+
| `-PtestLatestDeps=true` | `testLatestDeps()` | `io.opentelemetry.instrumentation.testing.util.TestLatestDeps` (testing-common) |
156+
| `otel.semconv-stability.opt-in=…` | `emitStableDatabaseSemconv()`, `emitOldDatabaseSemconv()`, `emitStableCodeSemconv()`, etc. | `io.opentelemetry.instrumentation.api.internal.SemconvStability` |
157+
| `otel.instrumentation.<module>.experimental-*` | per-module `EXPERIMENTAL_ATTRIBUTES` constant — see [testing-experimental-flags.md](testing-experimental-flags.md) | within the test class |
158+
159+
### Inline ternary in `equalTo(...)` with `null` for "absent"
160+
161+
Push the ternary as deep as possible — into the `equalTo` value or single attribute key —
162+
rather than duplicating two whole `hasAttributesSatisfyingExactly(...)` blocks under a
163+
`flag ? a : b`. The assertion API treats `null` as "expect attribute absent":
164+
165+
```java
166+
equalTo(DB_USER, emitStableDatabaseSemconv() ? null : USER_DB)
167+
equalTo(ERROR_TYPE, emitStableDatabaseSemconv() ? "42601" : null)
168+
equalTo(SOME_KEY, EXPERIMENTAL_ATTRIBUTES ? "value" : null)
169+
span.hasName(testLatestDeps() ? "GET" : "HTTP GET")
170+
.hasParent(trace.getSpan(testLatestDeps() ? 0 : 1))
171+
```
172+
173+
### Use `if` (not `if/else`) when structure differs
174+
175+
When the *set* of attributes (count, keys) — not just values — differs between modes, use
176+
top-level `if` blocks rather than a ternary swapping the entire assertion list. For semconv
177+
this is **mandatory**: `/dup` mode must execute both branches, so never use `if/else`
178+
see [testing-semconv-stability.md](testing-semconv-stability.md) for details.
179+
180+
```java
181+
if (emitStableCodeSemconv()) {
182+
assertThat(attributes).containsEntry(CODE_FUNCTION_NAME, "MyClass.myMethod");
183+
}
184+
if (emitOldCodeSemconv()) {
185+
assertThat(attributes).containsEntry(CODE_NAMESPACE, "MyClass");
186+
}
187+
```

.github/agents/knowledge/testing-semconv-stability.md

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,26 +76,31 @@ for other domains `check { dependsOn(testStableSemconv) }`.
7676

7777
## Asserting Attributes in Tests
7878

79-
Preferred approach (most compact) — inline ternary with `emitStable*()`, where `null` means
80-
the attribute is expected absent:
79+
For the cross-cutting shape — inline ternary with `null` for "absent", static-imported flag
80+
accessors, and `assumeTrue(...)` guidance — see
81+
[testing-general-patterns.md](testing-general-patterns.md#flag-gated--mode-dependent-assertions).
82+
The semconv-specific patterns below build on that shape.
8183

82-
```java
83-
equalTo(DB_USER, emitStableDatabaseSemconv() ? null : USER_DB)
84-
equalTo(ERROR_TYPE, emitStableDatabaseSemconv() ? "42601" : null)
85-
span.hasName(emitStableDatabaseSemconv() ? "SELECT" : "SELECT dbname")
86-
```
84+
### `maybeStable(OLD_KEY)` for 1:1 key renames
8785

88-
Use `maybeStable(oldKey)` when only the attribute key changes and the value stays the same.
89-
`maybeStable()` does NOT cover `dup` mode — use separate `if` blocks for that.
86+
Use `maybeStable(OLD_KEY)` when only the attribute *key* flips between old and stable
87+
semconv and the value is identical:
9088

9189
```java
9290
span.hasAttribute(equalTo(maybeStable(DB_STATEMENT), "SELECT ?"));
9391
```
9492

95-
Use separate `if` blocks (not `if/else`) when assertion structure differs significantly between
96-
modes — this ensures both branches execute in `dup` mode:
93+
`maybeStable()` does **not** cover `/dup` mode (it returns one key, not both), and does
94+
**not** apply where the mapping isn't 1:1 — for example `DB_RESPONSE_STATUS_CODE`
95+
`ERROR_TYPE`. Use `emitOld*()` / `emitStable*()` `if` blocks for those.
96+
97+
### `if` blocks (never `if/else`) for `/dup` mode
98+
99+
`/dup` mode emits **both** old and stable attributes, so each must be asserted in an
100+
independent top-level `if` block. Never use `if/else`:
97101

98102
```java
103+
// CORRECT — both branches run in /dup mode
99104
if (emitStableCodeSemconv()) {
100105
assertThat(attributes).containsEntry(CODE_FUNCTION_NAME, "MyClass.myMethod");
101106
}
@@ -104,8 +109,6 @@ if (emitOldCodeSemconv()) {
104109
}
105110
```
106111

107-
Use `assumeTrue(emitStable*())` only when an entire test is meaningful in one mode only.
108-
109112
## Key Rules
110113

111114
- Add `@SuppressWarnings("deprecation")` at class level when tests use old Semconv constants.

conventions/src/main/kotlin/io/opentelemetry/instrumentation/gradle/StaticImportFormatter.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@ class StaticImportFormatter : FormatterFunc.NeedsFile, Serializable {
7777
"io.opentelemetry.instrumentation.api.internal.SemconvStability",
7878
"emit[a-zA-Z0-9]*",
7979
),
80+
Rule(
81+
"TestLatestDeps",
82+
"io.opentelemetry.instrumentation.testing.util.TestLatestDeps",
83+
"testLatestDeps",
84+
),
8085
Rule(
8186
"SqlDialect",
8287
"io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlDialect",

instrumentation/apache-dubbo-2.7/library-autoconfigure/src/test/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/DubboHeadersGetterTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.instrumentation.apachedubbo.v2_7;
77

8+
import static io.opentelemetry.instrumentation.testing.util.TestLatestDeps.testLatestDeps;
89
import static java.util.Collections.singletonMap;
910
import static org.assertj.core.api.Assertions.assertThat;
1011
import static org.mockito.Mockito.when;
@@ -32,7 +33,7 @@ void testKeys() throws ReflectiveOperationException {
3233
when(context.getLocalAddress()).thenReturn(new InetSocketAddress(1));
3334

3435
// for latest dep tests call getObjectAttachments, otherwise call getAttachments
35-
if (Boolean.getBoolean("testLatestDeps")) {
36+
if (testLatestDeps()) {
3637
when(getObjectAttachments()).thenReturn(singletonMap("key", "value"));
3738
} else {
3839
when(rpcInvocation.getAttachments()).thenReturn(singletonMap("key", "value"));

instrumentation/apache-dubbo-2.7/testing/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/AbstractDubboTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableRpcSemconv;
1010
import static io.opentelemetry.instrumentation.testing.GlobalTraceUtil.runWithSpan;
1111
import static io.opentelemetry.instrumentation.testing.junit.service.SemconvServiceStabilityUtil.maybeStablePeerService;
12+
import static io.opentelemetry.instrumentation.testing.util.TestLatestDeps.testLatestDeps;
1213
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
1314
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
1415
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
@@ -186,7 +187,7 @@ void testApacheDubboBase() throws ReflectiveOperationException {
186187
: "hello"),
187188
equalTo(
188189
maybeStablePeerService(),
189-
hasServicePeerName() && Boolean.getBoolean("testLatestDeps")
190+
hasServicePeerName() && testLatestDeps()
190191
? "test-peer-service"
191192
: null),
192193
satisfies(
@@ -370,7 +371,7 @@ void testApacheDubboTest() throws ReflectiveOperationException {
370371
: "hello"),
371372
equalTo(
372373
maybeStablePeerService(),
373-
hasServicePeerName() && Boolean.getBoolean("testLatestDeps")
374+
hasServicePeerName() && testLatestDeps()
374375
? "test-peer-service"
375376
: null),
376377
satisfies(
@@ -484,7 +485,7 @@ static void assertNetworkType(AbstractStringAssert<?> stringAssert) {
484485

485486
static void assertLatestDeps(
486487
AbstractAssert<?, ?> assertion, Consumer<AbstractAssert<?, ?>> action) {
487-
if (Boolean.getBoolean("testLatestDeps")) {
488+
if (testLatestDeps()) {
488489
action.accept(assertion);
489490
} else {
490491
assertion.isNull();

instrumentation/apache-dubbo-2.7/testing/src/main/java/io/opentelemetry/instrumentation/apachedubbo/v2_7/AbstractDubboTraceChainTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableRpcSemconv;
1111
import static io.opentelemetry.instrumentation.testing.GlobalTraceUtil.runWithSpan;
1212
import static io.opentelemetry.instrumentation.testing.junit.service.SemconvServiceStabilityUtil.maybeStablePeerService;
13+
import static io.opentelemetry.instrumentation.testing.util.TestLatestDeps.testLatestDeps;
1314
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
1415
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
1516
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
@@ -229,7 +230,7 @@ void testDubboChain() throws ReflectiveOperationException {
229230
: "hello"),
230231
equalTo(
231232
maybeStablePeerService(),
232-
hasServicePeerName() && Boolean.getBoolean("testLatestDeps")
233+
hasServicePeerName() && testLatestDeps()
233234
? "test-peer-service"
234235
: null),
235236
satisfies(
@@ -285,7 +286,7 @@ void testDubboChain() throws ReflectiveOperationException {
285286
: "hello"),
286287
equalTo(
287288
maybeStablePeerService(),
288-
hasServicePeerName() && Boolean.getBoolean("testLatestDeps")
289+
hasServicePeerName() && testLatestDeps()
289290
? "test-peer-service"
290291
: null),
291292
satisfies(
@@ -510,7 +511,7 @@ void testDubboChainInJvm() throws ReflectiveOperationException {
510511
: "hello"),
511512
equalTo(
512513
maybeStablePeerService(),
513-
hasServicePeerName() && Boolean.getBoolean("testLatestDeps")
514+
hasServicePeerName() && testLatestDeps()
514515
? "test-peer-service"
515516
: null),
516517
satisfies(

instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import static io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest.CONNECTION_TIMEOUT;
99
import static io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest.READ_TIMEOUT;
10+
import static io.opentelemetry.instrumentation.testing.util.TestLatestDeps.testLatestDeps;
1011

1112
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
1213
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
@@ -224,7 +225,7 @@ protected void configure(HttpClientTestOptions.Builder optionsBuilder) {
224225
// apparently apache http client does not report the 302 status code?
225226
optionsBuilder.setResponseCodeOnRedirectError(null);
226227

227-
if (Boolean.getBoolean("testLatestDeps")) {
228+
if (testLatestDeps()) {
228229
optionsBuilder.disableTestHttps();
229230
optionsBuilder.disableTestRemoteConnection();
230231
}

instrumentation/armeria/armeria-1.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaHttpServerTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
77

8+
import static io.opentelemetry.instrumentation.testing.util.TestLatestDeps.testLatestDeps;
9+
810
import com.linecorp.armeria.server.ServerBuilder;
911
import io.opentelemetry.instrumentation.armeria.v1_3.AbstractArmeriaHttpServerTest;
1012
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
@@ -31,7 +33,7 @@ protected void configure(HttpServerTestOptions options) {
3133
options.setTestHttpPipelining(false);
3234
// span for non-standard request is created by netty instrumentation when not running latest
3335
// dep tests
34-
if (Boolean.getBoolean("testLatestDeps")) {
36+
if (testLatestDeps()) {
3537
options.disableTestNonStandardHttpMethod();
3638
} else {
3739
options.setResponseCodeOnNonStandardHttpMethod(405);

0 commit comments

Comments
 (0)