Peer service testing #14963
Peer service testing #14963
Conversation
|
🔧 The result from spotlessApply was committed to the PR branch. |
1 similar comment
|
🔧 The result from spotlessApply was committed to the PR branch. |
trask
force-pushed
the
peer-service-testing
branch
from
4d2a8de to
1d59409
Compare
| tasks { | ||
| test { | ||
| systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false") | ||
| systemProperty("otel.instrumentation.common.peer-service-mapping", "127.0.0.1=test-peer-service,localhost=test-peer-service,192.0.2.1=test-peer-service") |
There was a problem hiding this comment.
this could be simplified by #13434
trask
force-pushed
the
peer-service-testing
branch
4 times, most recently
from
6dfc2bc to
53e3efa
Compare
Instead of using a global AutoConfigurationCustomizerProvider that applies peer service mapping to all instrumentations (including JDBC, etc), add the system property directly to each HTTP client javaagent test configuration. This ensures peer service testing is targeted only at HTTP client instrumentations where it's needed. Modified modules: - apache-httpclient (2.0, 4.0, 5.0) - apache-httpasyncclient-4.1 - async-http-client (1.9, 2.0) - google-http-client-1.19 - http-url-connection - java-http-client - jetty-httpclient (9.2, 12.0) - jodd-http-4.2 - kubernetes-client-7.0 - netty (3.8, 4.0, 4.1) - okhttp (2.2, 3.0) - play-ws (1.0, 2.0, 2.1) - reactor-netty (0.9, 1.0) - spring-web (3.1, 6.0) - spring-webflux-5.0 - vertx-http-client (3.0, 4.0, 5.0) Deleted: - PeerServiceTestConfigSupplier.java (global approach)
trask
force-pushed
the
peer-service-testing
branch
from
387f86e to
3378a01
Compare
|
🔧 The result from spotlessApply was committed to the PR branch. |
|
🔧 The result from spotlessApply was committed to the PR branch. |
| .hasParent(trace.getSpan(0)) | ||
| .hasStatus(StatusData.error()) | ||
| .hasAttributesSatisfyingExactly( | ||
| .hasAttributesSatisfying( |
There was a problem hiding this comment.
relaxing here was needed because this is shared by both Java agent tests and library tests
and I decided to only test peer.service in Java agent tests (though we could revisit that decision)
| .hasKind(CLIENT) | ||
| .hasParent(trace.getSpan(0)) | ||
| .hasAttributesSatisfyingExactly( | ||
| .hasAttributesSatisfying( |
| }) | ||
| .satisfies( | ||
| spanData -> { | ||
| // Check for peer.service when running with javaagent instrumentation | ||
| String distroName = | ||
| spanData | ||
| .getResource() | ||
| .getAttribute(TelemetryIncubatingAttributes.TELEMETRY_DISTRO_NAME); | ||
| if ("opentelemetry-java-instrumentation".equals(distroName)) { | ||
| String expectedPeerService = options.getExpectedPeerServiceName().apply(uri); | ||
| if (expectedPeerService != null) { | ||
| assertThat(spanData.getAttributes()) | ||
| .containsEntry(PeerIncubatingAttributes.PEER_SERVICE, expectedPeerService); | ||
| } | ||
| } |
There was a problem hiding this comment.
I didn't want to deal with configuring all of the library http client telemetry builders, but now that the javaagent ones are working, I don't mind revisiting
(in general I don't think peer.service infra is as important for library instrumentation because it's generally easy to add a peer.service constant identifier when you add instrumentation around a particular service call in that case)
| Builder setTestPeerService(boolean value); | ||
|
|
||
| Builder setExpectedPeerServiceName(Function<URI, String> value); | ||
|
|
There was a problem hiding this comment.
not really used with current hacky implementation, but maybe they should be
1 participant
Resolves #10361