Skip to content

Commit efa2151

Browse files
committed
Merge remote-tracking branch 'upstream/main' into isin
# Conflicts: # instrumentation/elasticsearch/elasticsearch-transport-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/transport/AbstractElasticsearchTransportClientTest.java # instrumentation/netty/netty-4.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/client/Netty40ConnectionSpanTest.java # instrumentation/netty/netty-4.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/Netty41ClientSslTest.java # instrumentation/netty/netty-4.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/Netty41ConnectionSpanTest.java
2 parents 56ea177 + d1e3fa1 commit efa2151

519 files changed

Lines changed: 1959 additions & 1916 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: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
1717
| Style | Style guide | Always ||
1818
| Style | Uppercase field names should reflect semantic constants or immutable value constants such as `Duration` timeouts/intervals, not simply `static final` | Always ||
1919
| Naming | Getter naming (`get` / `is`) | Always ||
20-
| Naming | Boolean/collection option naming (`set*Enabled`, `setCapture*`, `setEmitExperimental*`) | `Experimental` or `TelemetryBuilder` setter changes | `library-patterns.md` |
2120
| Naming | Module/package naming | New or renamed modules/packages | `module-naming.md` |
2221
| Javaagent | Advice patterns | `@Advice` classes | `javaagent-advice-patterns.md` |
2322
| Javaagent | Module structure patterns | `InstrumentationModule`, `TypeInstrumentation`, `Singletons` | `javaagent-module-patterns.md` |

.github/agents/knowledge/gradle-conventions.md

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,37 @@ Verify `build.gradle.kts` applies the correct plugin for the module type:
5151
| `library/` | `otel.library-instrumentation` |
5252
| `testing/` | `otel.java-conventions` |
5353

54-
## Boolean Project Properties
54+
## Shared Gradle Project Properties
5555

56-
For simple boolean Gradle project properties in `build.gradle.kts`, prefer the repository's most
57-
common pattern:
56+
In `build.gradle.kts`, prefer the shared `otelProps` extension for repository-wide Gradle project
57+
properties that are already modeled there, including:
58+
59+
- `testLatestDeps`
60+
- `denyUnsafe`
61+
- `collectMetadata`
62+
- `testJavaVersion`
63+
- `testJavaVM`
64+
- `maxTestRetries`
65+
- `enableStrictContext`
66+
67+
Examples:
5868

5969
```kotlin
60-
val myFlag = findProperty("myFlag") == "true"
61-
if (myFlag) {
70+
if (otelProps.testLatestDeps) {
6271
// ...
6372
}
73+
74+
tasks.withType<Test>().configureEach {
75+
systemProperty("collectMetadata", otelProps.collectMetadata)
76+
}
6477
```
6578

79+
For module-local one-off properties that are not part of `otelProps`, using `findProperty(...)`
80+
directly is still fine.
81+
82+
`settings.gradle.kts` is the exception: it cannot use project extensions like `otelProps`, so
83+
direct `gradle.startParameter.projectProperties[...]` access is expected there.
84+
6685
## `testInstrumentation` Dependencies
6786

6887
The `testInstrumentation` configuration declares which other javaagent instrumentation modules
@@ -186,7 +205,7 @@ should apply to all tasks. If so, move them to `withType<Test>().configureEach`.
186205
```kotlin
187206
tasks {
188207
withType<Test>().configureEach {
189-
systemProperty("collectMetadata", findProperty("collectMetadata"))
208+
systemProperty("collectMetadata", otelProps.collectMetadata)
190209
// ... other properties common to all test tasks
191210
}
192211

@@ -205,7 +224,7 @@ review**. Only verify correctness when they are already present.
205224

206225
| Property | Type | Value |
207226
| --- | --- | --- |
208-
| `collectMetadata` | System property | Pass-through of the `collectMetadata` Gradle project property; defaults to `"false"` |
227+
| `collectMetadata` | System property | Pass-through of `otelProps.collectMetadata`; defaults to `false` |
209228
| `metadataConfig` | System property | A single `key=value` string describing the non-default configuration active during this test run |
210229

211230
When already present, verify:

.github/agents/knowledge/library-patterns.md

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -44,32 +44,3 @@ public final class MyLibraryTelemetry {
4444
- Builder setter methods return `this` and are annotated `@CanIgnoreReturnValue`.
4545
- The builder's constructor is package-private — only `Telemetry.builder()` creates it.
4646
- `build()` returns the `{Library}Telemetry` instance.
47-
48-
## Boolean and Collection Option Naming
49-
50-
Builder methods on `TelemetryBuilder` and `Experimental` classes must follow these
51-
naming patterns. Flag any new method that deviates.
52-
53-
### Canonical patterns
54-
55-
| Semantic | Pattern | Examples |
56-
| --- | --- | --- |
57-
| Feature on/off toggle | `set*Enabled(boolean)` | `setSqlCommenterEnabled`, `setQuerySanitizationEnabled`, `setDataSourceInstrumenterEnabled` |
58-
| Emit experimental signals | `setEmitExperimental*(boolean)` | `setEmitExperimentalTelemetry`, `setEmitExperimentalMetrics` |
59-
| Capture data (boolean) | `setCapture*(boolean)` | `setCaptureExperimentalSpanAttributes`, `setCaptureEnduserId`, `setCaptureQuery` |
60-
| Capture data (collection) | `setCapture*(Collection)` | `setCaptureRequestHeaders`, `setCaptureResponseHeaders`, `setCaptureRequestParameters` |
61-
62-
### Rules
63-
64-
- **`set*Enabled`** is for features that are turned on or off (sanitization, SQL commenter,
65-
instrumenter toggles). The thing being named is a feature or processing step.
66-
- **`setCapture*`** is for options that control whether a piece of data is collected or
67-
emitted (attributes, headers, query text, message content). Use the verb form for both
68-
boolean and collection variants.
69-
- **`setEmitExperimental*`** is reserved for the `Experimental` class toggle that enables
70-
all experimental telemetry for an instrumentation. Do not use on `TelemetryBuilder`.
71-
72-
### Known exceptions
73-
74-
- `setPreferJfrMetrics(boolean)` — selects JFR over JMX for overlapping metrics; it is a
75-
strategy selector, not a feature toggle or data capture flag.

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

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,29 @@
33
## Quick Reference
44

55
- Use when: test files (`**/src/test/**`) are in scope
6-
- Review focus: assertion style, test class visibility, attribute assertion patterns
6+
- Review focus: assertion style, test class visibility, resource cleanup patterns, attribute assertion patterns
77

88
## Assertion Framework
99

1010
- JUnit 5, AssertJ assertions (not JUnit `assertEquals`/`assertTrue`).
1111
- Test classes and methods should be package-private (no `public`).
1212

13+
## Test Resource Cleanup
14+
15+
- In JUnit tests, when an `AutoCloseable` is intended to remain live for most or all of the test
16+
and only needs cleanup at test end, prefer `AutoCleanupExtension` with `deferCleanup(...)`
17+
over wrapping most of the test body in try-with-resources.
18+
- Reuse an existing `cleanup` extension when one is already in scope.
19+
Otherwise, add a `@RegisterExtension` field when the deferred-cleanup pattern improves
20+
clarity or avoids wrapping most of the test body.
21+
- Keep try-with-resources for semantically scoped resources whose lifetime must end before the
22+
rest of the test continues, such as `Scope` / `Context.makeCurrent()`, Mockito
23+
`MockedStatic`, and short-lived readers, writers, streams, response bodies, or parsers.
24+
- Do not use `AutoCleanupExtension` in non-JUnit helper methods, `@BeforeAll`, or other shared
25+
setup code where cleanup is not tied to a single test method.
26+
- If the test intentionally closes the resource mid-test or asserts behavior around explicit
27+
close, keep the direct close or try-with-resources in the test body.
28+
1329
## Span Attribute Assertions
1430

1531
- Use `span.hasAttributesSatisfyingExactly(...)` with `equalTo(...)`/`satisfies(...)` for
@@ -30,19 +46,24 @@
3046
`equalTo(AttributeKey<Long>, int)` overload, so `equalTo(longKey("iteration"), iteration)` is
3147
preferred over `equalTo(longKey("iteration"), (long) iteration)`.
3248

33-
## `satisfies()` Lambda Parameters
49+
## Attribute Assertion `satisfies()` Lambda Parameters
3450

35-
**`satisfies()` lambda parameters are `AbstractAssert` instances, not raw values.**
36-
Inside a `satisfies(AttributeKey, Consumer)` lambda the parameter (e.g., `taskId`) is an
37-
`AbstractStringAssert<?>` (for string keys), `AbstractLongAssert<?>` (for long keys), etc.
51+
**Attribute-assertion `satisfies()` lambda parameters are `AbstractAssert` instances, not raw
52+
values.** Inside a `satisfies(AttributeKey, Consumer)` lambda the parameter (e.g., `taskId`) is
53+
an `AbstractStringAssert<?>` (for string keys), `AbstractLongAssert<?>` (for long keys), etc.
3854
Fluent assertion calls like `taskId.contains(jobName)` or `taskId.startsWith(prefix)` are
3955
already proper AssertJ assertions — they throw on failure. Do **not** wrap them in
4056
`assertThat(value.contains(x)).isTrue()`, which degrades the failure message.
4157

42-
- For generic outer `satisfies(AttributeKey, lambda)` parameters, use `val`.
58+
- For `satisfies(AttributeKey, lambda)` outer parameters, use `val`.
4359
Do not use short generic alternatives like `k` or `v` for the outer parameter.
44-
- If a `satisfies(...)` lambda contains a nested inner lambda and a second parameter name is
45-
required, keep the outer parameter as `val` and use `v` for the nested parameter.
60+
- If an attribute-assertion `satisfies(...)` lambda contains a nested inner lambda and a second
61+
parameter name is required, keep the outer parameter as `val` and use `v` for the nested
62+
parameter.
63+
- This naming guidance does **not** apply to non-attribute `satisfies(...)` usages such as
64+
`span.satisfies(...)`, `point.satisfies(...)`, or `assertThat(result).satisfies(...)`.
65+
In those cases, prefer a descriptive subject name like `spanData`, `pointData`, `resource`,
66+
or `result` instead of generic names like `val`.
4667

4768
## AssertJ Idiomatic Simplifications
4869

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
and deprecated config key `otel.instrumentation.graphql.add-operation-name-to-span-name.enabled`
99
in favor of `setQuerySanitizationEnabled()`, `setOperationNameInSpanNameEnabled()`, and
1010
`otel.instrumentation.graphql.operation-name-in-span-name.enabled`
11+
- Deprecated the declarative config name `statement_sanitizer` in favor of `query_sanitization`
12+
- Deprecated the declarative config group `common.database` in favor of `common.db`
13+
- Deprecated the GraphQL declarative config name `query_sanitizer` in favor of
14+
`query_sanitization`
1115

1216
## Version 2.26.1 (2026-03-23)
1317

conventions/src/main/kotlin/io.opentelemetry.instrumentation.base.gradle.kts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
/** Common setup for manual instrumentation of libraries and javaagent instrumentation. */
22

3+
import io.opentelemetry.instrumentation.gradle.OtelPropsExtension
4+
35
plugins {
46
`java-library`
7+
id("otel.props-conventions")
58
}
69

10+
val otelProps = the<OtelPropsExtension>()
11+
712
/**
813
* We define three dependency configurations to use when adding dependencies to libraries being
914
* instrumented.
@@ -26,9 +31,6 @@ plugins {
2631
* precedence. Use this to restrict the latest version dependency from the default `+`, for
2732
* example to restrict to just a major version by specifying `2.+`.
2833
*/
29-
30-
val testLatestDeps = gradle.startParameter.projectProperties["testLatestDeps"] == "true"
31-
3234
@CacheableRule
3335
abstract class TestLatestDepsRule : ComponentMetadataRule {
3436
override fun execute(context: ComponentMetadataContext) {
@@ -77,15 +79,15 @@ configurations {
7779
// mutating the version for latest dep tests.
7880
configuration.dependencies.whenObjectAdded {
7981
val dep = copy()
80-
if (testLatestDeps) {
82+
if (otelProps.testLatestDeps) {
8183
(dep as ExternalDependency).version {
8284
require("latest.release")
8385
}
8486
}
8587
testImplementation.dependencies.add(dep)
8688
}
8789
}
88-
if (testLatestDeps) {
90+
if (otelProps.testLatestDeps) {
8991
dependencies {
9092
components {
9193
all<TestLatestDepsRule>()
@@ -108,7 +110,7 @@ configurations {
108110
}
109111
}
110112

111-
if (testLatestDeps) {
113+
if (otelProps.testLatestDeps) {
112114
afterEvaluate {
113115
tasks {
114116
withType<JavaCompile>().configureEach {

conventions/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-testing.gradle.kts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import io.opentelemetry.instrumentation.gradle.OtelPropsExtension
2+
13
plugins {
24
`java-library`
35

@@ -14,7 +16,7 @@ val testIndyProperty = providers.gradleProperty("testIndy")
1416
.map { it == "true" }
1517
.orElse(false)
1618

17-
val denyUnsafe = gradle.startParameter.projectProperties["denyUnsafe"] == "true"
19+
val otelProps = the<OtelPropsExtension>()
1820

1921
dependencies {
2022
/*
@@ -148,7 +150,7 @@ afterEvaluate {
148150
shadowJar.archiveFile.get().asFile,
149151
failOnContextLeakOverride,
150152
testIndyEnabled,
151-
denyUnsafe
153+
otelProps.denyUnsafe
152154
)
153155
)
154156

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.instrumentation.gradle
7+
8+
import org.gradle.api.JavaVersion
9+
import org.gradle.api.Project
10+
11+
open class OtelPropsExtension(private val project: Project) {
12+
val testLatestDeps: Boolean
13+
get() = project.findProperty("testLatestDeps") == "true"
14+
15+
val denyUnsafe: Boolean
16+
get() = project.findProperty("denyUnsafe") == "true"
17+
18+
val collectMetadata: Boolean
19+
get() = project.findProperty("collectMetadata") == "true"
20+
21+
val testJavaVersion: JavaVersion?
22+
get() = project.findProperty("testJavaVersion")?.toString()?.let(JavaVersion::toVersion)
23+
24+
val testJavaVM: String?
25+
get() = project.findProperty("testJavaVM")?.toString()
26+
27+
val maxTestRetries: Int?
28+
get() = project.findProperty("maxTestRetries")?.toString()?.toInt()
29+
30+
val enableStrictContext: Boolean
31+
get() = project.findProperty("enableStrictContext")?.toString()?.toBoolean() ?: true
32+
}

conventions/src/main/kotlin/otel.errorprone-conventions.gradle.kts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
import io.opentelemetry.instrumentation.gradle.OtelPropsExtension
12
import net.ltgt.gradle.errorprone.errorprone
23

34
plugins {
5+
id("otel.props-conventions")
46
id("net.ltgt.errorprone")
57
}
68

@@ -10,7 +12,7 @@ dependencies {
1012
}
1113

1214
val disableErrorProne = properties["disableErrorProne"]?.toString()?.toBoolean() ?: false
13-
val testLatestDeps = gradle.startParameter.projectProperties["testLatestDeps"] == "true"
15+
val otelProps = the<OtelPropsExtension>()
1416

1517
tasks {
1618
withType<JavaCompile>().configureEach {
@@ -133,7 +135,7 @@ tasks {
133135
disable("AddNullMarkedToClass")
134136
disable("AddNullMarkedToPackageInfo")
135137

136-
if (testLatestDeps) {
138+
if (otelProps.testLatestDeps) {
137139
// Some latest dep tests are compiled for java 17 although the base version uses an older
138140
// version. Disable rules that suggest using new language features.
139141
disable("StatementSwitchToExpressionSwitch")

conventions/src/main/kotlin/otel.java-conventions.gradle.kts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import io.opentelemetry.instrumentation.gradle.OtelJavaExtension
2+
import io.opentelemetry.instrumentation.gradle.OtelPropsExtension
23
import org.gradle.api.tasks.testing.logging.TestExceptionFormat
34
import java.time.Duration
45

@@ -8,12 +9,14 @@ plugins {
89
checkstyle
910
idea
1011

12+
id("otel.props-conventions")
1113
id("otel.errorprone-conventions")
1214
id("otel.spotless-conventions")
1315
id("org.sonatype.gradle.plugins.scan")
1416
}
1517

1618
val otelJava = extensions.create<OtelJavaExtension>("otelJava")
19+
val otelProps = the<OtelPropsExtension>()
1720

1821
afterEvaluate {
1922
val previousBaseArchiveName = base.archivesName.get()
@@ -320,9 +323,8 @@ tasks.withType<Test>().configureEach {
320323
useJUnitPlatform()
321324

322325
// work around jvm crash on openJ9 8 after updating armeria to 1.33.1
323-
val testJavaVersion = gradle.startParameter.projectProperties["testJavaVersion"]?.let(JavaVersion::toVersion)
324-
val useJ9 = gradle.startParameter.projectProperties["testJavaVM"]?.run { this == "openj9" }
325-
?: false
326+
val testJavaVersion = otelProps.testJavaVersion
327+
val useJ9 = otelProps.testJavaVM == "openj9"
326328
if (useJ9 && testJavaVersion != null && testJavaVersion.isJava8) {
327329
jvmArgs("-Xjit:exclude={io/opentelemetry/testing/internal/io/netty/buffer/HeapByteBufUtil.*}," +
328330
"exclude={io/opentelemetry/testing/internal/io/netty/buffer/UnpooledHeapByteBuf.*}," +
@@ -332,10 +334,10 @@ tasks.withType<Test>().configureEach {
332334

333335
// There's no real harm in setting this for all tests even if any happen to not be using context
334336
// propagation.
335-
jvmArgs("-Dio.opentelemetry.context.enableStrictContext=${rootProject.findProperty("enableStrictContext") ?: true}")
337+
jvmArgs("-Dio.opentelemetry.context.enableStrictContext=${otelProps.enableStrictContext}")
336338
// TODO: Have agent map unshaded to shaded.
337339
if (project.findProperty("disableShadowRelocate") != "true") {
338-
jvmArgs("-Dio.opentelemetry.javaagent.shaded.io.opentelemetry.context.enableStrictContext=${rootProject.findProperty("enableStrictContext") ?: true}")
340+
jvmArgs("-Dio.opentelemetry.javaagent.shaded.io.opentelemetry.context.enableStrictContext=${otelProps.enableStrictContext}")
339341
} else {
340342
jvmArgs("-Dotel.instrumentation.opentelemetry-api.enabled=false")
341343
jvmArgs("-Dotel.instrumentation.opentelemetry-instrumentation-api.enabled=false")
@@ -359,7 +361,7 @@ tasks.withType<Test>().configureEach {
359361
timeout.set(Duration.ofMinutes(15))
360362

361363
val defaultMaxRetries = if (System.getenv().containsKey("CI")) 5 else 0
362-
val maxTestRetries = gradle.startParameter.projectProperties["maxTestRetries"]?.toInt() ?: defaultMaxRetries
364+
val maxTestRetries = otelProps.maxTestRetries ?: defaultMaxRetries
363365

364366
develocity.testRetry {
365367
// You can see tests that were retried by this mechanism in the collected test reports and build scans.
@@ -388,9 +390,8 @@ class KeystoreArgumentsProvider(
388390
}
389391

390392
afterEvaluate {
391-
val testJavaVersion = gradle.startParameter.projectProperties["testJavaVersion"]?.let(JavaVersion::toVersion)
392-
val useJ9 = gradle.startParameter.projectProperties["testJavaVM"]?.run { this == "openj9" }
393-
?: false
393+
val testJavaVersion = otelProps.testJavaVersion
394+
val useJ9 = otelProps.testJavaVM == "openj9"
394395
tasks.withType<Test>().configureEach {
395396
if (testJavaVersion != null) {
396397
javaLauncher.set(

0 commit comments

Comments
 (0)