Skip to content

Commit 4b142bd

Browse files
otelbot[bot]trask
andauthored
Code review sweep (run 25128763258) (open-telemetry#18413)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent 5f63164 commit 4b142bd

9 files changed

Lines changed: 25 additions & 20 deletions

File tree

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -375,14 +375,17 @@ ambiguous and the cast would otherwise be required). Do not flag those cases.
375375

376376
## [Semconv] Constants by Module Type
377377

378-
- `library/src/main/`: incubating semconv constants (from
379-
`io.opentelemetry.semconv.incubating`) must be copied locally as `private static final`
380-
fields with a `// copied from <ClassName>` comment. Stable semconv constants (from
381-
`io.opentelemetry.semconv`) may be imported directly.
378+
- `library/src/main/`: constants from `io.opentelemetry.semconv.incubating.*` must be
379+
copied locally as `private static final` fields with a `// copied from <ClassName>`
380+
comment. Constants from `io.opentelemetry.semconv.*` (stable) must be imported
381+
directly via `import static` and must not be copied locally.
382382
- `javaagent/src/main/`: all semconv artifact constants (stable and incubating) may be used
383383
directly.
384384
- tests: all semconv artifact constants are allowed.
385385

386+
The trigger for copying is the import package, not the constant name. Only convert an
387+
import to a local copy when it comes from `io.opentelemetry.semconv.incubating.*`.
388+
386389
## [NewModule] New Instrumentation Checklist
387390

388391
If a new module is added, verify all of the following:

instrumentation/aws-lambda/aws-lambda-events-2.2/library/build.gradle.kts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ dependencies {
3737
testImplementation("uk.org.webcompere:system-stubs-jupiter")
3838
}
3939

40-
tasks.withType<Test>().configureEach {
40+
tasks.test {
4141
// required on jdk17
4242
jvmArgs("--add-opens=java.base/java.lang=ALL-UNNAMED")
4343
jvmArgs("--add-opens=java.base/java.util=ALL-UNNAMED")

instrumentation/aws-lambda/aws-lambda-events-2.2/testing/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/v2_2/AbstractAwsLambdaSqsEventHandlerTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ private static SQSEvent.SQSMessage newMessage() {
151151
Constructor<SQSEvent.SQSMessage> ctor = SQSEvent.SQSMessage.class.getDeclaredConstructor();
152152
ctor.setAccessible(true);
153153
return ctor.newInstance();
154-
} catch (ReflectiveOperationException | SecurityException t) {
155-
throw new AssertionError(t);
154+
} catch (ReflectiveOperationException | SecurityException e) {
155+
throw new AssertionError(e);
156156
}
157157
}
158158
}

instrumentation/aws-lambda/aws-lambda-events-3.11/library/src/test/java/io/opentelemetry/instrumentation/awslambdaevents/v3_11/AwsLambdaSqsEventWrapperTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ private static SQSEvent.SQSMessage newMessage() {
106106
try {
107107
Constructor<SQSEvent.SQSMessage> ctor = SQSEvent.SQSMessage.class.getDeclaredConstructor();
108108
return ctor.newInstance();
109-
} catch (ReflectiveOperationException t) {
110-
throw new LinkageError(t.getMessage(), t);
109+
} catch (ReflectiveOperationException e) {
110+
throw new LinkageError(e.getMessage(), e);
111111
}
112112
}
113113
}

instrumentation/aws-lambda/aws-lambda-events-3.11/library/src/test/java/io/opentelemetry/instrumentation/awslambdaevents/v3_11/AwsLambdaSqsMessageHandlerTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ private static SQSEvent.SQSMessage newMessage() {
148148
try {
149149
Constructor<SQSEvent.SQSMessage> ctor = SQSEvent.SQSMessage.class.getDeclaredConstructor();
150150
return ctor.newInstance();
151-
} catch (ReflectiveOperationException t) {
152-
throw new LinkageError(t.getMessage(), t);
151+
} catch (ReflectiveOperationException e) {
152+
throw new LinkageError(e.getMessage(), e);
153153
}
154154
}
155155

instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/S3ClientTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public AmazonS3ClientBuilder configureClient(AmazonS3ClientBuilder client) {
6363
@ParameterizedTest
6464
@MethodSource("provideS3Arguments")
6565
void testRequestHandlerIsHookedUpWithBuilder(boolean addHandler, int size, int position)
66-
throws Exception {
66+
throws ReflectiveOperationException {
6767
AmazonS3ClientBuilder builder = AmazonS3ClientBuilder.standard().withRegion(Regions.US_EAST_1);
6868

6969
if (addHandler) {
@@ -85,7 +85,8 @@ private static Stream<Arguments> provideS3Arguments() {
8585
@ParameterizedTest
8686
@MethodSource("provideS3Arguments")
8787
@SuppressWarnings("deprecation") // AmazonS3Client constructor is deprecated
88-
void testRequestHandlerIsHookedUpWithConstructor(boolean addHandler, int size) throws Exception {
88+
void testRequestHandlerIsHookedUpWithConstructor(boolean addHandler, int size)
89+
throws ReflectiveOperationException {
8990
BasicAWSCredentials credentials = new BasicAWSCredentials("asdf", "qwerty");
9091
AmazonS3Client client = new AmazonS3Client(credentials);
9192
if (addHandler) {

instrumentation/aws-sdk/aws-sdk-1.11/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/SqsImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ private static void addTracing(
9696
Request<?> request,
9797
Response<?> response,
9898
Instrumenter<SqsProcessRequest, Response<?>> consumerProcessInstrumenter,
99-
Context receiveContext) {
99+
@Nullable Context receiveContext) {
100100
if (messagesField == null) {
101101
return;
102102
}

instrumentation/aws-sdk/aws-sdk-1.11/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/TracingIterator.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class TracingIterator implements Iterator<Message> {
2020
private final Instrumenter<SqsProcessRequest, Response<?>> instrumenter;
2121
private final Request<?> request;
2222
private final Response<?> response;
23-
private final Context receiveContext;
23+
@Nullable private final Context receiveContext;
2424

2525
/*
2626
* Note: this may potentially create problems if this iterator is used from different threads. But
@@ -35,7 +35,7 @@ private TracingIterator(
3535
Instrumenter<SqsProcessRequest, Response<?>> instrumenter,
3636
Request<?> request,
3737
Response<?> response,
38-
Context receiveContext) {
38+
@Nullable Context receiveContext) {
3939
this.delegateIterator = delegateIterator;
4040
this.instrumenter = instrumenter;
4141
this.request = request;
@@ -48,7 +48,7 @@ static Iterator<Message> wrap(
4848
Instrumenter<SqsProcessRequest, Response<?>> instrumenter,
4949
Request<?> request,
5050
Response<?> response,
51-
Context receiveContext) {
51+
@Nullable Context receiveContext) {
5252
return new TracingIterator(delegateIterator, instrumenter, request, response, receiveContext);
5353
}
5454

instrumentation/aws-sdk/aws-sdk-1.11/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/TracingList.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,23 @@
1515
import java.util.Iterator;
1616
import java.util.List;
1717
import java.util.function.Consumer;
18+
import javax.annotation.Nullable;
1819

1920
class TracingList extends SdkInternalList<Message> {
2021
private static final long serialVersionUID = 1L;
2122

2223
private final transient Instrumenter<SqsProcessRequest, Response<?>> instrumenter;
2324
private final transient Request<?> request;
2425
private final transient Response<?> response;
25-
private final transient Context receiveContext;
26+
@Nullable private final transient Context receiveContext;
2627
private boolean firstIterator = true;
2728

2829
private TracingList(
2930
List<Message> list,
3031
Instrumenter<SqsProcessRequest, Response<?>> instrumenter,
3132
Request<?> request,
3233
Response<?> response,
33-
Context receiveContext) {
34+
@Nullable Context receiveContext) {
3435
super(list);
3536
this.instrumenter = instrumenter;
3637
this.request = request;
@@ -43,7 +44,7 @@ static SdkInternalList<Message> wrap(
4344
Instrumenter<SqsProcessRequest, Response<?>> instrumenter,
4445
Request<?> request,
4546
Response<?> response,
46-
Context receiveContext) {
47+
@Nullable Context receiveContext) {
4748
return new TracingList(list, instrumenter, request, response, receiveContext);
4849
}
4950

0 commit comments

Comments
 (0)