Skip to content

Commit

Permalink
Investigate incorrect CI failure reporting (#3952)
Browse files Browse the repository at this point in the history
  • Loading branch information
@JonasKunz
JonasKunz authored Jan 27, 2025
1 parent 6cc2a84 commit 4c59145
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 28 deletions.
14 changes: 9 additions & 5 deletions ...-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java
View file
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
3 changes: 2 additions & 1 deletion apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcDbIT.java
View file
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
15 changes: 15 additions & 0 deletions ...rc/test/java/co/elastic/apm/agent/resttemplate/SpringRestTemplateInstrumentationTest.java
View file
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;
}

}
}
37 changes: 20 additions & 17 deletions ...ugin/src/test/java/co/elastic/apm/agent/springwebclient/WebClientInstrumentationTest.java
View file
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
6 changes: 3 additions & 3 deletions ...tion-tests/src/test/java/co/elastic/apm/servlet/JakartaeeTomcatWithSecurityManagerIT.java
View file
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
View file
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

0 comments on commit 4c59145

Please sign in to comment.