Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate incorrect CI failure reporting #3952

Merged
merged 10 commits into from
Jan 27, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@

import co.elastic.apm.agent.AbstractInstrumentationTest;
import co.elastic.apm.agent.configuration.SpanConfiguration;
import co.elastic.apm.agent.impl.transaction.SpanImpl;
import co.elastic.apm.agent.sdk.internal.db.signature.SignatureParser;
import co.elastic.apm.agent.impl.context.DbImpl;
import co.elastic.apm.agent.impl.context.DestinationImpl;
import co.elastic.apm.agent.tracer.Outcome;
import co.elastic.apm.agent.impl.transaction.SpanImpl;
import co.elastic.apm.agent.impl.transaction.TransactionImpl;
import co.elastic.apm.agent.jdbc.helper.JdbcGlobalState;
import co.elastic.apm.agent.sdk.internal.db.signature.SignatureParser;
import co.elastic.apm.agent.tracer.Outcome;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -441,8 +441,12 @@ private SpanImpl assertSpanRecorded(String rawSql, boolean preparedStatement, lo
DbImpl db = span.getContext().getDb();
assertThat(db).hasStatement(rawSql);
DatabaseMetaData metaData = connection.getMetaData();

assertThat(db.getUser()).isEqualToIgnoringCase(metaData.getUserName());
String expectedUserName = metaData.getUserName();
if (expectedUserName == null || expectedUserName.isEmpty()) {
assertThat(db.getUser()).isNullOrEmpty();
} else {
assertThat(db.getUser()).isEqualToIgnoringCase(expectedUserName);
}
assertThat(db.getType()).isEqualToIgnoringCase("sql");

assertThat(db.getAffectedRowsCount())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public static Iterable<Object[]> data() {
{"jdbc:tc:postgresql:9://hostname/databasename", "postgresql", "databasename", true},
{"jdbc:tc:postgresql:10://hostname/databasename", "postgresql", "databasename", true},
{"jdbc:tc:mariadb:10://hostname/databasename", "mariadb", "databasename", true},
{"jdbc:tc:sqlserver:2017-CU12://hostname/databasename", "mssql", "master", false}, // for mssql the 'master' name comes from the runtime catalog fallback
// TODO: SQL Server image seems to be broken with recent kernel versions: https://github.com/microsoft/mssql-docker/issues/868
//{"jdbc:tc:sqlserver:2017-CU12://hostname/databasename", "mssql", "master", false}, // for mssql the 'master' name comes from the runtime catalog fallback
{"jdbc:tc:db2:11.5.0.0a://hostname/databasename", "db2", "test", true},
{"jdbc:tc:oracle://hostname/databasename", "oracle", "xepdb1", true},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ protected boolean isBodyCapturingSupported() {
return Java17Code.isBodyCapturingSupported(restTemplate);
}

@Override
public boolean isTestHttpCallWithUserInfoEnabled() {
return Java17Code.isTestHttpCallWithUserInfoEnabled(restTemplate);
}

@Override
protected void performPost(String path, byte[] content, String contentTypeHeader) throws Exception {
Java17Code.performPost(restTemplate, path, content, contentTypeHeader);
Expand Down Expand Up @@ -106,5 +111,15 @@ public static boolean isBodyCapturingSupported(Object restTemplateObj) {
return true;
}

public static boolean isTestHttpCallWithUserInfoEnabled(Object restTemplateObj) {
RestTemplate restTemplate = (RestTemplate) restTemplateObj;
if (restTemplate.getRequestFactory() instanceof HttpComponentsClientHttpRequestFactory) {
// newer http components don't support userinfo in URI anymore:
// I/O error on GET request for "http://user:passwd@localhost:50931/": Request URI authority contains deprecated userinfo component
return false;
}
return true;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,30 +39,33 @@ public class WebClientInstrumentationTest extends AbstractHttpClientInstrumentat

private final RequestStrategy strategy;

private final boolean isNetty;
private final boolean circularRedirectSupported;

public WebClientInstrumentationTest(String clientIgnored, Object webClient, RequestStrategy strategy, boolean isNetty) {
private final boolean uriUserInfoSupported;

public WebClientInstrumentationTest(String clientName, Object webClient, RequestStrategy strategy) {
this.webClient = webClient;
this.strategy = strategy;
this.isNetty = isNetty;
this.circularRedirectSupported = !"netty".equals(clientName);
this.uriUserInfoSupported = !"hc5".equals(clientName) && !"netty".equals(clientName);
}

@Parameterized.Parameters(name = "client = {0}, request strategy = {2}")
public static Object[][] testParams() {
if (JvmRuntimeInfo.ofCurrentVM().getMajorVersion() >= 17) {
return new Object[][]{
{"jetty", Clients.jettyClient(), RequestStrategy.EXCHANGE, false},
{"jetty", Clients.jettyClient(), RequestStrategy.EXCHANGE_TO_FLUX, false},
{"jetty", Clients.jettyClient(), RequestStrategy.EXCHANGE_TO_MONO, false},
{"jetty", Clients.jettyClient(), RequestStrategy.RETRIEVE, false},
{"netty", Clients.nettyClient(), RequestStrategy.EXCHANGE, true},
{"netty", Clients.nettyClient(), RequestStrategy.EXCHANGE_TO_FLUX, true},
{"netty", Clients.nettyClient(), RequestStrategy.EXCHANGE_TO_MONO, true},
{"netty", Clients.nettyClient(), RequestStrategy.RETRIEVE, true},
{"hc5", Clients.reactiveHttpClient5(), RequestStrategy.EXCHANGE, false},
{"hc5", Clients.reactiveHttpClient5(), RequestStrategy.EXCHANGE_TO_FLUX, false},
{"hc5", Clients.reactiveHttpClient5(), RequestStrategy.EXCHANGE_TO_MONO, false},
{"hc5", Clients.reactiveHttpClient5(), RequestStrategy.RETRIEVE, false}
{"jetty", Clients.jettyClient(), RequestStrategy.EXCHANGE},
{"jetty", Clients.jettyClient(), RequestStrategy.EXCHANGE_TO_FLUX},
{"jetty", Clients.jettyClient(), RequestStrategy.EXCHANGE_TO_MONO},
{"jetty", Clients.jettyClient(), RequestStrategy.RETRIEVE},
{"netty", Clients.nettyClient(), RequestStrategy.EXCHANGE},
{"netty", Clients.nettyClient(), RequestStrategy.EXCHANGE_TO_FLUX},
{"netty", Clients.nettyClient(), RequestStrategy.EXCHANGE_TO_MONO},
{"netty", Clients.nettyClient(), RequestStrategy.RETRIEVE},
{"hc5", Clients.reactiveHttpClient5(), RequestStrategy.EXCHANGE},
{"hc5", Clients.reactiveHttpClient5(), RequestStrategy.EXCHANGE_TO_FLUX},
{"hc5", Clients.reactiveHttpClient5(), RequestStrategy.EXCHANGE_TO_MONO},
{"hc5", Clients.reactiveHttpClient5(), RequestStrategy.RETRIEVE}
};
} else {
return new Object[0][0];
Expand All @@ -72,13 +75,13 @@ public static Object[][] testParams() {
@Override
public boolean isRequireCheckErrorWhenCircularRedirect() {
// circular redirect does not trigger an error to capture with netty
return !isNetty;
return circularRedirectSupported;
}

@Override
public boolean isTestHttpCallWithUserInfoEnabled() {
// user info URI does not work with netty
return !isNetty;
return uriUserInfoSupported;
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
package co.elastic.apm.servlet;

import co.elastic.apm.agent.test.AgentTestContainer;
import co.elastic.apm.servlet.tests.JakartaExternalPluginTestApp;
import co.elastic.apm.servlet.tests.JakartaeeServletApiTestApp;
import co.elastic.apm.servlet.tests.TestApp;
Expand All @@ -38,8 +37,9 @@ public JakartaeeTomcatWithSecurityManagerIT(final String tomcatVersion) {
@Parameterized.Parameters(name = "Tomcat {0}")
public static Iterable<Object[]> data() {
return Arrays.asList(new Object[][]{
{"10.0.10-jdk11-adoptopenjdk-hotspot"},
{"10.1.0-jdk17-temurin"}, // Servlet 6.x
{"10.0.10-jdk11-adoptopenjdk-hotspot"}
// TODO: investigate why on 10.1.0-jdk17-temurin it returns 500 for the status page
//{"10.1.0-jdk17-temurin"}, // Servlet 6.x
});
}

Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -549,11 +549,11 @@
</plugin>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.0.0-M8</version>
<version>3.5.2</version>
</plugin>
<plugin>
<artifactId>maven-failsafe-plugin</artifactId>
<version>3.0.0-M8</version>
<version>3.5.2</version>
</plugin>
<plugin>
<artifactId>maven-jar-plugin</artifactId>
Expand Down
Loading