From bf2912912032489774a735e4711aeebeb542c290 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Wed, 18 Dec 2024 17:39:38 +0800 Subject: [PATCH 1/5] add test client error --- .../junit/http/AbstractHttpClientTest.java | 20 +++++++++++++++++++ .../junit/http/HttpClientTestOptions.java | 5 +++++ 2 files changed, 25 insertions(+) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java index 73db5eac8c02..3879a4660a95 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java @@ -1029,6 +1029,26 @@ void spanEndsAfterHeadersReceived() throws Exception { }); } + @Test + void requestClientError() throws Exception { + assumeTrue(options.getTestClientError()); + + URI uri = resolveAddress("/client-error"); + String method = "GET"; + int responseCode = doRequest(method, uri); + + assertThat(responseCode).isEqualTo(400); + + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> + assertClientSpan(span, uri, method, 400, null) + .hasNoParent() + .hasStatus(StatusData.error()), + span -> assertServerSpan(span).hasParent(trace.getSpan(0)))); + } + // Visible for spock bridge. SpanDataAssert assertClientSpan( SpanDataAssert span, diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java index 4500176bbf0c..edce2d3c9ee5 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java @@ -89,6 +89,8 @@ public boolean isLowLevelInstrumentation() { public abstract boolean getTestNonStandardHttpMethod(); + public abstract boolean getTestClientError(); + public abstract Function getHttpProtocolVersion(); @Nullable @@ -131,6 +133,7 @@ default Builder withDefaults() { .setTestCallbackWithParent(true) .setTestErrorWithCallback(true) .setTestNonStandardHttpMethod(true) + .setTestClientError(false) .setHttpProtocolVersion(uri -> "1.1"); } @@ -174,6 +177,8 @@ default Builder withDefaults() { Builder setTestNonStandardHttpMethod(boolean value); + Builder setTestClientError(boolean value); + Builder setHttpProtocolVersion(Function value); @CanIgnoreReturnValue From b1f274db9f96902ad172e5bb4a4c258127d3b418 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Wed, 18 Dec 2024 19:34:54 +0800 Subject: [PATCH 2/5] add test error --- .../testing/junit/http/HttpClientTestOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java index edce2d3c9ee5..a33d4d3e4a52 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java @@ -133,7 +133,7 @@ default Builder withDefaults() { .setTestCallbackWithParent(true) .setTestErrorWithCallback(true) .setTestNonStandardHttpMethod(true) - .setTestClientError(false) + .setTestClientError(true) .setHttpProtocolVersion(uri -> "1.1"); } From a1fb938f468ba8757d555a6309c63ce910f67cd9 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Thu, 26 Dec 2024 20:34:40 +0800 Subject: [PATCH 3/5] fix ut --- .../junit/http/AbstractHttpClientTest.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java index 3879a4660a95..1e9e9eefb4ea 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java @@ -1035,18 +1035,26 @@ void requestClientError() throws Exception { URI uri = resolveAddress("/client-error"); String method = "GET"; - int responseCode = doRequest(method, uri); - assertThat(responseCode).isEqualTo(400); + testing.runWithSpan( + "parent", + () -> { + try { + doRequest(method, uri); + } catch (Throwable ignored) { + // ignored + } + }); testing.waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), span -> assertClientSpan(span, uri, method, 400, null) - .hasNoParent() + .hasParent(trace.getSpan(0)) .hasStatus(StatusData.error()), - span -> assertServerSpan(span).hasParent(trace.getSpan(0)))); + span -> assertServerSpan(span).hasParent(trace.getSpan(1)))); } // Visible for spock bridge. From 400f3f55fe90729492ce3a3b2bad12978f3e277e Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Tue, 31 Dec 2024 18:08:20 +0800 Subject: [PATCH 4/5] add parameter test --- .../junit/http/AbstractHttpClientTest.java | 34 +++---------------- .../junit/http/HttpClientTestOptions.java | 5 --- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java index 1e9e9eefb4ea..224c3097985c 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java @@ -55,6 +55,7 @@ import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -437,8 +438,9 @@ void redirectToSecuredCopiesAuthHeader() throws Exception { // TODO: add basic auth scenario - @Test - void errorSpan() { + @ParameterizedTest + @CsvSource({"/error,500", "/client-error,400"}) + void errorSpan(String path, int resposeCode) { String method = "GET"; URI uri = resolveAddress("/error"); @@ -1029,34 +1031,6 @@ void spanEndsAfterHeadersReceived() throws Exception { }); } - @Test - void requestClientError() throws Exception { - assumeTrue(options.getTestClientError()); - - URI uri = resolveAddress("/client-error"); - String method = "GET"; - - testing.runWithSpan( - "parent", - () -> { - try { - doRequest(method, uri); - } catch (Throwable ignored) { - // ignored - } - }); - - testing.waitAndAssertTraces( - trace -> - trace.hasSpansSatisfyingExactly( - span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), - span -> - assertClientSpan(span, uri, method, 400, null) - .hasParent(trace.getSpan(0)) - .hasStatus(StatusData.error()), - span -> assertServerSpan(span).hasParent(trace.getSpan(1)))); - } - // Visible for spock bridge. SpanDataAssert assertClientSpan( SpanDataAssert span, diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java index a33d4d3e4a52..4500176bbf0c 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpClientTestOptions.java @@ -89,8 +89,6 @@ public boolean isLowLevelInstrumentation() { public abstract boolean getTestNonStandardHttpMethod(); - public abstract boolean getTestClientError(); - public abstract Function getHttpProtocolVersion(); @Nullable @@ -133,7 +131,6 @@ default Builder withDefaults() { .setTestCallbackWithParent(true) .setTestErrorWithCallback(true) .setTestNonStandardHttpMethod(true) - .setTestClientError(true) .setHttpProtocolVersion(uri -> "1.1"); } @@ -177,8 +174,6 @@ default Builder withDefaults() { Builder setTestNonStandardHttpMethod(boolean value); - Builder setTestClientError(boolean value); - Builder setHttpProtocolVersion(Function value); @CanIgnoreReturnValue From b4c3dbeb0f4c077159b13ca7aff78362de66c248 Mon Sep 17 00:00:00 2001 From: "shalk(xiao kun)" Date: Tue, 31 Dec 2024 18:11:06 +0800 Subject: [PATCH 5/5] fixit --- .../testing/junit/http/AbstractHttpClientTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java index 224c3097985c..63d113ea40e1 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java @@ -440,9 +440,9 @@ void redirectToSecuredCopiesAuthHeader() throws Exception { @ParameterizedTest @CsvSource({"/error,500", "/client-error,400"}) - void errorSpan(String path, int resposeCode) { + void errorSpan(String path, int responseCode) { String method = "GET"; - URI uri = resolveAddress("/error"); + URI uri = resolveAddress(path); testing.runWithSpan( "parent", @@ -458,7 +458,9 @@ void errorSpan(String path, int resposeCode) { trace -> { trace.hasSpansSatisfyingExactly( span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), - span -> assertClientSpan(span, uri, method, 500, null).hasParent(trace.getSpan(0)), + span -> + assertClientSpan(span, uri, method, responseCode, null) + .hasParent(trace.getSpan(0)), span -> assertServerSpan(span).hasParent(trace.getSpan(1))); }); }