Skip to content

Commit

Permalink
#3439 - Handle removal of ClientResponse#rawStatusCode and bump to Sp…
Browse files Browse the repository at this point in the history
…ring Boot to 3.2 (#3440)


Co-authored-by: Jonas Kunz <[email protected]>
  • Loading branch information
NicklasWallgren and JonasKunz authored Nov 29, 2023
1 parent 445685e commit c5a8373
Show file tree
Hide file tree
Showing 21 changed files with 311 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
===== Features
* Added support for OpenTelemetry annotations - `WithSpan` and `SpanAttribute` - {pull}3406[#3406]
* Only automatically apply redacted exceptions for Corretto JVM 17-20. Outside that, user should use capture_exception_details=false to workaround the JVM race-condition bug if it gets triggered: {pull}3438[#3438]
* Added support for Spring 6.1 / Spring-Boot 3.2 - {pull}3440[#3440]
[[release-notes-1.x]]
=== Java Agent version 1.x
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
<artifactId>apm-httpclient-core</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>apm-spring-webflux-common</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-web</artifactId>
Expand All @@ -60,6 +65,11 @@
<artifactId>spring-webflux</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-context</artifactId>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>${project.groupId}</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import co.elastic.apm.agent.tracer.Span;
import co.elastic.apm.agent.tracer.Tracer;
import co.elastic.apm.agent.tracer.reference.ReferenceCountedMap;
import co.elastic.apm.agent.webfluxcommon.SpringWebVersionUtils;
import org.reactivestreams.Subscription;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -74,14 +75,15 @@ public void onNext(T t) {
try {
if (span != null && t instanceof ClientResponse) {
ClientResponse clientResponse = (ClientResponse) t;
int statusCode = clientResponse.rawStatusCode();
int statusCode = SpringWebVersionUtils.getClientStatusCode(clientResponse);
span.withOutcome(ResultUtil.getOutcomeByHttpClientStatus(statusCode));
span.getContext().getHttp().withStatusCode(statusCode);
}
subscriber.onNext(t);
} catch (Throwable e) {
thrown = e;
throw e;
// Since spring 6.1 we can't throw checked exceptions here anymore, so we have to use this trick
throwException(e);
} finally {
doExit(hasActivated, "onNext", span);
discardIf(thrown != null);
Expand Down Expand Up @@ -193,4 +195,17 @@ private void cancelSpan() {
}
}

@SuppressWarnings("unchecked")
private static <T extends Throwable> void throwException(Throwable exception, Object dummy) throws T {
throw (T) exception;
}

/**
* Utility method for throwing a checked exception like an unchecked one.
*/
private static void throwException(Throwable exception) {
throwException(exception, null);
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.springwebclient;

import co.elastic.apm.agent.sdk.internal.PluginClassLoaderRootPackageCustomizer;

import java.util.Arrays;
import java.util.Collection;

public class WebclientPluginRootPackageCustomizer extends PluginClassLoaderRootPackageCustomizer {
@Override
public Collection<String> pluginClassLoaderRootPackages() {
return Arrays.asList(getPluginPackage(), "co.elastic.apm.agent.webfluxcommon");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
co.elastic.apm.agent.springwebclient.WebclientPluginRootPackageCustomizer
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>co.elastic.apm</groupId>
<artifactId>apm-spring-webflux</artifactId>
<version>1.44.1-SNAPSHOT</version>
</parent>

<artifactId>apm-spring-webflux-common-spring5</artifactId>
<name>${project.groupId}:${project.artifactId}</name>

<properties>
<!-- for licence header plugin -->
<apm-agent-parent.base.dir>${project.basedir}/../../..</apm-agent-parent.base.dir>
<animal.sniffer.skip>true</animal.sniffer.skip>
</properties>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-dependencies</artifactId>
<version>${version.spring-boot-2}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-webflux</artifactId>
<scope>provided</scope>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.springwebflux;
package co.elastic.apm.agent.webfluxcommon;

import org.springframework.http.HttpStatus;
import org.springframework.http.server.reactive.ServerHttpResponse;
import org.springframework.web.reactive.function.client.ClientResponse;

/**
* This class is compiled with spring-web 5.x, relying on the {@link ServerHttpResponse#getStatusCode()}, which changed in 6.0.0.
Expand All @@ -29,8 +30,13 @@
public class SpringWeb5Utils implements SpringWebVersionUtils.ISpringWebVersionUtils {

@Override
public int getStatusCode(Object response) {
public int getServerStatusCode(Object response) {
HttpStatus statusCode = ((ServerHttpResponse) response).getStatusCode();
return statusCode != null ? statusCode.value() : 200;
}

@Override
public int getClientStatusCode(Object response) {
return ((ClientResponse) response).rawStatusCode();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.springwebflux;
package co.elastic.apm.agent.webfluxcommon;

import org.springframework.web.reactive.function.server.ServerResponse;

import javax.annotation.Nullable;

Expand All @@ -27,8 +29,8 @@
*/
public class SpringWebVersionUtils {

private static final String SPRING_WEB_5_UTILS_CLASS_NAME = "co.elastic.apm.agent.springwebflux.SpringWeb5Utils";
private static final String SPRING_WEB_6_UTILS_CLASS_NAME = "co.elastic.apm.agent.springwebflux.SpringWeb6Utils";
private static final String SPRING_WEB_5_UTILS_CLASS_NAME = "co.elastic.apm.agent.webfluxcommon.SpringWeb5Utils";
private static final String SPRING_WEB_6_UTILS_CLASS_NAME = "co.elastic.apm.agent.webfluxcommon.SpringWeb6Utils";

@Nullable
private static ISpringWebVersionUtils instance = null;
Expand All @@ -40,21 +42,16 @@ private static synchronized void initialize() throws Exception {
return;
}
try {
// check if using spring 6.0.0 or higher
Class.forName("org.springframework.http.HttpStatusCode");
try {
// loading class by name so to avoid linkage attempt when spring-web 6 is unavailable
instance = (ISpringWebVersionUtils) Class.forName(SPRING_WEB_6_UTILS_CLASS_NAME).getDeclaredConstructor().newInstance();
} catch (Exception e) {
throw new IllegalStateException("Spring-web 6.x+ identified, but failed to load related utility class", e);
}
} catch (ClassNotFoundException ignored) {
// assuming spring-web < 6.x
try {
// loading class by name so to avoid linkage attempt on spring-web 6, where the getStatusCode API has changed
instance = (ISpringWebVersionUtils) Class.forName(SPRING_WEB_5_UTILS_CLASS_NAME).getDeclaredConstructor().newInstance();
} catch (Exception e) {
throw new IllegalStateException("Spring-web < 6.x identified, but failed to load related utility class", e);
Class<?> statusCodeType = ServerResponse.class.getMethod("statusCode").getReturnType();
switch (statusCodeType.getName()) {
case "org.springframework.http.HttpStatusCode": // spring 6.0.0 or higher
instance = (ISpringWebVersionUtils) Class.forName(SPRING_WEB_6_UTILS_CLASS_NAME).getDeclaredConstructor().newInstance();
break;
case "org.springframework.http.HttpStatus": // spring < 6
instance = (ISpringWebVersionUtils) Class.forName(SPRING_WEB_5_UTILS_CLASS_NAME).getDeclaredConstructor().newInstance();
break;
default:
throw new IllegalStateException("Unexpected type: "+statusCodeType.getName());
}
} finally {
initialized = true;
Expand All @@ -77,10 +74,27 @@ private static ISpringWebVersionUtils getImplementation() throws Exception {
* expected
* @return the status code of the provided response
*/
public static int getStatusCode(Object response) throws Exception {
public static int getServerStatusCode(Object response) throws Exception {
ISpringWebVersionUtils implementation = getImplementation();
if (implementation != null) {
return implementation.getServerStatusCode(response);
}
return 200;
}


/**
* A utility method to get the status code of a {@code org.springframework.web.reactive.function.client.ClientResponse} from any version
* of Spring framework.
*
* @param response must be of type {@code org.springframework.web.reactive.function.client.ClientResponse}, otherwise an Exception is
* expected
* @return the status code of the provided response
*/
public static int getClientStatusCode(Object response) throws Exception {
ISpringWebVersionUtils implementation = getImplementation();
if (implementation != null) {
return implementation.getStatusCode(response);
return implementation.getClientStatusCode(response);
}
return 200;
}
Expand All @@ -93,6 +107,16 @@ public interface ISpringWebVersionUtils {
* @param response must be of type {@code org.springframework.http.server.reactive.ServerHttpResponse}
* @return the corresponding status code
*/
int getStatusCode(Object response);
int getServerStatusCode(Object response);

/**
* A utility method to get the status code of a {@code org.springframework.web.reactive.function.client.ClientResponse} from any version
* of Spring framework.
*
* @param response must be of type {@code org.springframework.web.reactive.function.client.ClientResponse}
* @return the corresponding status code
*/
int getClientStatusCode(Object response);

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.springwebflux;
package co.elastic.apm.agent.webfluxcommon;

import org.junit.jupiter.api.Test;
import org.springframework.http.HttpStatus;
import org.springframework.http.server.reactive.ServerHttpResponse;
import org.springframework.web.reactive.function.client.ClientResponse;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
Expand All @@ -30,14 +31,21 @@
class SpringWeb5UtilsTest {

@Test
void testGetStatusCode() throws Exception {
void testGetServerStatusCode() throws Exception {
ServerHttpResponse mockResponse = mock(ServerHttpResponse.class);
doReturn(HttpStatus.IM_USED).when(mockResponse).getStatusCode();
assertThat(SpringWebVersionUtils.getStatusCode(mockResponse)).isEqualTo(226);
assertThat(SpringWebVersionUtils.getServerStatusCode(mockResponse)).isEqualTo(226);
}

@Test
void testWrongResponseType() {
assertThatThrownBy(() -> SpringWebVersionUtils.getStatusCode(new Object())).isInstanceOf(ClassCastException.class);
assertThatThrownBy(() -> SpringWebVersionUtils.getServerStatusCode(new Object())).isInstanceOf(ClassCastException.class);
}

@Test
void testGetClientStatusCode() throws Exception {
ClientResponse mockResponse = mock(ClientResponse.class);
doReturn(226).when(mockResponse).rawStatusCode();
assertThat(SpringWebVersionUtils.getClientStatusCode(mockResponse)).isEqualTo(226);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>co.elastic.apm</groupId>
<artifactId>apm-spring-webflux</artifactId>
<version>1.44.1-SNAPSHOT</version>
</parent>

<artifactId>apm-spring-webflux-common</artifactId>
<name>${project.groupId}:${project.artifactId}</name>

<properties>
<!-- for licence header plugin -->
<apm-agent-parent.base.dir>${project.basedir}/../../..</apm-agent-parent.base.dir>
<animal.sniffer.skip>true</animal.sniffer.skip>
</properties>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-dependencies</artifactId>
<version>${version.spring-boot-3}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>apm-spring-webflux-common-spring5</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-webflux</artifactId>
<scope>provided</scope>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
package co.elastic.apm.agent.springwebflux;
package co.elastic.apm.agent.webfluxcommon;


import org.springframework.http.HttpStatusCode;
import org.springframework.http.server.reactive.ServerHttpResponse;
import org.springframework.web.reactive.function.client.ClientResponse;

/**
* This class is compiled with spring-web 6.x, as it relies on {@link HttpStatusCode} and an API that was introduced in 6.0.0.
Expand All @@ -28,8 +30,14 @@
@SuppressWarnings("unused") //Created via reflection
public class SpringWeb6Utils implements SpringWebVersionUtils.ISpringWebVersionUtils {
@Override
public int getStatusCode(Object response) {
public int getServerStatusCode(Object response) {
HttpStatusCode statusCode = ((ServerHttpResponse) response).getStatusCode();
return statusCode != null ? statusCode.value() : 200;
}

@Override
public int getClientStatusCode(Object response) {
HttpStatusCode statusCode = ((ClientResponse) response).statusCode();
return statusCode.value();
}
}
Loading

0 comments on commit c5a8373

Please sign in to comment.