Skip to content

Commit 8948946

Browse files
otelbot[bot]trask
otelbot[bot]
and
trask
authored
Code review sweep (run 24961972815) (#18316)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent d9fba53 commit 8948946

10 files changed

Lines changed: 56 additions & 60 deletions

File tree

‎instrumentation/geode-1.4/metadata.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ semantic_conventions:
66
- DATABASE_CLIENT_METRICS
77
configurations:
88
- name: otel.instrumentation.common.db.query-sanitization.enabled
9+
declarative_name: java.common.db.query_sanitization.enabled
910
description: Enables query sanitization for database queries.
1011
type: boolean
1112
default: true

‎instrumentation/google-http-client-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/googlehttpclient/v1_19/GoogleHttpRequestInstrumentation.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,12 @@ public static AdviceScope start(HttpRequest request) {
7373
return new AdviceScope(context, context.makeCurrent(), request);
7474
}
7575

76-
public void end(HttpResponse response, Throwable throwable) {
76+
public void end(@Nullable HttpResponse response, @Nullable Throwable throwable) {
7777
scope.close();
7878
instrumenter().end(context, request, response, throwable);
7979
}
8080

81-
public void endWhenThrown(HttpRequest request, Throwable throwable) {
81+
public void endWhenThrown(HttpRequest request, @Nullable Throwable throwable) {
8282
scope.close();
8383
if (throwable != null) {
8484
instrumenter().end(context, request, null, throwable);
@@ -119,8 +119,8 @@ public static AdviceScope methodEnter(@Advice.This HttpRequest request) {
119119

120120
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
121121
public static void methodExit(
122-
@Advice.Return HttpResponse response,
123-
@Advice.Thrown Throwable throwable,
122+
@Advice.Return @Nullable HttpResponse response,
123+
@Advice.Thrown @Nullable Throwable throwable,
124124
@Advice.Enter @Nullable AdviceScope scope) {
125125

126126
if (scope != null) {
@@ -146,7 +146,7 @@ public static AdviceScope methodEnter(@Advice.This HttpRequest request) {
146146
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
147147
public static void methodExit(
148148
@Advice.This HttpRequest request,
149-
@Advice.Thrown Throwable throwable,
149+
@Advice.Thrown @Nullable Throwable throwable,
150150
@Advice.Enter @Nullable AdviceScope scope) {
151151

152152
if (scope != null) {

‎instrumentation/graphql-java/graphql-java-20.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/graphql/v20_0/GraphqlInstrumentation.java‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,14 @@
55

66
package io.opentelemetry.javaagent.instrumentation.graphql.v20_0;
77

8-
import static io.opentelemetry.javaagent.instrumentation.graphql.v20_0.GraphqlSingletons.addInstrumentation;
98
import static net.bytebuddy.matcher.ElementMatchers.named;
109
import static net.bytebuddy.matcher.ElementMatchers.none;
1110

1211
import com.google.errorprone.annotations.CanIgnoreReturnValue;
13-
import graphql.execution.instrumentation.Instrumentation;
1412
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1513
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
1614
import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi;
1715
import net.bytebuddy.asm.Advice;
18-
import net.bytebuddy.asm.Advice.AssignReturned;
1916
import net.bytebuddy.asm.AsmVisitorWrapper;
2017
import net.bytebuddy.description.field.FieldDescription;
2118
import net.bytebuddy.description.field.FieldList;
@@ -38,7 +35,7 @@ public ElementMatcher<TypeDescription> typeMatcher() {
3835

3936
@Override
4037
public void transform(TypeTransformer transformer) {
41-
transformer.applyAdviceToMethod(none(), getClass().getName() + "$AddInstrumentationAdvice");
38+
transformer.applyAdviceToMethod(none(), getClass().getName() + "$InitAdvice");
4239

4340
transformer.applyTransformer(
4441
(builder, typeDescription, classLoader, javaModule, protectionDomain) ->
@@ -106,12 +103,13 @@ public void visitFieldInsn(
106103
}
107104

108105
@SuppressWarnings("unused")
109-
public static class AddInstrumentationAdvice {
110-
@AssignReturned.ToReturned
111-
@Advice.OnMethodExit(suppress = Throwable.class, inline = false)
112-
public static Instrumentation onExit(@Advice.Return Instrumentation instrumentation) {
113-
// this advice is here only to get GraphqlSingletons injected and checked by muzzle
114-
return addInstrumentation(instrumentation);
106+
public static class InitAdvice {
107+
@Advice.OnMethodEnter(inline = false)
108+
@SuppressWarnings("ReturnValueIgnored")
109+
public static void init() {
110+
// the sole purpose of this advice is to ensure that GraphqlSingletons is recognized as
111+
// helper class and injected into the application class loader
112+
GraphqlSingletons.class.getName();
115113
}
116114
}
117115
}

‎instrumentation/graphql-java/graphql-java-20.0/library/src/main/java/io/opentelemetry/instrumentation/graphql/v20_0/Graphql20OpenTelemetryInstrumentationState.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ public void setContext(Context context) {
2828
}
2929

3030
@Nullable
31-
public Context setContextForPath(ResultPath resultPath, Context context) {
31+
Context setContextForPath(ResultPath resultPath, Context context) {
3232
return contextStorage.putIfAbsent(resultPath.toString(), context);
3333
}
3434

35-
public Context getParentContextForPath(ResultPath resultPath) {
35+
Context getParentContextForPath(ResultPath resultPath) {
3636

3737
// Navigate up the path until we find the closest parent context
3838
for (ResultPath currentPath = resultPath.getParent();

‎instrumentation/graphql-java/graphql-java-common-12.0/library/src/main/java/io/opentelemetry/instrumentation/graphql/common/v12_0/internal/OpenTelemetryInstrumentationHelper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public static OpenTelemetryInstrumentationHelper create(
6464
openTelemetry, instrumentationName, ignored -> "GraphQL Operation")
6565
.setSpanStatusExtractor(
6666
(spanStatusBuilder, instrumentationExecutionParameters, executionResult, error) -> {
67-
if (!executionResult.getErrors().isEmpty()) {
67+
if (executionResult != null && !executionResult.getErrors().isEmpty()) {
6868
spanStatusBuilder.setStatus(StatusCode.ERROR);
6969
} else {
7070
SpanStatusExtractor.getDefault()

‎instrumentation/graphql-java/graphql-java-common-12.0/testing/src/main/java/io/opentelemetry/instrumentation/graphql/common/v12_0/AbstractGraphqlTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import static io.opentelemetry.semconv.incubating.GraphqlIncubatingAttributes.GRAPHQL_OPERATION_NAME;
1717
import static io.opentelemetry.semconv.incubating.GraphqlIncubatingAttributes.GRAPHQL_OPERATION_TYPE;
1818
import static java.nio.charset.StandardCharsets.UTF_8;
19+
import static java.util.Objects.requireNonNull;
1920

2021
import graphql.ExecutionResult;
2122
import graphql.GraphQL;
@@ -80,7 +81,8 @@ void setup() throws IOException {
8081

8182
try (Reader reader =
8283
new InputStreamReader(
83-
this.getClass().getClassLoader().getResourceAsStream("schema.graphqls"), UTF_8)) {
84+
requireNonNull(getClass().getClassLoader().getResourceAsStream("schema.graphqls")),
85+
UTF_8)) {
8486
graphqlSchema = buildSchema(reader);
8587
GraphQL.Builder graphqlBuilder = GraphQL.newGraphQL(graphqlSchema);
8688
configure(graphqlBuilder);

‎instrumentation/grpc-1.6/javaagent/build.gradle.kts‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,6 @@ tasks {
5353
testClassesDirs = sourceSets.test.get().output.classesDirs
5454
classpath = sourceSets.test.get().runtimeClasspath
5555

56-
// exclude our grpc library instrumentation, the ContextStorageOverride contained within it
57-
// breaks the tests
58-
classpath = classpath.filter {
59-
!it.absolutePath.contains("opentelemetry-grpc-1.6")
60-
}
61-
6256
systemProperty("metadataConfig", "otel.instrumentation.grpc.experimental-span-attributes=true")
6357
jvmArgs("-Dotel.instrumentation.grpc.experimental-span-attributes=true")
6458
}
@@ -67,12 +61,6 @@ tasks {
6761
testClassesDirs = sourceSets.test.get().output.classesDirs
6862
classpath = sourceSets.test.get().runtimeClasspath
6963

70-
// exclude our grpc library instrumentation, the ContextStorageOverride contained within it
71-
// breaks the tests
72-
classpath = classpath.filter {
73-
!it.absolutePath.contains("opentelemetry-grpc-1.6")
74-
}
75-
7664
jvmArgs("-Dotel.semconv-stability.opt-in=rpc")
7765
systemProperty("metadataConfig", "otel.semconv-stability.opt-in=rpc")
7866
}
@@ -81,12 +69,6 @@ tasks {
8169
testClassesDirs = sourceSets.test.get().output.classesDirs
8270
classpath = sourceSets.test.get().runtimeClasspath
8371

84-
// exclude our grpc library instrumentation, the ContextStorageOverride contained within it
85-
// breaks the tests
86-
classpath = classpath.filter {
87-
!it.absolutePath.contains("opentelemetry-grpc-1.6")
88-
}
89-
9072
jvmArgs("-Dotel.semconv-stability.opt-in=rpc/dup")
9173
systemProperty("metadataConfig", "otel.semconv-stability.opt-in=rpc/dup")
9274
}

‎instrumentation/grpc-1.6/library/src/test/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTest.java‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.instrumentation.grpc.v1_6;
77

8+
import static io.opentelemetry.api.common.AttributeKey.stringKey;
89
import static java.util.Collections.singletonList;
910
import static java.util.concurrent.TimeUnit.SECONDS;
1011
import static org.assertj.core.api.Assertions.assertThat;
@@ -20,7 +21,6 @@
2021
import io.grpc.Status;
2122
import io.grpc.stub.MetadataUtils;
2223
import io.grpc.stub.StreamObserver;
23-
import io.opentelemetry.api.common.AttributeKey;
2424
import io.opentelemetry.api.common.AttributesBuilder;
2525
import io.opentelemetry.api.trace.SpanKind;
2626
import io.opentelemetry.context.Context;
@@ -36,9 +36,6 @@ class GrpcTest extends AbstractGrpcTest {
3636
@RegisterExtension
3737
static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();
3838

39-
private static final AttributeKey<String> CUSTOM_KEY = AttributeKey.stringKey("customKey");
40-
private static final AttributeKey<String> CUSTOM_KEY2 = AttributeKey.stringKey("customKey2");
41-
4239
private static final Metadata.Key<String> CUSTOM_METADATA_KEY =
4340
Metadata.Key.of("customMetadataKey", Metadata.ASCII_STRING_MARSHALLER);
4441

@@ -134,14 +131,14 @@ public void sayHello(
134131
span.hasName("example.Greeter/SayHello")
135132
.hasKind(SpanKind.CLIENT)
136133
.hasParent(trace.getSpan(0))
137-
.hasAttribute(CUSTOM_KEY2, "clientSideValue")
138-
.hasAttribute(CUSTOM_KEY, "customValue"),
134+
.hasAttribute(stringKey("customKey2"), "clientSideValue")
135+
.hasAttribute(stringKey("customKey"), "customValue"),
139136
span ->
140137
span.hasName("example.Greeter/SayHello")
141138
.hasKind(SpanKind.SERVER)
142139
.hasParent(trace.getSpan(1))
143-
.hasAttribute(CUSTOM_KEY2, "serverSideValue")
144-
.hasAttribute(CUSTOM_KEY, "customValue")));
140+
.hasAttribute(stringKey("customKey2"), "serverSideValue")
141+
.hasAttribute(stringKey("customKey"), "customValue")));
145142
}
146143

147144
private static class CustomAttributesExtractor
@@ -161,7 +158,7 @@ public void onEnd(
161158

162159
Metadata metadata = grpcRequest.getMetadata();
163160
if (metadata != null) {
164-
attributes.put(CUSTOM_KEY, metadata.get(CUSTOM_METADATA_KEY));
161+
attributes.put(stringKey("customKey"), metadata.get(CUSTOM_METADATA_KEY));
165162
}
166163
}
167164
}
@@ -179,7 +176,7 @@ private static class CustomAttributesExtractorV2
179176
public void onStart(
180177
AttributesBuilder attributes, Context parentContext, GrpcRequest grpcRequest) {
181178

182-
attributes.put(CUSTOM_KEY2, valueOfKey2);
179+
attributes.put(stringKey("customKey2"), valueOfKey2);
183180
}
184181

185182
@Override

‎instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcStreamingTest.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ RPC_SERVICE, emitOldRpcSemconv() ? "example.Greeter" : null),
304304
histogram ->
305305
histogram.hasPointsSatisfying(
306306
point ->
307-
point.hasAttributesSatisfying(
307+
point.hasAttributesSatisfyingExactly(
308308
equalTo(SERVER_ADDRESS, "localhost"),
309309
equalTo(SERVER_PORT, server.getPort()),
310310
equalTo(RPC_METHOD, "Conversation"),
@@ -347,15 +347,19 @@ RPC_SERVICE, emitOldRpcSemconv() ? "example.Greeter" : null),
347347
histogram ->
348348
histogram.hasPointsSatisfying(
349349
point ->
350-
point.hasAttributesSatisfying(
350+
point.hasAttributesSatisfyingExactly(
351351
equalTo(SERVER_ADDRESS, "localhost"),
352352
equalTo(SERVER_PORT, server.getPort()),
353353
equalTo(RPC_METHOD, "Conversation"),
354354
equalTo(RPC_SERVICE, "example.Greeter"),
355355
equalTo(RPC_SYSTEM, "grpc"),
356356
equalTo(
357-
RPC_GRPC_STATUS_CODE,
358-
(long) Status.Code.OK.value())))));
357+
RPC_GRPC_STATUS_CODE, (long) Status.Code.OK.value()),
358+
equalTo(
359+
NETWORK_TYPE,
360+
Boolean.getBoolean("testLatestDeps")
361+
? "ipv4"
362+
: null)))));
359363
}
360364
if (emitStableRpcSemconv()) {
361365
testing()
@@ -369,7 +373,7 @@ RPC_SERVICE, emitOldRpcSemconv() ? "example.Greeter" : null),
369373
histogram ->
370374
histogram.hasPointsSatisfying(
371375
point ->
372-
point.hasAttributesSatisfying(
376+
point.hasAttributesSatisfyingExactly(
373377
equalTo(RPC_SYSTEM_NAME, "grpc"),
374378
equalTo(SERVER_ADDRESS, "localhost"),
375379
equalTo(SERVER_PORT, server.getPort()),

‎instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1799,14 +1799,18 @@ private void assertMetrics(Server server, Status.Code statusCode) {
17991799
histogram ->
18001800
histogram.hasPointsSatisfying(
18011801
point ->
1802-
point.hasAttributesSatisfying(
1802+
point.hasAttributesSatisfyingExactly(
18031803
equalTo(SERVER_ADDRESS, "localhost"),
18041804
equalTo(SERVER_PORT, server.getPort()),
18051805
equalTo(RPC_METHOD, "SayHello"),
18061806
equalTo(RPC_SERVICE, "example.Greeter"),
18071807
equalTo(RPC_SYSTEM, "grpc"),
1808+
equalTo(RPC_GRPC_STATUS_CODE, (long) statusCode.value()),
18081809
equalTo(
1809-
RPC_GRPC_STATUS_CODE, (long) statusCode.value())))));
1810+
NETWORK_TYPE,
1811+
Boolean.getBoolean("testLatestDeps")
1812+
? "ipv4"
1813+
: null)))));
18101814

18111815
testing()
18121816
.waitAndAssertMetrics(
@@ -1819,14 +1823,18 @@ private void assertMetrics(Server server, Status.Code statusCode) {
18191823
histogram ->
18201824
histogram.hasPointsSatisfying(
18211825
point ->
1822-
point.hasAttributesSatisfying(
1826+
point.hasAttributesSatisfyingExactly(
18231827
equalTo(SERVER_ADDRESS, "localhost"),
18241828
equalTo(SERVER_PORT, server.getPort()),
18251829
equalTo(RPC_METHOD, "SayHello"),
18261830
equalTo(RPC_SERVICE, "example.Greeter"),
18271831
equalTo(RPC_SYSTEM, "grpc"),
1832+
equalTo(RPC_GRPC_STATUS_CODE, (long) statusCode.value()),
18281833
equalTo(
1829-
RPC_GRPC_STATUS_CODE, (long) statusCode.value())))));
1834+
NETWORK_TYPE,
1835+
Boolean.getBoolean("testLatestDeps")
1836+
? "ipv4"
1837+
: null)))));
18301838
if (hasSizeMetric) {
18311839
testing()
18321840
.waitAndAssertMetrics(
@@ -1839,15 +1847,19 @@ private void assertMetrics(Server server, Status.Code statusCode) {
18391847
histogram ->
18401848
histogram.hasPointsSatisfying(
18411849
point ->
1842-
point.hasAttributesSatisfying(
1850+
point.hasAttributesSatisfyingExactly(
18431851
equalTo(SERVER_ADDRESS, "localhost"),
18441852
equalTo(SERVER_PORT, server.getPort()),
18451853
equalTo(RPC_METHOD, "SayHello"),
18461854
equalTo(RPC_SERVICE, "example.Greeter"),
18471855
equalTo(RPC_SYSTEM, "grpc"),
18481856
equalTo(
1849-
RPC_GRPC_STATUS_CODE,
1850-
(long) statusCode.value())))));
1857+
RPC_GRPC_STATUS_CODE, (long) statusCode.value()),
1858+
equalTo(
1859+
NETWORK_TYPE,
1860+
Boolean.getBoolean("testLatestDeps")
1861+
? "ipv4"
1862+
: null)))));
18511863
}
18521864
}
18531865
if (emitStableRpcSemconv()) {
@@ -1880,7 +1892,7 @@ private void assertMetrics(Server server, Status.Code statusCode) {
18801892
histogram ->
18811893
histogram.hasPointsSatisfying(
18821894
point ->
1883-
point.hasAttributesSatisfying(
1895+
point.hasAttributesSatisfyingExactly(
18841896
equalTo(RPC_SYSTEM_NAME, "grpc"),
18851897
equalTo(SERVER_ADDRESS, "localhost"),
18861898
equalTo(SERVER_PORT, server.getPort()),

0 commit comments

Comments
 (0)