This is an automated email from the ASF dual-hosted git repository.
ffang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new 758f6a51c42 [CXF-9206]Duplicated value of 'cxf.server.requests' metric
for JAX RS Server (#3010)
758f6a51c42 is described below
commit 758f6a51c4257353c3d2f3236aa718516cdeda82
Author: Freeman(Yue) Fang <[email protected]>
AuthorDate: Thu Apr 2 18:29:15 2026 -0400
[CXF-9206]Duplicated value of 'cxf.server.requests' metric for JAX RS
Server (#3010)
[CXF-9206]avoid creating Endpoint MetricContext for JAXRS endpoint
---
.../interceptors/AbstractMetricsInterceptor.java | 15 ++
systests/jaxrs/pom.xml | 5 +
.../jaxrs/JaxRsMetricsClientServerTest.java | 173 +++++++++++++++++++++
.../jaxrs/metrics/JAXRSClientMetricsTest.java | 17 +-
.../jaxrs/metrics/JAXRSServerMetricsTest.java | 19 +--
5 files changed, 198 insertions(+), 31 deletions(-)
diff --git
a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/interceptors/AbstractMetricsInterceptor.java
b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/interceptors/AbstractMetricsInterceptor.java
index 5e3cd95cb1a..80a190b0c00 100644
---
a/rt/features/metrics/src/main/java/org/apache/cxf/metrics/interceptors/AbstractMetricsInterceptor.java
+++
b/rt/features/metrics/src/main/java/org/apache/cxf/metrics/interceptors/AbstractMetricsInterceptor.java
@@ -89,9 +89,22 @@ public abstract class AbstractMetricsInterceptor extends
AbstractPhaseIntercepto
ctx.addContext((MetricsContext)o);
}
}
+
+ private boolean isJaxRsEndpoint(Endpoint endpoint) {
+ if (endpoint == null || endpoint.getEndpointInfo() == null
+ || endpoint.getEndpointInfo().getBinding() == null) {
+ return false;
+ }
+ return "http://apache.org/cxf/binding/jaxrs";
+ .equals(endpoint.getEndpointInfo().getBinding().getBindingId());
+ }
private Object createEndpointMetrics(Message m) {
final Endpoint ep = m.getExchange().getEndpoint();
+ if (isJaxRsEndpoint(ep)) {
+ //shouldn't create endpoint context for JAX-RS endpoints
+ return null;
+ }
Object o = ep.get(MetricsContext.class.getName());
if (o == null) {
List<MetricsContext> contexts = new ArrayList<>();
@@ -163,6 +176,8 @@ public abstract class AbstractMetricsInterceptor extends
AbstractPhaseIntercepto
if (o != null) {
return o;
}
+
+
List<MetricsContext> contexts = new ArrayList<>();
for (MetricsProvider p :
getMetricProviders(message.getExchange().getBus())) {
MetricsContext c =
p.createResourceContext(message.getExchange().getEndpoint(),
diff --git a/systests/jaxrs/pom.xml b/systests/jaxrs/pom.xml
index c473acdd363..a402e9ff08a 100644
--- a/systests/jaxrs/pom.xml
+++ b/systests/jaxrs/pom.xml
@@ -539,6 +539,11 @@
<version>${cxf.netty.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>io.micrometer</groupId>
+ <artifactId>micrometer-core</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
<plugins>
diff --git
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JaxRsMetricsClientServerTest.java
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JaxRsMetricsClientServerTest.java
new file mode 100644
index 00000000000..a7b3c233c9c
--- /dev/null
+++
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JaxRsMetricsClientServerTest.java
@@ -0,0 +1,173 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF 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 org.apache.cxf.systest.jaxrs;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.MediaType;
+import org.apache.cxf.jaxrs.JAXRSServerFactoryBean;
+import org.apache.cxf.jaxrs.client.JAXRSClientFactoryBean;
+import org.apache.cxf.jaxrs.lifecycle.SingletonResourceProvider;
+import org.apache.cxf.metrics.MetricsFeature;
+import org.apache.cxf.metrics.micrometer.MicrometerMetricsProperties;
+import org.apache.cxf.metrics.micrometer.MicrometerMetricsProvider;
+import
org.apache.cxf.metrics.micrometer.provider.DefaultExceptionClassProvider;
+import
org.apache.cxf.metrics.micrometer.provider.DefaultTimedAnnotationProvider;
+import org.apache.cxf.metrics.micrometer.provider.StandardTags;
+import org.apache.cxf.metrics.micrometer.provider.StandardTagsProvider;
+import
org.apache.cxf.metrics.micrometer.provider.jaxrs.JaxrsOperationTagsCustomizer;
+import org.apache.cxf.metrics.micrometer.provider.jaxrs.JaxrsTags;
+import org.apache.cxf.testutil.common.AbstractBusClientServerTestBase;
+import org.apache.cxf.testutil.common.AbstractBusTestServerBase;
+
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Timer;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test for CXF-9206: cxf.server.requests metric must not be
+ * counted twice for JAX-RS server endpoints when MetricsFeature is used.
+ *
+ * Root cause: the CXF-9168 fix (de39a25) made createEndpointContext() eagerly
+ * return a MicrometerServerMetricsContext for all server-side endpoints so
that
+ * requests failing before a BindingOperationInfo is available are still
counted.
+ * For JAX-RS, createResourceContext() already returns its own MetricsContext
per
+ * request, so ExchangeMetrics ended up incrementing cxf.server.requests twice.
+ *
+ * The fix skips creating a server context in createEndpointContext() when the
+ * endpoint uses the JAX-RS binding (JAXRSBindingFactory.JAXRS_BINDING_ID).
+ */
+public class JaxRsMetricsClientServerTest extends
AbstractBusClientServerTestBase {
+
+ private static final MeterRegistry METER_REGISTRY = new
SimpleMeterRegistry();
+ private static final String PORT =
allocatePort(JaxRsMetricsClientServerTest.class);
+
+
+ @Path("/hello")
+ public interface HelloService {
+ @GET
+ @Path("/{name}")
+ @Produces(MediaType.TEXT_PLAIN)
+ String sayHello(@PathParam("name") String name);
+ }
+
+ public static class HelloServiceImpl implements HelloService {
+ @Override
+ public String sayHello(String name) {
+ return "Hello " + name;
+ }
+ }
+
+
+ public static class Server extends AbstractBusTestServerBase {
+ protected void run() {
+ var jaxrsTags = new JaxrsTags();
+ var operationsCustomizer = new
JaxrsOperationTagsCustomizer(jaxrsTags);
+ var tagsProvider = new StandardTagsProvider(
+ new DefaultExceptionClassProvider(), new StandardTags());
+ var properties = new MicrometerMetricsProperties();
+
+ var provider = new MicrometerMetricsProvider(
+ METER_REGISTRY,
+ tagsProvider,
+ List.of(operationsCustomizer),
+ new DefaultTimedAnnotationProvider(),
+ properties
+ );
+
+ JAXRSServerFactoryBean sf = new JAXRSServerFactoryBean();
+ sf.setResourceClasses(HelloServiceImpl.class);
+ sf.setResourceProvider(HelloServiceImpl.class,
+ new SingletonResourceProvider(new HelloServiceImpl()));
+ sf.setAddress("http://localhost:"; + PORT + "/");
+ sf.setFeatures(Arrays.asList(new MetricsFeature(provider)));
+ sf.create();
+ }
+ }
+
+
+ @BeforeClass
+ public static void startServers() throws Exception {
+ createStaticBus();
+ assertTrue("server did not launch correctly",
launchServer(Server.class, true));
+ }
+
+
+ private HelloService createClient() {
+ JAXRSClientFactoryBean factory = new JAXRSClientFactoryBean();
+ factory.setServiceClass(HelloService.class);
+ factory.setAddress("http://localhost:"; + PORT + "/");
+ factory.setHeaders(Collections.singletonMap("Accept", "text/plain"));
+ return (HelloService) factory.create();
+ }
+
+
+ /**
+ * CXF-9206: a single successful JAX-RS request must produce exactly one
+ * sample in cxf.server.requests (count == 1), not two.
+ */
+ @Test
+ public void testSuccessfulRequestIsCountedOnce() {
+ METER_REGISTRY.clear();
+
+ String response = createClient().sayHello("world");
+ assertEquals("Hello world", response);
+
+ Timer timer = METER_REGISTRY.find("cxf.server.requests").timer();
+ assertNotNull("Expected cxf.server.requests timer to be registered",
timer);
+ assertEquals(
+ "cxf.server.requests count should be 1 for a single request",
+ 1,
+ timer.count()
+ );
+ }
+
+ /**
+ * Two successive requests must accumulate to count == 2, not 4.
+ */
+ @Test
+ public void testTwoRequestsAreCountedTwice() {
+ METER_REGISTRY.clear();
+
+ HelloService client = createClient();
+ client.sayHello("alice");
+ client.sayHello("bob");
+
+ Timer timer = METER_REGISTRY.find("cxf.server.requests").timer();
+ assertNotNull("Expected cxf.server.requests timer to be registered",
timer);
+ assertEquals(
+ "cxf.server.requests count should be 2 for two requests",
+ 2,
+ timer.count()
+ );
+ }
+}
diff --git
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/metrics/JAXRSClientMetricsTest.java
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/metrics/JAXRSClientMetricsTest.java
index 8cd75213919..903589ec550 100644
---
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/metrics/JAXRSClientMetricsTest.java
+++
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/metrics/JAXRSClientMetricsTest.java
@@ -65,17 +65,15 @@ public class JAXRSClientMetricsTest {
private MetricsProvider provider;
private MetricsContext operationContext;
private MetricsContext resourceContext;
- private MetricsContext endpointContext;
-
+
@Before
public void setUp() {
- endpointContext = Mockito.mock(MetricsContext.class);
operationContext = Mockito.mock(MetricsContext.class);
resourceContext = Mockito.mock(MetricsContext.class);
provider = new MetricsProvider() {
public MetricsContext createEndpointContext(Endpoint endpoint,
boolean asClient, String cid) {
- return endpointContext;
+ return null;
}
public MetricsContext createOperationContext(Endpoint endpoint,
BindingOperationInfo boi,
@@ -109,8 +107,6 @@ public class JAXRSClientMetricsTest {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(operationContext);
}
}
@@ -135,8 +131,6 @@ public class JAXRSClientMetricsTest {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(operationContext);
}
}
@@ -160,8 +154,6 @@ public class JAXRSClientMetricsTest {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(operationContext);
}
}
@@ -189,8 +181,6 @@ public class JAXRSClientMetricsTest {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(operationContext);
}
}
@@ -210,9 +200,6 @@ public class JAXRSClientMetricsTest {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verifyNoInteractions(operationContext);
}
}
}
diff --git
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/metrics/JAXRSServerMetricsTest.java
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/metrics/JAXRSServerMetricsTest.java
index e11c8da5ddd..b0db62c138a 100644
---
a/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/metrics/JAXRSServerMetricsTest.java
+++
b/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/metrics/JAXRSServerMetricsTest.java
@@ -63,8 +63,7 @@ public class JAXRSServerMetricsTest extends
AbstractClientServerTestBase {
private static MetricsProvider provider;
private static MetricsContext operationContext;
private static MetricsContext resourceContext;
- private static MetricsContext endpointContext;
-
+
@Rule public ExpectedException expectedException =
ExpectedException.none();
public static class BookLibrary implements Library {
@@ -97,18 +96,17 @@ public class JAXRSServerMetricsTest extends
AbstractClientServerTestBase {
@BeforeClass
public static void startServers() throws Exception {
- endpointContext = Mockito.mock(MetricsContext.class);
operationContext = Mockito.mock(MetricsContext.class);
resourceContext = Mockito.mock(MetricsContext.class);
provider = new MetricsProvider() {
public MetricsContext createEndpointContext(Endpoint endpoint,
boolean asClient, String cid) {
- return endpointContext;
+ return null;
}
public MetricsContext createOperationContext(Endpoint endpoint,
BindingOperationInfo boi,
boolean asClient, String cid) {
- return operationContext;
+ return null;
}
public MetricsContext createResourceContext(Endpoint endpoint,
String resourceName,
@@ -126,7 +124,6 @@ public class JAXRSServerMetricsTest extends
AbstractClientServerTestBase {
public void setUp() {
Mockito.reset(resourceContext);
Mockito.reset(operationContext);
- Mockito.reset(endpointContext);
}
@Test
@@ -143,8 +140,6 @@ public class JAXRSServerMetricsTest extends
AbstractClientServerTestBase {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(operationContext);
}
}
@@ -164,8 +159,6 @@ public class JAXRSServerMetricsTest extends
AbstractClientServerTestBase {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(operationContext);
}
}
@@ -185,8 +178,6 @@ public class JAXRSServerMetricsTest extends
AbstractClientServerTestBase {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(operationContext);
}
}
@@ -202,8 +193,6 @@ public class JAXRSServerMetricsTest extends
AbstractClientServerTestBase {
} finally {
Mockito.verify(resourceContext,
times(1)).start(any(Exchange.class));
Mockito.verify(resourceContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(operationContext);
}
}
@@ -217,8 +206,6 @@ public class JAXRSServerMetricsTest extends
AbstractClientServerTestBase {
expectedException.expect(ProcessingException.class);
client.get().readEntity(Book.class);
} finally {
- Mockito.verify(endpointContext,
times(1)).start(any(Exchange.class));
- Mockito.verify(endpointContext, times(1)).stop(anyLong(),
anyLong(), anyLong(), any(Exchange.class));
Mockito.verifyNoInteractions(resourceContext);
Mockito.verifyNoInteractions(operationContext);
}
