From 3b5aa7aac69c5866f7beef4d6635c83068ed98e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Wed, 26 Feb 2025 09:57:10 +0100 Subject: [PATCH 1/9] Add endpoint discover for spring mvc --- ...ppSecDispatcherServletInstrumentation.java | 78 +++++++++ .../springweb/RequestMappingInfoIterator.java | 95 +++++++++++ .../test/boot/SpringBootBasedTest.groovy | 18 ++ .../groovy/test/boot/TestController.groovy | 11 ++ ...ervletWithPathPatternsInstrumentation.java | 78 +++++++++ ...stMappingInfoWithPathPatternsIterator.java | 112 +++++++++++++ .../EndpointCollectorSpringBootTest.groovy | 54 ++++++ .../boot/SpringBootBasedTest.groovy | 18 ++ .../springweb6/boot/TestController.groovy | 10 ++ .../agent/test/base/HttpServerTest.groovy | 29 ++++ .../datadog/trace/api/ConfigDefaults.java | 2 + .../trace/api/config/AppSecConfig.java | 5 + .../main/java/datadog/trace/api/Config.java | 23 +++ .../datadog/trace/api/telemetry/Endpoint.java | 155 ++++++++++++++++++ .../api/telemetry/EndpointCollector.java | 26 +++ .../datadog/telemetry/BufferedEvents.java | 21 +++ .../java/datadog/telemetry/EventSink.java | 6 + .../java/datadog/telemetry/EventSource.java | 23 ++- .../telemetry/ExtendedHeartbeatData.java | 11 ++ .../datadog/telemetry/TelemetryRequest.java | 19 +++ .../telemetry/TelemetryRequestBody.java | 36 ++++ .../datadog/telemetry/TelemetryService.java | 12 +- .../datadog/telemetry/TelemetrySystem.java | 4 + .../datadog/telemetry/api/RequestType.java | 3 +- .../endpoint/EndpointPeriodicAction.java | 19 +++ .../BufferedEventsSpecification.groovy | 14 ++ .../datadog/telemetry/EventSourceTest.groovy | 8 +- .../TelemetryServiceSpecification.groovy | 31 +++- .../telemetry/TestTelemetryRouter.groovy | 31 ++++ 29 files changed, 942 insertions(+), 10 deletions(-) create mode 100644 dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java create mode 100644 dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoIterator.java create mode 100644 dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletWithPathPatternsInstrumentation.java create mode 100644 dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoWithPathPatternsIterator.java create mode 100644 dd-java-agent/instrumentation/spring-webmvc-5.3/src/test/groovy/test/boot/EndpointCollectorSpringBootTest.groovy create mode 100644 internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java create mode 100644 internal-api/src/main/java/datadog/trace/api/telemetry/EndpointCollector.java create mode 100644 telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java new file mode 100644 index 00000000000..92adedccbc4 --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java @@ -0,0 +1,78 @@ +package datadog.trace.instrumentation.springweb; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isProtected; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.Config; +import datadog.trace.api.telemetry.EndpointCollector; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.matcher.ElementMatcher; +import org.springframework.context.ApplicationContext; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.servlet.mvc.method.RequestMappingInfo; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; + +@AutoService(InstrumenterModule.class) +public class AppSecDispatcherServletInstrumentation extends InstrumenterModule.AppSec + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public AppSecDispatcherServletInstrumentation() { + super("spring-web"); + } + + @Override + public String instrumentedType() { + return "org.springframework.web.servlet.DispatcherServlet"; + } + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + return not( + hasClassNamed( + "org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition")); + } + + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".RequestMappingInfoIterator"}; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isMethod() + .and(isProtected()) + .and(named("onRefresh")) + .and(takesArgument(0, named("org.springframework.context.ApplicationContext"))) + .and(takesArguments(1)), + AppSecDispatcherServletInstrumentation.class.getName() + "$AppSecHandlerMappingAdvice"); + } + + public static class AppSecHandlerMappingAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void afterRefresh(@Advice.Argument(0) final ApplicationContext springCtx) { + if (Config.get().isApiSecurityEndpointCollectionEnabled()) { + final RequestMappingHandlerMapping handler = + springCtx.getBean(RequestMappingHandlerMapping.class); + if (handler == null) { + return; + } + final Map mappings = handler.getHandlerMethods(); + if (mappings == null || mappings.isEmpty()) { + return; + } + EndpointCollector.get().supplier(new RequestMappingInfoIterator(mappings)); + } + } + } +} diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoIterator.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoIterator.java new file mode 100644 index 00000000000..aea4cbb6273 --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoIterator.java @@ -0,0 +1,95 @@ +package datadog.trace.instrumentation.springweb; + +import datadog.trace.api.telemetry.Endpoint; +import datadog.trace.api.telemetry.Endpoint.Method; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Queue; +import java.util.Set; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.servlet.mvc.condition.MediaTypeExpression; +import org.springframework.web.servlet.mvc.method.RequestMappingInfo; + +public class RequestMappingInfoIterator implements Iterator { + + private final Map mappings; + private final Queue queue = new LinkedList<>(); + private Iterator> iterator; + + public RequestMappingInfoIterator(final Map mappings) { + this.mappings = mappings; + } + + private Iterator> iterator() { + if (iterator == null) { + iterator = mappings.entrySet().iterator(); + } + return iterator; + } + + @Override + public boolean hasNext() { + return !queue.isEmpty() || iterator().hasNext(); + } + + @Override + public Endpoint next() { + if (queue.isEmpty()) { + fetchNext(); + } + final Endpoint endpoint = queue.poll(); + if (endpoint == null) { + throw new NoSuchElementException(); + } + return endpoint; + } + + private void fetchNext() { + final Iterator> delegate = iterator(); + if (!delegate.hasNext()) { + return; + } + final Map.Entry nextEntry = delegate.next(); + final RequestMappingInfo nextInfo = nextEntry.getKey(); + final HandlerMethod nextHandler = nextEntry.getValue(); + final List requestBody = + parseMediaTypes(nextInfo.getConsumesCondition().getExpressions()); + final List responseBody = + parseMediaTypes(nextInfo.getProducesCondition().getExpressions()); + for (final String path : nextInfo.getPatternsCondition().getPatterns()) { + final List methods = Method.parseMethods(nextInfo.getMethodsCondition().getMethods()); + for (final String method : methods) { + final Endpoint endpoint = + new Endpoint() + .type(Endpoint.Type.REST) + .operation(Endpoint.Operation.HTTP_REQUEST) + .path(path) + .method(method) + .requestBodyType(requestBody) + .responseBodyType(responseBody); + if (nextHandler != null) { + final Map metadata = new HashMap<>(); + metadata.put("handler", nextHandler.toString()); + endpoint.metadata(metadata); + } + queue.add(endpoint); + } + } + } + + private List parseMediaTypes(final Set expressions) { + if (expressions == null || expressions.isEmpty()) { + return null; + } + final List result = new ArrayList<>(expressions.size()); + for (final MediaTypeExpression expression : expressions) { + result.add(expression.toString()); + } + return result; + } +} diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy index 18a2fce745c..fc9ee36389c 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy @@ -9,6 +9,7 @@ import datadog.trace.api.iast.IastContext import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.SourceTypes import datadog.trace.api.iast.propagation.PropagationModule +import datadog.trace.api.telemetry.Endpoint import datadog.trace.bootstrap.instrumentation.api.Tags import datadog.trace.core.DDSpan import datadog.trace.instrumentation.springweb.SpringWebHttpServerDecorator @@ -19,6 +20,7 @@ import okhttp3.Response import org.springframework.boot.SpringApplication import org.springframework.boot.context.embedded.EmbeddedWebApplicationContext import org.springframework.context.ConfigurableApplicationContext +import org.springframework.http.MediaType import org.springframework.web.servlet.handler.HandlerInterceptorAdapter import org.springframework.web.servlet.view.RedirectView import test.SetupSpecHelper @@ -129,6 +131,22 @@ class SpringBootBasedTest extends HttpServerTest true } + @Override + boolean testEndpointDiscovery() { + true + } + + @Override + void assertEndpointDiscovery(final List endpoints) { + final discovered = endpoints.collectEntries { [(it.method): it] } as Map + assert discovered.keySet().containsAll([Endpoint.Method.POST, Endpoint.Method.PATCH, Endpoint.Method.PUT]) + discovered.values().each { + assert it.requestBodyType.containsAll([MediaType.APPLICATION_JSON_VALUE]) + assert it.responseBodyType.containsAll([MediaType.TEXT_PLAIN_VALUE]) + assert it.metadata['handler'] == 'public org.springframework.http.ResponseEntity test.boot.TestController.discovery()' + } + } + @Override Serializable expectedServerSpanRoute(ServerEndpoint endpoint) { switch (endpoint) { diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/TestController.groovy b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/TestController.groovy index 62b0ae0b1e1..7a505d8be63 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/TestController.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/TestController.groovy @@ -22,6 +22,7 @@ import javax.servlet.http.HttpServletRequest import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_URLENCODED +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ENDPOINT_DISCOVERY import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.FORWARDED @@ -158,6 +159,16 @@ class TestController { } } + @RequestMapping(value = "/discovery", + method = [RequestMethod.POST, RequestMethod.PATCH, RequestMethod.PUT], + consumes = MediaType.APPLICATION_JSON_VALUE, + produces = MediaType.TEXT_PLAIN_VALUE) + ResponseEntity discovery() { + HttpServerTest.controller(ENDPOINT_DISCOVERY) { + new ResponseEntity(ENDPOINT_DISCOVERY.body, HttpStatus.valueOf(ENDPOINT_DISCOVERY.status)) + } + } + @ExceptionHandler ResponseEntity handleException(Throwable throwable) { new ResponseEntity(throwable.message, HttpStatus.INTERNAL_SERVER_ERROR) diff --git a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletWithPathPatternsInstrumentation.java b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletWithPathPatternsInstrumentation.java new file mode 100644 index 00000000000..e78b5bd7c0a --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletWithPathPatternsInstrumentation.java @@ -0,0 +1,78 @@ +package datadog.trace.instrumentation.springweb; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isProtected; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.Config; +import datadog.trace.api.telemetry.EndpointCollector; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.matcher.ElementMatcher; +import org.springframework.context.ApplicationContext; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.servlet.mvc.method.RequestMappingInfo; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; + +@AutoService(InstrumenterModule.class) +public class AppSecDispatcherServletWithPathPatternsInstrumentation + extends InstrumenterModule.AppSec + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public AppSecDispatcherServletWithPathPatternsInstrumentation() { + super("spring-web"); + } + + @Override + public String instrumentedType() { + return "org.springframework.web.servlet.DispatcherServlet"; + } + + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".RequestMappingInfoWithPathPatternsIterator"}; + } + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + return hasClassNamed( + "org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition"); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isMethod() + .and(isProtected()) + .and(named("onRefresh")) + .and(takesArgument(0, named("org.springframework.context.ApplicationContext"))) + .and(takesArguments(1)), + AppSecDispatcherServletWithPathPatternsInstrumentation.class.getName() + + "$AppSecHandlerMappingAdvice"); + } + + public static class AppSecHandlerMappingAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void afterRefresh(@Advice.Argument(0) final ApplicationContext springCtx) { + if (Config.get().isApiSecurityEndpointCollectionEnabled()) { + final RequestMappingHandlerMapping handler = + springCtx.getBean(RequestMappingHandlerMapping.class); + if (handler == null) { + return; + } + final Map mappings = handler.getHandlerMethods(); + if (mappings == null || mappings.isEmpty()) { + return; + } + EndpointCollector.get().supplier(new RequestMappingInfoWithPathPatternsIterator(mappings)); + } + } + } +} diff --git a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoWithPathPatternsIterator.java b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoWithPathPatternsIterator.java new file mode 100644 index 00000000000..aea8d8a1639 --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoWithPathPatternsIterator.java @@ -0,0 +1,112 @@ +package datadog.trace.instrumentation.springweb; + +import datadog.trace.api.telemetry.Endpoint; +import datadog.trace.api.telemetry.Endpoint.Method; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Queue; +import java.util.Set; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.servlet.mvc.condition.MediaTypeExpression; +import org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition; +import org.springframework.web.servlet.mvc.condition.PatternsRequestCondition; +import org.springframework.web.servlet.mvc.method.RequestMappingInfo; + +public class RequestMappingInfoWithPathPatternsIterator implements Iterator { + + private final Map mappings; + private final Queue queue = new LinkedList<>(); + private Iterator> iterator; + + public RequestMappingInfoWithPathPatternsIterator( + final Map mappings) { + this.mappings = mappings; + } + + private Iterator> iterator() { + if (iterator == null) { + iterator = mappings.entrySet().iterator(); + } + return iterator; + } + + @Override + public boolean hasNext() { + return !queue.isEmpty() || iterator().hasNext(); + } + + @Override + public Endpoint next() { + if (queue.isEmpty()) { + fetchNext(); + } + final Endpoint endpoint = queue.poll(); + if (endpoint == null) { + throw new NoSuchElementException(); + } + return endpoint; + } + + private void fetchNext() { + final Iterator> delegate = iterator(); + if (!delegate.hasNext()) { + return; + } + final Map.Entry nextEntry = delegate.next(); + final RequestMappingInfo nextInfo = nextEntry.getKey(); + final HandlerMethod nextHandler = nextEntry.getValue(); + final List requestBody = + parseMediaTypes(nextInfo.getConsumesCondition().getExpressions()); + final List responseBody = + parseMediaTypes(nextInfo.getProducesCondition().getExpressions()); + for (final String path : getPatterns(nextInfo)) { + final List methods = Method.parseMethods(nextInfo.getMethodsCondition().getMethods()); + for (final String method : methods) { + final Endpoint endpoint = + new Endpoint() + .type(Endpoint.Type.REST) + .operation(Endpoint.Operation.HTTP_REQUEST) + .path(path) + .method(method) + .requestBodyType(requestBody) + .responseBodyType(responseBody); + if (nextHandler != null) { + final Map metadata = new HashMap<>(); + metadata.put("handler", nextHandler.toString()); + endpoint.metadata(metadata); + } + queue.add(endpoint); + } + } + } + + private Set getPatterns(final RequestMappingInfo info) { + final Set result = new HashSet<>(); + final PatternsRequestCondition patternsCondition = info.getPatternsCondition(); + if (patternsCondition != null) { + result.addAll(patternsCondition.getPatterns()); + } + final PathPatternsRequestCondition pathPatternsCondition = info.getPathPatternsCondition(); + if (pathPatternsCondition != null) { + result.addAll(pathPatternsCondition.getPatternValues()); + } + return result; + } + + private List parseMediaTypes(final Set expressions) { + if (expressions == null || expressions.isEmpty()) { + return null; + } + final List result = new ArrayList<>(expressions.size()); + for (final MediaTypeExpression expression : expressions) { + result.add(expression.toString()); + } + return result; + } +} diff --git a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/test/groovy/test/boot/EndpointCollectorSpringBootTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/test/groovy/test/boot/EndpointCollectorSpringBootTest.groovy new file mode 100644 index 00000000000..34013616182 --- /dev/null +++ b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/test/groovy/test/boot/EndpointCollectorSpringBootTest.groovy @@ -0,0 +1,54 @@ +package test.boot + +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.telemetry.Endpoint +import datadog.trace.api.telemetry.EndpointCollector +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.http.HttpStatus +import org.springframework.http.MediaType +import org.springframework.http.ResponseEntity +import org.springframework.stereotype.Controller +import org.springframework.web.bind.annotation.RequestMapping +import org.springframework.web.bind.annotation.RequestMethod +import org.springframework.web.servlet.config.annotation.EnableWebMvc + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ENDPOINT_DISCOVERY + +@SpringBootTest(classes = DiscoveryController) +@EnableWebMvc +@AutoConfigureMockMvc +class EndpointCollectorSpringBootTest extends AgentTestRunner { + + @Controller + static class DiscoveryController { + + @RequestMapping(value = "/discovery", + method = [RequestMethod.POST, RequestMethod.PATCH, RequestMethod.PUT], + consumes = MediaType.APPLICATION_JSON_VALUE, + produces = MediaType.TEXT_PLAIN_VALUE) + ResponseEntity discovery() { + HttpServerTest.controller(ENDPOINT_DISCOVERY) { + new ResponseEntity(ENDPOINT_DISCOVERY.body, HttpStatus.valueOf(ENDPOINT_DISCOVERY.status)) + } + } + } + + void 'test endpoint discovery'() { + when: + final endpoints = EndpointCollector.get().drain().toList().findAll { it.path == ENDPOINT_DISCOVERY.path } + + then: + final discovery = endpoints.collectEntries { [(it.method): it] } as Map + discovery.keySet().containsAll([Endpoint.Method.POST, Endpoint.Method.PATCH, Endpoint.Method.PUT]) + discovery.values().each { + assert it.path == ENDPOINT_DISCOVERY.path + assert it.type == Endpoint.Type.REST + assert it.operation == Endpoint.Operation.HTTP_REQUEST + assert it.requestBodyType.containsAll([MediaType.APPLICATION_JSON_VALUE]) + assert it.responseBodyType.containsAll([MediaType.TEXT_PLAIN_VALUE]) + assert it.metadata['handler'] == 'test.boot.EndpointCollectorSpringBootTest$DiscoveryController#discovery()' + } + } +} diff --git a/dd-java-agent/instrumentation/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy index 8d6a2a81b72..7700fac82ef 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/SpringBootBasedTest.groovy @@ -1,5 +1,6 @@ package datadog.trace.instrumentation.springweb6.boot +import datadog.trace.api.telemetry.Endpoint import datadog.trace.agent.test.asserts.TraceAssert import datadog.trace.agent.test.base.HttpServer import datadog.trace.agent.test.base.HttpServerTest @@ -26,6 +27,7 @@ import org.springframework.beans.BeansException import org.springframework.boot.SpringApplication import org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext import org.springframework.context.ConfigurableApplicationContext +import org.springframework.http.MediaType import org.springframework.web.servlet.HandlerInterceptor import org.springframework.web.socket.BinaryMessage import org.springframework.web.socket.TextMessage @@ -275,6 +277,22 @@ class SpringBootBasedTest extends HttpServerTest "/path/{id}/param" } + @Override + boolean testEndpointDiscovery() { + true + } + + @Override + void assertEndpointDiscovery(final List endpoints) { + final discovered = endpoints.collectEntries { [(it.method): it] } as Map + assert discovered.keySet().containsAll([Endpoint.Method.POST, Endpoint.Method.PATCH, Endpoint.Method.PUT]) + discovered.values().each { + assert it.requestBodyType.containsAll([MediaType.APPLICATION_JSON_VALUE]) + assert it.responseBodyType.containsAll([MediaType.TEXT_PLAIN_VALUE]) + assert it.metadata['handler'] == 'datadog.trace.instrumentation.springweb6.boot.TestController#discovery()' + } + } + def "test character encoding of #testPassword"() { setup: def authProvider = context.getBean(SavingAuthenticationProvider) diff --git a/dd-java-agent/instrumentation/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/TestController.groovy b/dd-java-agent/instrumentation/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/TestController.groovy index d234af4dcc9..06ddc978215 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/TestController.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-6.0/src/test/groovy/datadog/trace/instrumentation/springweb6/boot/TestController.groovy @@ -137,6 +137,16 @@ class TestController { } } + @RequestMapping(value = "/discovery", + method = [RequestMethod.POST, RequestMethod.PATCH, RequestMethod.PUT], + consumes = MediaType.APPLICATION_JSON_VALUE, + produces = MediaType.TEXT_PLAIN_VALUE) + ResponseEntity discovery() { + HttpServerTest.controller(ENDPOINT_DISCOVERY) { + new ResponseEntity(ENDPOINT_DISCOVERY.body, HttpStatus.valueOf(ENDPOINT_DISCOVERY.status)) + } + } + @ExceptionHandler ResponseEntity handleException(Throwable throwable) { new ResponseEntity(throwable.message, HttpStatus.INTERNAL_SERVER_ERROR) diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 0edf3993aaa..a1d402e46b4 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -26,6 +26,8 @@ import datadog.trace.api.gateway.RequestContextSlot import datadog.trace.api.http.StoredBodySupplier import datadog.trace.api.iast.IastContext import datadog.trace.api.normalize.SimpleHttpPathNormalizer +import datadog.trace.api.telemetry.Endpoint +import datadog.trace.api.telemetry.EndpointCollector import datadog.trace.bootstrap.blocking.BlockingActionHelper import datadog.trace.bootstrap.instrumentation.api.AgentTracer import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags @@ -404,6 +406,10 @@ abstract class HttpServerTest extends WithHttpServer { server instanceof WebsocketServer } + boolean testEndpointDiscovery() { + false + } + @Override int version() { return 0 @@ -458,6 +464,8 @@ abstract class HttpServerTest extends WithHttpServer { SESSION_ID("session", 200, null), WEBSOCKET("websocket", 101, null) + ENDPOINT_DISCOVERY('discovery', 200, 'OK') + private final String path private final String rawPath final String query @@ -2100,6 +2108,27 @@ abstract class HttpServerTest extends WithHttpServer { } } + void 'test endpoint discovery'() { + setup: + assumeTrue(testEndpointDiscovery()) + + when: + final endpoints = EndpointCollector.get().drain().toList() + final discovered = endpoints.findAll { it.path == ServerEndpoint.ENDPOINT_DISCOVERY.path } + + then: + !discovered.isEmpty() + discovered.each { + assert it.path == ServerEndpoint.ENDPOINT_DISCOVERY.path + assert it.type == Endpoint.Type.REST + assert it.operation == Endpoint.Operation.HTTP_REQUEST + } + assertEndpointDiscovery(discovered) + } + + // to be overridden for more specific asserts + void assertEndpointDiscovery(final List endpoints) { } + void controllerSpan(TraceAssert trace, ServerEndpoint endpoint = null) { def exception = endpoint == CUSTOM_EXCEPTION ? expectedCustomExceptionType() : expectedExceptionType() def errorMessage = endpoint?.body diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index 172b8f2051b..15beae2903a 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -115,6 +115,8 @@ public final class ConfigDefaults { static final int DEFAULT_APPSEC_WAF_TIMEOUT = 100000; // 0.1 s static final boolean DEFAULT_API_SECURITY_ENABLED = false; static final float DEFAULT_API_SECURITY_SAMPLE_DELAY = 30.0f; + static final boolean DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_ENABLED = true; + static final int DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_MESSAGE_LIMIT = 300; static final boolean DEFAULT_APPSEC_RASP_ENABLED = true; static final boolean DEFAULT_APPSEC_STACK_TRACE_ENABLED = true; static final int DEFAULT_APPSEC_MAX_STACK_TRACES = 2; diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java index a7f71b6f664..d81d6cc1880 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java @@ -27,6 +27,11 @@ public final class AppSecConfig { public static final String API_SECURITY_ENABLED_EXPERIMENTAL = "experimental.api-security.enabled"; public static final String API_SECURITY_SAMPLE_DELAY = "api-security.sample.delay"; + public static final String API_SECURITY_REQUEST_SAMPLE_RATE = "api-security.request.sample.rate"; + public static final String API_SECURITY_ENDPOINT_COLLECTION_ENABLED = + "api-security.endpoint.collection.enabled"; + public static final String API_SECURITY_ENDPOINT_COLLECTION_MESSAGE_LIMIT = + "api-security.endpoint.collection.message.limit"; public static final String APPSEC_SCA_ENABLED = "appsec.sca.enabled"; public static final String APPSEC_RASP_ENABLED = "appsec.rasp.enabled"; diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index eb4091e1156..5354c90063b 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -293,6 +293,8 @@ public static String getHostName() { private final int appSecMaxStackTraceDepth; private final boolean apiSecurityEnabled; private final float apiSecuritySampleDelay; + private final boolean apiSecurityEndpointCollectionEnabled; + private final int apiSecurityEndpointCollectionMessageLimit; private final IastDetectionMode iastDetectionMode; private final int iastMaxConcurrentRequests; @@ -1387,6 +1389,15 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) API_SECURITY_ENABLED, DEFAULT_API_SECURITY_ENABLED, API_SECURITY_ENABLED_EXPERIMENTAL); apiSecuritySampleDelay = configProvider.getFloat(API_SECURITY_SAMPLE_DELAY, DEFAULT_API_SECURITY_SAMPLE_DELAY); + apiSecurityEndpointCollectionEnabled = + configProvider.getBoolean( + API_SECURITY_ENDPOINT_COLLECTION_ENABLED, + getAppSecActivation() != ProductActivation.FULLY_DISABLED + && DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_ENABLED); + apiSecurityEndpointCollectionMessageLimit = + configProvider.getInteger( + API_SECURITY_ENDPOINT_COLLECTION_MESSAGE_LIMIT, + DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_MESSAGE_LIMIT); iastDebugEnabled = configProvider.getBoolean(IAST_DEBUG_ENABLED, DEFAULT_IAST_DEBUG_ENABLED); @@ -2766,6 +2777,14 @@ public float getApiSecuritySampleDelay() { return apiSecuritySampleDelay; } + public int getApiSecurityEndpointCollectionMessageLimit() { + return apiSecurityEndpointCollectionMessageLimit; + } + + public boolean isApiSecurityEndpointCollectionEnabled() { + return apiSecurityEndpointCollectionEnabled; + } + public ProductActivation getIastActivation() { return instrumenterConfig.getIastActivation(); } @@ -4798,6 +4817,10 @@ public String toString() { + appSecHttpBlockedTemplateJson + ", apiSecurityEnabled=" + apiSecurityEnabled + + ", apiSecurityEndpointCollectionEnabled=" + + apiSecurityEndpointCollectionEnabled + + ", apiSecurityEndpointCollectionMessageLimit=" + + apiSecurityEndpointCollectionMessageLimit + ", cwsEnabled=" + cwsEnabled + ", cwsTlsRefresh=" diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java b/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java new file mode 100644 index 00000000000..6ddca894a8c --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java @@ -0,0 +1,155 @@ +package datadog.trace.api.telemetry; + +import static java.util.Collections.singletonList; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class Endpoint { + private String type; + private String method; + private String path; + private String operation; + private List requestBodyType; + private List responseBodyType; + private List responseCode; + private List authentication; + private Map metadata; + + public String getType() { + return type; + } + + public Endpoint type(final String type) { + this.type = type; + return this; + } + + public String getMethod() { + return method; + } + + public Endpoint method(final String method) { + this.method = method; + return this; + } + + public String getPath() { + return path; + } + + public Endpoint path(final String path) { + this.path = path; + return this; + } + + public String getOperation() { + return operation; + } + + public Endpoint operation(final String operation) { + this.operation = operation; + return this; + } + + public List getRequestBodyType() { + return requestBodyType; + } + + public Endpoint requestBodyType(final List requestBodyType) { + this.requestBodyType = requestBodyType; + return this; + } + + public List getResponseBodyType() { + return responseBodyType; + } + + public Endpoint responseBodyType(final List responseBodyType) { + this.responseBodyType = responseBodyType; + return this; + } + + public List getResponseCode() { + return responseCode; + } + + public Endpoint responseCode(final List responseCode) { + this.responseCode = responseCode; + return this; + } + + public List getAuthentication() { + return authentication; + } + + public Endpoint authentication(final List authentication) { + this.authentication = authentication; + return this; + } + + public Map getMetadata() { + return metadata; + } + + public Endpoint metadata(final Map metadata) { + this.metadata = metadata; + return this; + } + + public interface Operation { + String HTTP_REQUEST = "http.request"; + } + + public interface Type { + String REST = "REST"; + } + + public interface Method { + String CONNECT = "CONNECT"; + String HEAD = "HEAD"; + String GET = "GET"; + String POST = "POST"; + String PUT = "PUT"; + String PATCH = "PATCH"; + String OPTIONS = "OPTIONS"; + String DELETE = "DELETE"; + String TRACE = "TRACE"; + String ALL = "*"; + + Set METHODS = + new HashSet<>(Arrays.asList(CONNECT, HEAD, GET, POST, PUT, PATCH, OPTIONS, DELETE, TRACE)); + + static , C extends Collection> List parseMethods(final C methods) { + if (methods == null || methods.isEmpty()) { + return singletonList(ALL); + } + final List result = new ArrayList<>(methods.size()); + for (E value : methods) { + final String methodName = value.name().toUpperCase(); + if (METHODS.contains(value.name())) { + result.add(methodName); + } + } + return result; + } + } + + public interface Authentication { + String JWT = "JWT"; + String BASIC = "basic"; + String OAUTH = "oauth"; + String OIDC = "OIDC"; + String API_KEY = "api_key"; + String SESSION = "session"; + String M_TLS = "mTLS"; + String SAML = "SAML"; + String FORM = "Form"; + String OTHER = "other"; + } +} diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/EndpointCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/EndpointCollector.java new file mode 100644 index 00000000000..b8434bfddae --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/EndpointCollector.java @@ -0,0 +1,26 @@ +package datadog.trace.api.telemetry; + +import static java.util.Collections.emptyIterator; + +import java.util.Iterator; +import java.util.concurrent.atomic.AtomicReference; + +public class EndpointCollector { + + private static final EndpointCollector INSTANCE = new EndpointCollector(); + + public static EndpointCollector get() { + return INSTANCE; + } + + private final AtomicReference> provider = + new AtomicReference<>(emptyIterator()); + + public Iterator drain() { + return provider.getAndSet(emptyIterator()); + } + + public void supplier(final Iterator supplier) { + provider.set(supplier); + } +} diff --git a/telemetry/src/main/java/datadog/telemetry/BufferedEvents.java b/telemetry/src/main/java/datadog/telemetry/BufferedEvents.java index cd9a13ba242..e118b82303a 100644 --- a/telemetry/src/main/java/datadog/telemetry/BufferedEvents.java +++ b/telemetry/src/main/java/datadog/telemetry/BufferedEvents.java @@ -6,6 +6,7 @@ import datadog.telemetry.api.Metric; import datadog.telemetry.dependency.Dependency; import datadog.trace.api.ConfigSetting; +import datadog.trace.api.telemetry.Endpoint; import datadog.trace.api.telemetry.ProductChange; import java.util.ArrayList; @@ -29,6 +30,8 @@ public final class BufferedEvents implements EventSource, EventSink { private int logMessageIndex; private ArrayList productChangeEvents; private int productChangeIndex; + private ArrayList endpointEvents; + private int endpointIndex; public void addConfigChangeEvent(ConfigSetting event) { if (configChangeEvents == null) { @@ -85,6 +88,14 @@ public void addProductChangeEvent(ProductChange event) { productChangeEvents.add(event); } + @Override + public void addEndpointEvent(final Endpoint event) { + if (endpointEvents == null) { + endpointEvents = new ArrayList<>(INITIAL_CAPACITY); + } + endpointEvents.add(event); + } + @Override public boolean hasConfigChangeEvent() { return configChangeEvents != null && configChangeIndex < configChangeEvents.size(); @@ -155,4 +166,14 @@ public boolean hasProductChangeEvent() { public ProductChange nextProductChangeEvent() { return productChangeEvents.get(productChangeIndex++); } + + @Override + public boolean hasEndpoint() { + return endpointEvents != null && endpointIndex < endpointEvents.size(); + } + + @Override + public Endpoint nextEndpoint() { + return endpointEvents.get(endpointIndex++); + } } diff --git a/telemetry/src/main/java/datadog/telemetry/EventSink.java b/telemetry/src/main/java/datadog/telemetry/EventSink.java index 2fcde6ddf75..58464184e38 100644 --- a/telemetry/src/main/java/datadog/telemetry/EventSink.java +++ b/telemetry/src/main/java/datadog/telemetry/EventSink.java @@ -6,6 +6,7 @@ import datadog.telemetry.api.Metric; import datadog.telemetry.dependency.Dependency; import datadog.trace.api.ConfigSetting; +import datadog.trace.api.telemetry.Endpoint; import datadog.trace.api.telemetry.ProductChange; /** @@ -28,6 +29,8 @@ interface EventSink { void addProductChangeEvent(ProductChange event); + void addEndpointEvent(Endpoint event); + EventSink NOOP = new Noop(); class Noop implements EventSink { @@ -53,5 +56,8 @@ public void addLogMessageEvent(LogMessage event) {} @Override public void addProductChangeEvent(ProductChange event) {} + + @Override + public void addEndpointEvent(Endpoint event) {} } } diff --git a/telemetry/src/main/java/datadog/telemetry/EventSource.java b/telemetry/src/main/java/datadog/telemetry/EventSource.java index 5f43e621754..b0088474c21 100644 --- a/telemetry/src/main/java/datadog/telemetry/EventSource.java +++ b/telemetry/src/main/java/datadog/telemetry/EventSource.java @@ -6,6 +6,7 @@ import datadog.telemetry.api.Metric; import datadog.telemetry.dependency.Dependency; import datadog.trace.api.ConfigSetting; +import datadog.trace.api.telemetry.Endpoint; import datadog.trace.api.telemetry.ProductChange; import java.util.Queue; @@ -43,6 +44,10 @@ interface EventSource { ProductChange nextProductChangeEvent(); + boolean hasEndpoint(); + + Endpoint nextEndpoint(); + default boolean isEmpty() { return !hasConfigChangeEvent() && !hasIntegrationEvent() @@ -50,7 +55,8 @@ default boolean isEmpty() { && !hasMetricEvent() && !hasDistributionSeriesEvent() && !hasLogMessageEvent() - && !hasProductChangeEvent(); + && !hasProductChangeEvent() + && !hasEndpoint(); } final class Queued implements EventSource { @@ -61,6 +67,7 @@ final class Queued implements EventSource { private final Queue distributionSeriesQueue; private final Queue logMessageQueue; private final Queue productChanges; + private final Queue endpoints; Queued( Queue configChangeQueue, @@ -69,7 +76,8 @@ final class Queued implements EventSource { Queue metricQueue, Queue distributionSeriesQueue, Queue logMessageQueue, - Queue productChanges) { + Queue productChanges, + Queue endpoints) { this.configChangeQueue = configChangeQueue; this.integrationQueue = integrationQueue; this.dependencyQueue = dependencyQueue; @@ -77,6 +85,7 @@ final class Queued implements EventSource { this.distributionSeriesQueue = distributionSeriesQueue; this.logMessageQueue = logMessageQueue; this.productChanges = productChanges; + this.endpoints = endpoints; } @Override @@ -148,5 +157,15 @@ public boolean hasProductChangeEvent() { public ProductChange nextProductChangeEvent() { return productChanges.poll(); } + + @Override + public boolean hasEndpoint() { + return !endpoints.isEmpty(); + } + + @Override + public Endpoint nextEndpoint() { + return endpoints.poll(); + } } } diff --git a/telemetry/src/main/java/datadog/telemetry/ExtendedHeartbeatData.java b/telemetry/src/main/java/datadog/telemetry/ExtendedHeartbeatData.java index 65f4a31bc86..75c26b4db9d 100644 --- a/telemetry/src/main/java/datadog/telemetry/ExtendedHeartbeatData.java +++ b/telemetry/src/main/java/datadog/telemetry/ExtendedHeartbeatData.java @@ -6,6 +6,7 @@ import datadog.telemetry.api.Metric; import datadog.telemetry.dependency.Dependency; import datadog.trace.api.ConfigSetting; +import datadog.trace.api.telemetry.Endpoint; import datadog.trace.api.telemetry.ProductChange; import java.util.ArrayList; @@ -121,5 +122,15 @@ public boolean hasProductChangeEvent() { public ProductChange nextProductChangeEvent() { return null; } + + @Override + public boolean hasEndpoint() { + return false; + } + + @Override + public Endpoint nextEndpoint() { + return null; + } } } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java b/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java index 915b3fcdc93..44d73130bbe 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java @@ -13,6 +13,7 @@ import datadog.trace.api.DDTags; import datadog.trace.api.InstrumenterConfig; import datadog.trace.api.ProductActivation; +import datadog.trace.api.telemetry.Endpoint; import datadog.trace.api.telemetry.ProductChange; import datadog.trace.api.telemetry.ProductChange.ProductType; import java.io.IOException; @@ -226,6 +227,24 @@ public void writeChangedProducts() { } } + public void writeEndpoints() { + if (!isWithinSizeLimits() || !eventSource.hasEndpoint()) { + return; + } + try { + log.debug("Writing endpoints"); + requestBody.beginEndpoints(); + while (eventSource.hasEndpoint() && isWithinSizeLimits()) { + Endpoint event = eventSource.nextEndpoint(); + requestBody.writeEndpoint(event); + eventSink.addEndpointEvent(event); + } + requestBody.endEndpoints(); + } catch (IOException e) { + throw new TelemetryRequestBody.SerializationException("asm-endpoints", e); + } + } + public void writeHeartbeat() { requestBody.writeHeartbeatEvent(); } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java b/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java index 8f32e9bbec8..4c135adbc9a 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java @@ -12,6 +12,7 @@ import datadog.trace.api.ConfigSetting; import datadog.trace.api.DDTags; import datadog.trace.api.Platform; +import datadog.trace.api.telemetry.Endpoint; import datadog.trace.api.telemetry.ProductChange.ProductType; import java.io.IOException; import java.util.EnumMap; @@ -303,6 +304,41 @@ public void endProducts() throws IOException { endMessageIfBatch(RequestType.APP_PRODUCT_CHANGE); } + public void beginEndpoints() throws IOException { + beginMessageIfBatch(RequestType.ASM_ENDPOINTS); + bodyWriter.name("endpoints"); + bodyWriter.beginArray(); + } + + public void writeEndpoint(final Endpoint endpoint) throws IOException { + bodyWriter.beginObject(); + bodyWriter.name("type").value(endpoint.getType()); + bodyWriter.name("method").value(endpoint.getMethod()); + bodyWriter.name("path").value(endpoint.getPath()); + bodyWriter.name("operation-name").value(endpoint.getOperation()); + if (endpoint.getRequestBodyType() != null) { + bodyWriter.name("request-body-type").jsonValue(endpoint.getRequestBodyType()); + } + if (endpoint.getResponseBodyType() != null) { + bodyWriter.name("response-body-type").jsonValue(endpoint.getResponseBodyType()); + } + if (endpoint.getResponseCode() != null) { + bodyWriter.name("response-code").jsonValue(endpoint.getResponseCode()); + } + if (endpoint.getAuthentication() != null) { + bodyWriter.name("authentication").jsonValue(endpoint.getAuthentication()); + } + if (endpoint.getMetadata() != null) { + bodyWriter.name("metadata").jsonValue(endpoint.getMetadata()); + } + bodyWriter.endObject(); + } + + public void endEndpoints() throws IOException { + bodyWriter.endArray(); + endMessageIfBatch(RequestType.ASM_ENDPOINTS); + } + public void writeInstallSignature(String installId, String installType, String installTime) throws IOException { if (installId == null && installType == null && installTime == null) { diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryService.java b/telemetry/src/main/java/datadog/telemetry/TelemetryService.java index 6e71cb7c6b3..1160c27223a 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryService.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryService.java @@ -8,6 +8,7 @@ import datadog.telemetry.api.RequestType; import datadog.telemetry.dependency.Dependency; import datadog.trace.api.ConfigSetting; +import datadog.trace.api.telemetry.Endpoint; import datadog.trace.api.telemetry.ProductChange; import java.util.Map; import java.util.concurrent.BlockingQueue; @@ -36,6 +37,9 @@ public class TelemetryService { private final BlockingQueue productChanges = new LinkedBlockingQueue<>(); private final ExtendedHeartbeatData extendedHeartbeatData = new ExtendedHeartbeatData(); + + private final BlockingQueue endpoints = new LinkedBlockingQueue<>(); + private final EventSource.Queued eventSource = new EventSource.Queued( configurations, @@ -44,7 +48,8 @@ public class TelemetryService { metrics, distributionSeries, logMessages, - productChanges); + productChanges, + endpoints); private final long messageBytesSoftLimit; private final boolean debug; @@ -124,6 +129,10 @@ public boolean addDistributionSeries(DistributionSeries series) { return this.distributionSeries.offer(series); } + public boolean addEndpoint(final Endpoint endpoint) { + return this.endpoints.offer(endpoint); + } + public void sendAppClosingEvent() { TelemetryRequest telemetryRequest = new TelemetryRequest( @@ -210,6 +219,7 @@ public boolean sendTelemetryEvents() { request.writeDistributions(); request.writeLogs(); request.writeChangedProducts(); + request.writeEndpoints(); isMoreDataAvailable = !this.eventSource.isEmpty(); } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetrySystem.java b/telemetry/src/main/java/datadog/telemetry/TelemetrySystem.java index 9605b3366c1..8a1c3af38de 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetrySystem.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetrySystem.java @@ -6,6 +6,7 @@ import datadog.telemetry.TelemetryRunnable.TelemetryPeriodicAction; import datadog.telemetry.dependency.DependencyPeriodicAction; import datadog.telemetry.dependency.DependencyService; +import datadog.telemetry.endpoint.EndpointPeriodicAction; import datadog.telemetry.integration.IntegrationPeriodicAction; import datadog.telemetry.log.LogPeriodicAction; import datadog.telemetry.metric.CiVisibilityMetricPeriodicAction; @@ -67,6 +68,9 @@ static Thread createTelemetryRunnable( log.debug("Telemetry log collection enabled"); } actions.add(new ProductChangeAction()); + if (Config.get().isApiSecurityEndpointCollectionEnabled()) { + actions.add(new EndpointPeriodicAction()); + } TelemetryRunnable telemetryRunnable = new TelemetryRunnable(telemetryService, actions); return AgentThreadFactory.newAgentThread( diff --git a/telemetry/src/main/java/datadog/telemetry/api/RequestType.java b/telemetry/src/main/java/datadog/telemetry/api/RequestType.java index a1a9f41ed3a..81f96e29366 100644 --- a/telemetry/src/main/java/datadog/telemetry/api/RequestType.java +++ b/telemetry/src/main/java/datadog/telemetry/api/RequestType.java @@ -12,7 +12,8 @@ public enum RequestType { GENERATE_METRICS("generate-metrics"), LOGS("logs"), DISTRIBUTIONS("distributions"), - MESSAGE_BATCH("message-batch"); + MESSAGE_BATCH("message-batch"), + ASM_ENDPOINTS("asm-endpoints"); private final String value; diff --git a/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java new file mode 100644 index 00000000000..91bbea44507 --- /dev/null +++ b/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java @@ -0,0 +1,19 @@ +package datadog.telemetry.endpoint; + +import datadog.telemetry.TelemetryRunnable; +import datadog.telemetry.TelemetryService; +import datadog.trace.api.telemetry.Endpoint; +import datadog.trace.api.telemetry.EndpointCollector; +import java.util.Iterator; + +public class EndpointPeriodicAction implements TelemetryRunnable.TelemetryPeriodicAction { + + @Override + public void doIteration(final TelemetryService service) { + for (final Iterator it = EndpointCollector.get().drain(); it.hasNext(); ) { + if (!service.addEndpoint(it.next())) { + break; + } + } + } +} diff --git a/telemetry/src/test/groovy/datadog/telemetry/BufferedEventsSpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/BufferedEventsSpecification.groovy index 7497fa6fe2c..4dcd8cd50b1 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/BufferedEventsSpecification.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/BufferedEventsSpecification.groovy @@ -7,6 +7,7 @@ import datadog.telemetry.api.Metric import datadog.telemetry.dependency.Dependency import datadog.trace.api.ConfigOrigin import datadog.trace.api.ConfigSetting +import datadog.trace.api.telemetry.Endpoint import datadog.trace.test.util.DDSpecification class BufferedEventsSpecification extends DDSpecification { @@ -22,6 +23,7 @@ class BufferedEventsSpecification extends DDSpecification { !events.hasIntegrationEvent() !events.hasLogMessageEvent() !events.hasMetricEvent() + !events.hasEndpoint() } def 'return added events'() { @@ -32,6 +34,7 @@ class BufferedEventsSpecification extends DDSpecification { def integration = new Integration("integration-name", true) def logMessage = new LogMessage() def metric = new Metric() + def endpoint = new Endpoint() when: events.addConfigChangeEvent(configSetting) @@ -92,6 +95,16 @@ class BufferedEventsSpecification extends DDSpecification { events.nextMetricEvent() == metric !events.hasMetricEvent() events.isEmpty() + + when: + events.addEndpointEvent(endpoint) + + then: + !events.isEmpty() + events.hasEndpoint() + events.nextEndpoint() == endpoint + !events.hasEndpoint() + events.isEmpty() } def 'noop sink'() { @@ -104,5 +117,6 @@ class BufferedEventsSpecification extends DDSpecification { sink.addDistributionSeriesEvent(null) sink.addDependencyEvent(null) sink.addConfigChangeEvent(null) + sink.addEndpointEvent(null) } } diff --git a/telemetry/src/test/groovy/datadog/telemetry/EventSourceTest.groovy b/telemetry/src/test/groovy/datadog/telemetry/EventSourceTest.groovy index ae004bc051e..5a492c18171 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/EventSourceTest.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/EventSourceTest.groovy @@ -7,6 +7,7 @@ import datadog.telemetry.api.Metric import datadog.telemetry.dependency.Dependency import datadog.trace.api.ConfigOrigin import datadog.trace.api.ConfigSetting +import datadog.trace.api.telemetry.Endpoint import datadog.trace.api.telemetry.ProductChange import datadog.trace.test.util.DDSpecification @@ -23,7 +24,8 @@ class EventSourceTest extends DDSpecification{ metricQueue : new LinkedBlockingQueue(), distributionSeriesQueue: new LinkedBlockingQueue(), logMessageQueue : new LinkedBlockingQueue(), - productChanges : new LinkedBlockingQueue() + productChanges : new LinkedBlockingQueue(), + endpointQueue : new LinkedBlockingQueue() ] def eventSource = new EventSource.Queued( @@ -33,7 +35,8 @@ class EventSourceTest extends DDSpecification{ eventQueues.metricQueue, eventQueues.distributionSeriesQueue, eventQueues.logMessageQueue, - eventQueues.productChanges + eventQueues.productChanges, + eventQueues.endpointQueue ) expect: @@ -60,5 +63,6 @@ class EventSourceTest extends DDSpecification{ "Distribution Series" | "distributionSeriesQueue" | new DistributionSeries() "Log Message" | "logMessageQueue" | new LogMessage() "Product Change" | "productChanges" | new ProductChange() + "Endpoint" | "endpointQueue" | new Endpoint() } } diff --git a/telemetry/src/test/groovy/datadog/telemetry/TelemetryServiceSpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/TelemetryServiceSpecification.groovy index 91a4c7d6c08..246dc3cbc10 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/TelemetryServiceSpecification.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/TelemetryServiceSpecification.groovy @@ -12,10 +12,13 @@ import datadog.trace.api.ConfigSetting import datadog.trace.api.config.AppSecConfig import datadog.trace.api.config.DebuggerConfig import datadog.trace.api.config.ProfilingConfig +import datadog.trace.api.telemetry.Endpoint import datadog.trace.api.telemetry.ProductChange import datadog.trace.test.util.DDSpecification import datadog.trace.util.Strings +import static datadog.trace.api.telemetry.Endpoint.Type.REST + class TelemetryServiceSpecification extends DDSpecification { def confKeyValue = ConfigSetting.of("confkey", "confvalue", ConfigOrigin.DEFAULT) def configuration = [confkey: confKeyValue] @@ -25,6 +28,7 @@ class TelemetryServiceSpecification extends DDSpecification { def distribution = new DistributionSeries().namespace("tracers").metric("distro").points([1, 2, 3]).tags(["tag1", "tag2"]).common(false) def logMessage = new LogMessage().message("log-message").tags("tag1:tag2").level(LogMessageLevel.DEBUG).stackTrace("stack-trace").tracerTime(32423).count(1) def productChange = new ProductChange().productType(ProductChange.ProductType.APPSEC).enabled(true) + def endpoint = new Endpoint().type('REST').method("GET").operation('http.request').path("/test") def 'happy path without data'() { setup: @@ -69,6 +73,7 @@ class TelemetryServiceSpecification extends DDSpecification { telemetryService.addDistributionSeries(distribution) telemetryService.addLogMessage(logMessage) telemetryService.addProductChange(productChange) + telemetryService.addEndpoint(endpoint) and: 'send messages' testHttpClient.expectRequest(TelemetryClient.Result.SUCCESS) @@ -85,7 +90,7 @@ class TelemetryServiceSpecification extends DDSpecification { then: testHttpClient.assertRequestBody(RequestType.MESSAGE_BATCH) - .assertBatch(7) + .assertBatch(8) .assertFirstMessage(RequestType.APP_HEARTBEAT).hasNoPayload() // no configuration here as it has already been sent with the app-started event .assertNextMessage(RequestType.APP_INTEGRATIONS_CHANGE).hasPayload().integrations([integration]) @@ -94,6 +99,7 @@ class TelemetryServiceSpecification extends DDSpecification { .assertNextMessage(RequestType.DISTRIBUTIONS).hasPayload().namespace("tracers").distributionSeries([distribution]) .assertNextMessage(RequestType.LOGS).hasPayload().logs([logMessage]) .assertNextMessage(RequestType.APP_PRODUCT_CHANGE).hasPayload().productChange(productChange) + .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint(endpoint) .assertNoMoreMessages() testHttpClient.assertNoMoreRequests() @@ -140,6 +146,7 @@ class TelemetryServiceSpecification extends DDSpecification { telemetryService.addDistributionSeries(distribution) telemetryService.addLogMessage(logMessage) telemetryService.addProductChange(productChange) + telemetryService.addEndpoint(endpoint) and: 'send messages' testHttpClient.expectRequest(TelemetryClient.Result.SUCCESS) @@ -147,7 +154,7 @@ class TelemetryServiceSpecification extends DDSpecification { then: testHttpClient.assertRequestBody(RequestType.MESSAGE_BATCH) - .assertBatch(8) + .assertBatch(9) .assertFirstMessage(RequestType.APP_HEARTBEAT).hasNoPayload() .assertNextMessage(RequestType.APP_CLIENT_CONFIGURATION_CHANGE).hasPayload().configuration([confKeyValue]) .assertNextMessage(RequestType.APP_INTEGRATIONS_CHANGE).hasPayload().integrations([integration]) @@ -156,6 +163,7 @@ class TelemetryServiceSpecification extends DDSpecification { .assertNextMessage(RequestType.DISTRIBUTIONS).hasPayload().namespace("tracers").distributionSeries([distribution]) .assertNextMessage(RequestType.LOGS).hasPayload().logs([logMessage]) .assertNextMessage(RequestType.APP_PRODUCT_CHANGE).hasPayload().productChange(productChange) + .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint(endpoint) .assertNoMoreMessages() testHttpClient.assertNoMoreRequests() } @@ -211,6 +219,7 @@ class TelemetryServiceSpecification extends DDSpecification { telemetryService.addDistributionSeries(distribution) telemetryService.addLogMessage(logMessage) telemetryService.addProductChange(productChange) + telemetryService.addEndpoint(endpoint) when: 'attempt with NOT_FOUND error' testHttpClient.expectRequest(TelemetryClient.Result.NOT_FOUND) @@ -233,7 +242,7 @@ class TelemetryServiceSpecification extends DDSpecification { then: 'attempt batch with SUCCESS' testHttpClient.assertRequestBody(RequestType.MESSAGE_BATCH) - .assertBatch(7) + .assertBatch(8) .assertFirstMessage(RequestType.APP_HEARTBEAT).hasNoPayload() // no configuration here as it has already been sent with the app-started event .assertNextMessage(RequestType.APP_INTEGRATIONS_CHANGE).hasPayload().integrations([integration]) @@ -242,6 +251,7 @@ class TelemetryServiceSpecification extends DDSpecification { .assertNextMessage(RequestType.DISTRIBUTIONS).hasPayload().namespace("tracers").distributionSeries([distribution]) .assertNextMessage(RequestType.LOGS).hasPayload().logs([logMessage]) .assertNextMessage(RequestType.APP_PRODUCT_CHANGE).hasPayload().productChange(productChange) + .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint(endpoint) .assertNoMoreMessages() testHttpClient.assertNoMoreRequests() @@ -318,6 +328,7 @@ class TelemetryServiceSpecification extends DDSpecification { telemetryService.addDistributionSeries(distribution) telemetryService.addLogMessage(logMessage) telemetryService.addProductChange(productChange) + telemetryService.addEndpoint(endpoint) testHttpClient.expectRequest(TelemetryClient.Result.SUCCESS) telemetryService.sendTelemetryEvents() @@ -339,11 +350,23 @@ class TelemetryServiceSpecification extends DDSpecification { then: testHttpClient.assertRequestBody(RequestType.MESSAGE_BATCH) - .assertBatch(4) + .assertBatch(5) .assertFirstMessage(RequestType.APP_HEARTBEAT).hasNoPayload() .assertNextMessage(RequestType.DISTRIBUTIONS).hasPayload().namespace("tracers").distributionSeries([distribution]) .assertNextMessage(RequestType.LOGS).hasPayload().logs([logMessage]) .assertNextMessage(RequestType.APP_PRODUCT_CHANGE).hasPayload().productChange(productChange) + .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint() // no endpoints fit in this request + .assertNoMoreMessages() + + when: 'sending third part of data' + testHttpClient.expectRequest(TelemetryClient.Result.SUCCESS) + !telemetryService.sendTelemetryEvents() + + then: + testHttpClient.assertRequestBody(RequestType.MESSAGE_BATCH) + .assertBatch(2) + .assertFirstMessage(RequestType.APP_HEARTBEAT).hasNoPayload() + .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint(endpoint) .assertNoMoreMessages() testHttpClient.assertNoMoreRequests() } diff --git a/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy b/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy index 4c2eb9b8676..680efcb427e 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/TestTelemetryRouter.groovy @@ -8,6 +8,7 @@ import datadog.telemetry.api.LogMessage import datadog.telemetry.api.Metric import datadog.telemetry.api.RequestType import datadog.trace.api.ConfigSetting +import datadog.trace.api.telemetry.Endpoint import datadog.trace.api.telemetry.ProductChange import groovy.json.JsonSlurper import okhttp3.Request @@ -268,6 +269,36 @@ class TestTelemetryRouter extends TelemetryRouter { return this } + PayloadAssertions endpoint(final Endpoint... endpoints) { + def expected = [] + endpoints.each { + final item = [ + 'type' : it.type, + 'method' : it.method, + 'path' : it.path, + 'operation-name': it.operation + ] as Map + if (it.requestBodyType) { + item['request-body-type'] = it.requestBodyType + } + if (it.responseBodyType) { + item['response-body-type'] = it.responseBodyType + } + if (it.authentication) { + item['authentication'] = it.authentication + } + if (it.responseCode) { + item['response-code'] = it.responseCode + } + if (it.metadata) { + item['metadata'] = it.metadata + } + expected.add(item) + } + assert this.payload['endpoints'] == expected + return this + } + PayloadAssertions products(boolean appsecEnabled = true, boolean profilerEnabled = false, boolean dynamicInstrumentationEnabled = false) { def expected = [ appsec: [enabled: appsecEnabled], From cb60df28b0d7838f55319707d98edd12f740e673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Thu, 20 Mar 2025 10:08:38 +0100 Subject: [PATCH 2/9] Update endpoints payload --- .../springweb/RequestMappingInfoIterator.java | 7 +++++- ...stMappingInfoWithPathPatternsIterator.java | 5 ++++ .../agent/test/base/HttpServerTest.groovy | 15 ++++++++---- .../datadog/trace/api/telemetry/Endpoint.java | 10 ++++++++ .../datadog/telemetry/TelemetryRequest.java | 13 ++++++++--- .../telemetry/TelemetryRequestBody.java | 7 +++--- .../datadog/telemetry/api/RequestType.java | 2 +- .../TelemetryServiceSpecification.groovy | 23 ++++--------------- 8 files changed, 51 insertions(+), 31 deletions(-) diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoIterator.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoIterator.java index aea4cbb6273..88ee3852f8c 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoIterator.java +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoIterator.java @@ -20,6 +20,7 @@ public class RequestMappingInfoIterator implements Iterator { private final Map mappings; private final Queue queue = new LinkedList<>(); private Iterator> iterator; + private boolean first = true; public RequestMappingInfoIterator(final Map mappings) { this.mappings = mappings; @@ -64,7 +65,7 @@ private void fetchNext() { for (final String path : nextInfo.getPatternsCondition().getPatterns()) { final List methods = Method.parseMethods(nextInfo.getMethodsCondition().getMethods()); for (final String method : methods) { - final Endpoint endpoint = + Endpoint endpoint = new Endpoint() .type(Endpoint.Type.REST) .operation(Endpoint.Operation.HTTP_REQUEST) @@ -77,6 +78,10 @@ private void fetchNext() { metadata.put("handler", nextHandler.toString()); endpoint.metadata(metadata); } + if (first) { + endpoint.first(true); + first = false; + } queue.add(endpoint); } } diff --git a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoWithPathPatternsIterator.java b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoWithPathPatternsIterator.java index aea8d8a1639..aff0ad018ec 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoWithPathPatternsIterator.java +++ b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/RequestMappingInfoWithPathPatternsIterator.java @@ -23,6 +23,7 @@ public class RequestMappingInfoWithPathPatternsIterator implements Iterator mappings; private final Queue queue = new LinkedList<>(); private Iterator> iterator; + private boolean first = true; public RequestMappingInfoWithPathPatternsIterator( final Map mappings) { @@ -81,6 +82,10 @@ private void fetchNext() { metadata.put("handler", nextHandler.toString()); endpoint.metadata(metadata); } + if (first) { + endpoint.first(true); + first = false; + } queue.add(endpoint); } } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index a1d402e46b4..03e6a060b81 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -462,7 +462,7 @@ abstract class HttpServerTest extends WithHttpServer { SECURE_SUCCESS("secure/success", 200, null), SESSION_ID("session", 200, null), - WEBSOCKET("websocket", 101, null) + WEBSOCKET("websocket", 101, null), ENDPOINT_DISCOVERY('discovery', 200, 'OK') @@ -2117,11 +2117,16 @@ abstract class HttpServerTest extends WithHttpServer { final discovered = endpoints.findAll { it.path == ServerEndpoint.ENDPOINT_DISCOVERY.path } then: + !endpoints.isEmpty() + endpoints.eachWithIndex { Endpoint entry, int i -> + assert entry.first == (i == 0) + } + !discovered.isEmpty() - discovered.each { - assert it.path == ServerEndpoint.ENDPOINT_DISCOVERY.path - assert it.type == Endpoint.Type.REST - assert it.operation == Endpoint.Operation.HTTP_REQUEST + discovered.eachWithIndex { endpoint, index -> + assert endpoint.path == ServerEndpoint.ENDPOINT_DISCOVERY.path + assert endpoint.type == Endpoint.Type.REST + assert endpoint.operation == Endpoint.Operation.HTTP_REQUEST } assertEndpointDiscovery(discovered) } diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java b/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java index 6ddca894a8c..96eacc62270 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java @@ -11,6 +11,7 @@ import java.util.Set; public class Endpoint { + private boolean fist; private String type; private String method; private String path; @@ -21,6 +22,15 @@ public class Endpoint { private List authentication; private Map metadata; + public boolean isFirst() { + return fist; + } + + public Endpoint first(boolean first) { + this.fist = first; + return this; + } + public String getType() { return type; } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java b/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java index 44d73130bbe..7e4efefb4bc 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java @@ -234,12 +234,19 @@ public void writeEndpoints() { try { log.debug("Writing endpoints"); requestBody.beginEndpoints(); - while (eventSource.hasEndpoint() && isWithinSizeLimits()) { - Endpoint event = eventSource.nextEndpoint(); + boolean first = false; + while (eventSource.hasEndpoint()) { + final Endpoint event = eventSource.nextEndpoint(); + if (event.isFirst()) { + first = true; + } requestBody.writeEndpoint(event); eventSink.addEndpointEvent(event); + if (!isWithinSizeLimits()) { + break; + } } - requestBody.endEndpoints(); + requestBody.endEndpoints(first); } catch (IOException e) { throw new TelemetryRequestBody.SerializationException("asm-endpoints", e); } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java b/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java index 4c135adbc9a..83cddd3555a 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryRequestBody.java @@ -305,7 +305,7 @@ public void endProducts() throws IOException { } public void beginEndpoints() throws IOException { - beginMessageIfBatch(RequestType.ASM_ENDPOINTS); + beginMessageIfBatch(RequestType.APP_ENDPOINTS); bodyWriter.name("endpoints"); bodyWriter.beginArray(); } @@ -334,9 +334,10 @@ public void writeEndpoint(final Endpoint endpoint) throws IOException { bodyWriter.endObject(); } - public void endEndpoints() throws IOException { + public void endEndpoints(final boolean first) throws IOException { bodyWriter.endArray(); - endMessageIfBatch(RequestType.ASM_ENDPOINTS); + bodyWriter.name("is_first").value(first); + endMessageIfBatch(RequestType.APP_ENDPOINTS); } public void writeInstallSignature(String installId, String installType, String installTime) diff --git a/telemetry/src/main/java/datadog/telemetry/api/RequestType.java b/telemetry/src/main/java/datadog/telemetry/api/RequestType.java index 81f96e29366..1ea613d9e90 100644 --- a/telemetry/src/main/java/datadog/telemetry/api/RequestType.java +++ b/telemetry/src/main/java/datadog/telemetry/api/RequestType.java @@ -13,7 +13,7 @@ public enum RequestType { LOGS("logs"), DISTRIBUTIONS("distributions"), MESSAGE_BATCH("message-batch"), - ASM_ENDPOINTS("asm-endpoints"); + APP_ENDPOINTS("app-endpoints"); private final String value; diff --git a/telemetry/src/test/groovy/datadog/telemetry/TelemetryServiceSpecification.groovy b/telemetry/src/test/groovy/datadog/telemetry/TelemetryServiceSpecification.groovy index 246dc3cbc10..a7be817d7e6 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/TelemetryServiceSpecification.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/TelemetryServiceSpecification.groovy @@ -17,8 +17,6 @@ import datadog.trace.api.telemetry.ProductChange import datadog.trace.test.util.DDSpecification import datadog.trace.util.Strings -import static datadog.trace.api.telemetry.Endpoint.Type.REST - class TelemetryServiceSpecification extends DDSpecification { def confKeyValue = ConfigSetting.of("confkey", "confvalue", ConfigOrigin.DEFAULT) def configuration = [confkey: confKeyValue] @@ -28,7 +26,7 @@ class TelemetryServiceSpecification extends DDSpecification { def distribution = new DistributionSeries().namespace("tracers").metric("distro").points([1, 2, 3]).tags(["tag1", "tag2"]).common(false) def logMessage = new LogMessage().message("log-message").tags("tag1:tag2").level(LogMessageLevel.DEBUG).stackTrace("stack-trace").tracerTime(32423).count(1) def productChange = new ProductChange().productType(ProductChange.ProductType.APPSEC).enabled(true) - def endpoint = new Endpoint().type('REST').method("GET").operation('http.request').path("/test") + def endpoint = new Endpoint().first(true).type('REST').method("GET").operation('http.request').path("/test") def 'happy path without data'() { setup: @@ -99,7 +97,7 @@ class TelemetryServiceSpecification extends DDSpecification { .assertNextMessage(RequestType.DISTRIBUTIONS).hasPayload().namespace("tracers").distributionSeries([distribution]) .assertNextMessage(RequestType.LOGS).hasPayload().logs([logMessage]) .assertNextMessage(RequestType.APP_PRODUCT_CHANGE).hasPayload().productChange(productChange) - .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint(endpoint) + .assertNextMessage(RequestType.APP_ENDPOINTS).hasPayload().endpoint(endpoint) .assertNoMoreMessages() testHttpClient.assertNoMoreRequests() @@ -163,7 +161,7 @@ class TelemetryServiceSpecification extends DDSpecification { .assertNextMessage(RequestType.DISTRIBUTIONS).hasPayload().namespace("tracers").distributionSeries([distribution]) .assertNextMessage(RequestType.LOGS).hasPayload().logs([logMessage]) .assertNextMessage(RequestType.APP_PRODUCT_CHANGE).hasPayload().productChange(productChange) - .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint(endpoint) + .assertNextMessage(RequestType.APP_ENDPOINTS).hasPayload().endpoint(endpoint) .assertNoMoreMessages() testHttpClient.assertNoMoreRequests() } @@ -251,7 +249,7 @@ class TelemetryServiceSpecification extends DDSpecification { .assertNextMessage(RequestType.DISTRIBUTIONS).hasPayload().namespace("tracers").distributionSeries([distribution]) .assertNextMessage(RequestType.LOGS).hasPayload().logs([logMessage]) .assertNextMessage(RequestType.APP_PRODUCT_CHANGE).hasPayload().productChange(productChange) - .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint(endpoint) + .assertNextMessage(RequestType.APP_ENDPOINTS).hasPayload().endpoint(endpoint) .assertNoMoreMessages() testHttpClient.assertNoMoreRequests() @@ -355,18 +353,7 @@ class TelemetryServiceSpecification extends DDSpecification { .assertNextMessage(RequestType.DISTRIBUTIONS).hasPayload().namespace("tracers").distributionSeries([distribution]) .assertNextMessage(RequestType.LOGS).hasPayload().logs([logMessage]) .assertNextMessage(RequestType.APP_PRODUCT_CHANGE).hasPayload().productChange(productChange) - .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint() // no endpoints fit in this request - .assertNoMoreMessages() - - when: 'sending third part of data' - testHttpClient.expectRequest(TelemetryClient.Result.SUCCESS) - !telemetryService.sendTelemetryEvents() - - then: - testHttpClient.assertRequestBody(RequestType.MESSAGE_BATCH) - .assertBatch(2) - .assertFirstMessage(RequestType.APP_HEARTBEAT).hasNoPayload() - .assertNextMessage(RequestType.ASM_ENDPOINTS).hasPayload().endpoint(endpoint) + .assertNextMessage(RequestType.APP_ENDPOINTS).hasPayload().endpoint(endpoint) .assertNoMoreMessages() testHttpClient.assertNoMoreRequests() } From d6bc2d8dddd78710245f89729ce86e3a0236f18f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Tue, 25 Mar 2025 14:55:51 +0100 Subject: [PATCH 3/9] Disable endpoint collection by default --- .../src/main/java/datadog/trace/api/ConfigDefaults.java | 3 ++- internal-api/src/main/java/datadog/trace/api/Config.java | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index 15beae2903a..230ec4efd54 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -115,7 +115,8 @@ public final class ConfigDefaults { static final int DEFAULT_APPSEC_WAF_TIMEOUT = 100000; // 0.1 s static final boolean DEFAULT_API_SECURITY_ENABLED = false; static final float DEFAULT_API_SECURITY_SAMPLE_DELAY = 30.0f; - static final boolean DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_ENABLED = true; + // TODO: change to true once the RFC is approved + static final boolean DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_ENABLED = false; static final int DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_MESSAGE_LIMIT = 300; static final boolean DEFAULT_APPSEC_RASP_ENABLED = true; static final boolean DEFAULT_APPSEC_STACK_TRACE_ENABLED = true; diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 5354c90063b..18759cd9138 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -1392,8 +1392,7 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) apiSecurityEndpointCollectionEnabled = configProvider.getBoolean( API_SECURITY_ENDPOINT_COLLECTION_ENABLED, - getAppSecActivation() != ProductActivation.FULLY_DISABLED - && DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_ENABLED); + DEFAULT_API_SECURITY_ENDPOINT_COLLECTION_ENABLED); apiSecurityEndpointCollectionMessageLimit = configProvider.getInteger( API_SECURITY_ENDPOINT_COLLECTION_MESSAGE_LIMIT, From 0e5386db9fd55f3b8db7f9cc6b06b4a1354c41b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Tue, 25 Mar 2025 15:02:01 +0100 Subject: [PATCH 4/9] Make better use of config options --- ...ppSecDispatcherServletInstrumentation.java | 25 +++++++++++-------- ...ervletWithPathPatternsInstrumentation.java | 25 +++++++++++-------- .../datadog/telemetry/TelemetryRequest.java | 4 ++- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java index 92adedccbc4..8f90c54ecf6 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java @@ -57,22 +57,25 @@ public void methodAdvice(MethodTransformer transformer) { AppSecDispatcherServletInstrumentation.class.getName() + "$AppSecHandlerMappingAdvice"); } + @Override + public boolean isEnabled() { + return super.isEnabled() && Config.get().isApiSecurityEndpointCollectionEnabled(); + } + public static class AppSecHandlerMappingAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void afterRefresh(@Advice.Argument(0) final ApplicationContext springCtx) { - if (Config.get().isApiSecurityEndpointCollectionEnabled()) { - final RequestMappingHandlerMapping handler = - springCtx.getBean(RequestMappingHandlerMapping.class); - if (handler == null) { - return; - } - final Map mappings = handler.getHandlerMethods(); - if (mappings == null || mappings.isEmpty()) { - return; - } - EndpointCollector.get().supplier(new RequestMappingInfoIterator(mappings)); + final RequestMappingHandlerMapping handler = + springCtx.getBean(RequestMappingHandlerMapping.class); + if (handler == null) { + return; + } + final Map mappings = handler.getHandlerMethods(); + if (mappings == null || mappings.isEmpty()) { + return; } + EndpointCollector.get().supplier(new RequestMappingInfoIterator(mappings)); } } } diff --git a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletWithPathPatternsInstrumentation.java b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletWithPathPatternsInstrumentation.java index e78b5bd7c0a..673ba9e8521 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletWithPathPatternsInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletWithPathPatternsInstrumentation.java @@ -57,22 +57,25 @@ public void methodAdvice(MethodTransformer transformer) { + "$AppSecHandlerMappingAdvice"); } + @Override + public boolean isEnabled() { + return super.isEnabled() && Config.get().isApiSecurityEndpointCollectionEnabled(); + } + public static class AppSecHandlerMappingAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void afterRefresh(@Advice.Argument(0) final ApplicationContext springCtx) { - if (Config.get().isApiSecurityEndpointCollectionEnabled()) { - final RequestMappingHandlerMapping handler = - springCtx.getBean(RequestMappingHandlerMapping.class); - if (handler == null) { - return; - } - final Map mappings = handler.getHandlerMethods(); - if (mappings == null || mappings.isEmpty()) { - return; - } - EndpointCollector.get().supplier(new RequestMappingInfoWithPathPatternsIterator(mappings)); + final RequestMappingHandlerMapping handler = + springCtx.getBean(RequestMappingHandlerMapping.class); + if (handler == null) { + return; + } + final Map mappings = handler.getHandlerMethods(); + if (mappings == null || mappings.isEmpty()) { + return; } + EndpointCollector.get().supplier(new RequestMappingInfoWithPathPatternsIterator(mappings)); } } } diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java b/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java index 7e4efefb4bc..0b4360b9f76 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetryRequest.java @@ -235,8 +235,10 @@ public void writeEndpoints() { log.debug("Writing endpoints"); requestBody.beginEndpoints(); boolean first = false; - while (eventSource.hasEndpoint()) { + int remaining = Config.get().getApiSecurityEndpointCollectionMessageLimit(); + while (eventSource.hasEndpoint() && remaining > 0) { final Endpoint event = eventSource.nextEndpoint(); + remaining--; if (event.isFirst()) { first = true; } From 642dd58bf96be3759f6ee760cea8ccc7679f14bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Wed, 26 Mar 2025 17:07:38 +0100 Subject: [PATCH 5/9] Fix failing tests --- .../EndpointCollectorSpringBootTest.groovy | 6 +++ .../agent/test/base/HttpServerTest.groovy | 3 ++ internal-api/build.gradle | 2 + .../datadog/trace/api/telemetry/Endpoint.java | 13 ------- .../telemetry/EndpointCollectorTest.groovy | 37 +++++++++++++++++++ .../endpoint/EndpointPeriodicAction.java | 12 +++++- .../EndpointPeriodicActionTest.groovy | 25 +++++++++++++ 7 files changed, 84 insertions(+), 14 deletions(-) create mode 100644 internal-api/src/test/groovy/datadog/trace/api/telemetry/EndpointCollectorTest.groovy create mode 100644 telemetry/src/test/groovy/datadog/telemetry/endpoint/EndpointPeriodicActionTest.groovy diff --git a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/test/groovy/test/boot/EndpointCollectorSpringBootTest.groovy b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/test/groovy/test/boot/EndpointCollectorSpringBootTest.groovy index 34013616182..bb771b0eb29 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-5.3/src/test/groovy/test/boot/EndpointCollectorSpringBootTest.groovy +++ b/dd-java-agent/instrumentation/spring-webmvc-5.3/src/test/groovy/test/boot/EndpointCollectorSpringBootTest.groovy @@ -15,6 +15,7 @@ import org.springframework.web.bind.annotation.RequestMethod import org.springframework.web.servlet.config.annotation.EnableWebMvc import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ENDPOINT_DISCOVERY +import static datadog.trace.api.config.AppSecConfig.API_SECURITY_ENDPOINT_COLLECTION_ENABLED @SpringBootTest(classes = DiscoveryController) @EnableWebMvc @@ -35,6 +36,11 @@ class EndpointCollectorSpringBootTest extends AgentTestRunner { } } + @Override + protected void configurePreAgent() { + injectSysConfig(API_SECURITY_ENDPOINT_COLLECTION_ENABLED, "true") + } + void 'test endpoint discovery'() { when: final endpoints = EndpointCollector.get().drain().toList().findAll { it.path == ENDPOINT_DISCOVERY.path } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 03e6a060b81..529a41510e7 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -84,6 +84,7 @@ import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.USER_B import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.WEBSOCKET import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace +import static datadog.trace.api.config.AppSecConfig.API_SECURITY_ENDPOINT_COLLECTION_ENABLED import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_QUERY_STRING import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_RESOURCE import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_TAG_QUERY_STRING @@ -163,6 +164,8 @@ abstract class HttpServerTest extends WithHttpServer { injectSysConfig(REQUEST_HEADER_TAGS, 'x-datadog-test-request-header:request_header_tag') // We don't inject a matching response header tag here since it would be always on and show up in all the tests injectSysConfig(TRACE_WEBSOCKET_MESSAGES_ENABLED, "true") + // allow endpoint discover for the tests + injectSysConfig(API_SECURITY_ENDPOINT_COLLECTION_ENABLED, "true") } // used in blocking tests to check if the handler was skipped diff --git a/internal-api/build.gradle b/internal-api/build.gradle index 0cd1ad54a37..edaaf1d860f 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -197,6 +197,8 @@ excludedClassesCoverage += [ // POJO 'datadog.trace.api.iast.util.Cookie', 'datadog.trace.api.iast.util.Cookie.Builder', + 'datadog.trace.api.telemetry.Endpoint', + 'datadog.trace.api.telemetry.Endpoint.Method', 'datadog.trace.api.telemetry.LogCollector.RawLogMessage', "datadog.trace.api.telemetry.MetricCollector.DistributionSeriesPoint", "datadog.trace.api.telemetry.MetricCollector", diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java b/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java index 96eacc62270..044e8fdca44 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/Endpoint.java @@ -149,17 +149,4 @@ static , C extends Collection> List parseMethods(fi return result; } } - - public interface Authentication { - String JWT = "JWT"; - String BASIC = "basic"; - String OAUTH = "oauth"; - String OIDC = "OIDC"; - String API_KEY = "api_key"; - String SESSION = "session"; - String M_TLS = "mTLS"; - String SAML = "SAML"; - String FORM = "Form"; - String OTHER = "other"; - } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/telemetry/EndpointCollectorTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/telemetry/EndpointCollectorTest.groovy new file mode 100644 index 00000000000..f905db7f390 --- /dev/null +++ b/internal-api/src/test/groovy/datadog/trace/api/telemetry/EndpointCollectorTest.groovy @@ -0,0 +1,37 @@ +package datadog.trace.api.telemetry + +import datadog.trace.test.util.DDSpecification + +class EndpointCollectorTest extends DDSpecification { + + void 'no metrics - drain empty list'() { + when: + final list = EndpointCollector.get().drain() + + then: + !list.hasNext() + } + + void 'set iterator'() { + setup: + final expected = (0..10).collect { index -> + new Endpoint() + .first(index == 0) + .type(Endpoint.Type.REST) + .operation(Endpoint.Operation.HTTP_REQUEST) + .path("/$index") + .requestBodyType(['application/json']) + .responseBodyType(['plain/text']) + .responseCode([200]) + .authentication(['JWT']) + .method(Endpoint.Method.METHODS[index % Endpoint.Method.METHODS.size()]) + } + EndpointCollector.get().supplier(expected.iterator()) + + when: + final received = EndpointCollector.get().drain().toList() + + then: + received == expected + } +} diff --git a/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java index 91bbea44507..8a7ae1dea45 100644 --- a/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java @@ -8,9 +8,19 @@ public class EndpointPeriodicAction implements TelemetryRunnable.TelemetryPeriodicAction { + private final EndpointCollector collector; + + public EndpointPeriodicAction() { + this(EndpointCollector.get()); + } + + public EndpointPeriodicAction(EndpointCollector collector) { + this.collector = collector; + } + @Override public void doIteration(final TelemetryService service) { - for (final Iterator it = EndpointCollector.get().drain(); it.hasNext(); ) { + for (final Iterator it = collector.drain(); it.hasNext(); ) { if (!service.addEndpoint(it.next())) { break; } diff --git a/telemetry/src/test/groovy/datadog/telemetry/endpoint/EndpointPeriodicActionTest.groovy b/telemetry/src/test/groovy/datadog/telemetry/endpoint/EndpointPeriodicActionTest.groovy new file mode 100644 index 00000000000..c2751d3f879 --- /dev/null +++ b/telemetry/src/test/groovy/datadog/telemetry/endpoint/EndpointPeriodicActionTest.groovy @@ -0,0 +1,25 @@ +package datadog.telemetry.endpoint + +import datadog.telemetry.TelemetryService +import datadog.trace.api.telemetry.Endpoint +import datadog.trace.api.telemetry.EndpointCollector +import spock.lang.Specification + +class EndpointPeriodicActionTest extends Specification { + + void 'test that common metrics are joined before being sent to telemetry #iterationIndex'() { + given: + final endpoint = new Endpoint().first(true).type('REST').method("GET").operation('http.request').path("/test") + final service = Mock(TelemetryService) + final endpointCollector = Mock(EndpointCollector) + final action = new EndpointPeriodicAction(endpointCollector) + + when: + action.doIteration(service) + + then: + 1 * endpointCollector.drain() >> [endpoint].iterator() + 1 * service.addEndpoint(endpoint) + 0 * _ + } +} From 83a89c4aba7543cace4580a450125f09bc8a5f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Wed, 26 Mar 2025 20:14:00 +0100 Subject: [PATCH 6/9] Fix muzzle directive --- .../instrumentation/spring-webmvc-3.1/build.gradle | 14 ++++++++++++++ .../AppSecDispatcherServletInstrumentation.java | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/build.gradle b/dd-java-agent/instrumentation/spring-webmvc-3.1/build.gradle index e52c9d5c722..0fb7ea9927d 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/build.gradle +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/build.gradle @@ -34,6 +34,20 @@ muzzle { extraDependency "javax.servlet:javax.servlet-api:3.0.1" extraDependency "org.springframework:spring-webmvc:3.1.0.RELEASE" } + + pass { + name = 'spring-mvc-pre-5.3' + group = 'org.springframework' + module = 'spring-webmvc' + versions = "[3.1.0.RELEASE,5.3)" + skipVersions += [ + '1.2.1', + '1.2.2', + '1.2.3', + '1.2.4'] // broken releases... missing dependencies + skipVersions += '3.2.1.RELEASE' // missing a required class. (bad release?) + extraDependency "javax.servlet:javax.servlet-api:3.0.1" + } } apply from: "$rootDir/gradle/java.gradle" diff --git a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java index 8f90c54ecf6..2283932dab2 100644 --- a/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java +++ b/dd-java-agent/instrumentation/spring-webmvc-3.1/src/main/java/datadog/trace/instrumentation/springweb/AppSecDispatcherServletInstrumentation.java @@ -41,6 +41,11 @@ public ElementMatcher.Junction classLoaderMatcher() { "org.springframework.web.servlet.mvc.condition.PathPatternsRequestCondition")); } + @Override + public String muzzleDirective() { + return "spring-mvc-pre-5.3"; + } + @Override public String[] helperClassNames() { return new String[] {packageName + ".RequestMappingInfoIterator"}; From 50c8ee363c46267fa96432a29c1dfc828f9e9659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Wed, 26 Mar 2025 23:30:08 +0100 Subject: [PATCH 7/9] Fix coverage --- .../endpoint/EndpointPeriodicAction.java | 30 +++++++++++++++- .../EndpointPeriodicActionTest.groovy | 35 +++++++++++++++++-- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java index 8a7ae1dea45..f555dfefb40 100644 --- a/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/endpoint/EndpointPeriodicAction.java @@ -21,9 +21,37 @@ public EndpointPeriodicAction(EndpointCollector collector) { @Override public void doIteration(final TelemetryService service) { for (final Iterator it = collector.drain(); it.hasNext(); ) { - if (!service.addEndpoint(it.next())) { + final Endpoint endpoint = it.next(); + if (!service.addEndpoint(endpoint)) { + collector.supplier(new HeadAndTailIterator(endpoint, it)); // try again latter break; } } } + + private static final class HeadAndTailIterator implements Iterator { + private final Endpoint head; + private final Iterator tail; + private boolean first; + + private HeadAndTailIterator(final Endpoint head, final Iterator tail) { + this.head = head; + this.tail = tail; + first = true; + } + + @Override + public boolean hasNext() { + return first || tail.hasNext(); + } + + @Override + public Endpoint next() { + if (first) { + first = false; + return head; + } + return tail.next(); + } + } } diff --git a/telemetry/src/test/groovy/datadog/telemetry/endpoint/EndpointPeriodicActionTest.groovy b/telemetry/src/test/groovy/datadog/telemetry/endpoint/EndpointPeriodicActionTest.groovy index c2751d3f879..34dc9402647 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/endpoint/EndpointPeriodicActionTest.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/endpoint/EndpointPeriodicActionTest.groovy @@ -7,19 +7,48 @@ import spock.lang.Specification class EndpointPeriodicActionTest extends Specification { - void 'test that common metrics are joined before being sent to telemetry #iterationIndex'() { + void 'test that endpoints are captured by the periodic action'() { given: final endpoint = new Endpoint().first(true).type('REST').method("GET").operation('http.request').path("/test") final service = Mock(TelemetryService) - final endpointCollector = Mock(EndpointCollector) + final endpointCollector = new EndpointCollector() + endpointCollector.supplier([endpoint].iterator()) final action = new EndpointPeriodicAction(endpointCollector) when: action.doIteration(service) then: - 1 * endpointCollector.drain() >> [endpoint].iterator() 1 * service.addEndpoint(endpoint) 0 * _ } + + void 'test that endpoints are not lost if the service is at capacity'() { + given: + final endpoints = [ + new Endpoint().first(true).type('REST').method("GET").operation('http.request').path("/test1"), + new Endpoint().type('REST').method("GET").operation('http.request').path("/test2"), + new Endpoint().type('REST').method("GET").operation('http.request').path("/test3"), + ] + final service = Mock(TelemetryService) + final endpointCollector = new EndpointCollector() + endpointCollector.supplier(endpoints.iterator()) + final action = new EndpointPeriodicAction(endpointCollector) + + when: + action.doIteration(service) + + then: + 1 * service.addEndpoint(endpoints[0]) >> true + 1 * service.addEndpoint(endpoints[1]) >> false + 0 * _ + + when: + action.doIteration(service) + + then: + 1 * service.addEndpoint(endpoints[1]) >> true + 1 * service.addEndpoint(endpoints[2]) >> true + 0 * _ + } } From bd769d9ca625d1c453136dbaf556a11cb17ea1b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Fri, 28 Mar 2025 12:07:40 +0100 Subject: [PATCH 8/9] Remove property --- .../src/main/java/datadog/trace/api/config/AppSecConfig.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java index d81d6cc1880..4feee469e75 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/AppSecConfig.java @@ -27,7 +27,6 @@ public final class AppSecConfig { public static final String API_SECURITY_ENABLED_EXPERIMENTAL = "experimental.api-security.enabled"; public static final String API_SECURITY_SAMPLE_DELAY = "api-security.sample.delay"; - public static final String API_SECURITY_REQUEST_SAMPLE_RATE = "api-security.request.sample.rate"; public static final String API_SECURITY_ENDPOINT_COLLECTION_ENABLED = "api-security.endpoint.collection.enabled"; public static final String API_SECURITY_ENDPOINT_COLLECTION_MESSAGE_LIMIT = From 5302575dc8a79e9981337e005475b4f3d593a78b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Fri, 28 Mar 2025 14:39:36 +0100 Subject: [PATCH 9/9] Update system tests commit --- .circleci/config.continue.yml.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.continue.yml.j2 b/.circleci/config.continue.yml.j2 index cfe77f45dc0..b563de669d9 100644 --- a/.circleci/config.continue.yml.j2 +++ b/.circleci/config.continue.yml.j2 @@ -36,7 +36,7 @@ instrumentation_modules: &instrumentation_modules "dd-java-agent/instrumentation debugger_modules: &debugger_modules "dd-java-agent/agent-debugger|dd-java-agent/agent-bootstrap|dd-java-agent/agent-builder|internal-api|communication|dd-trace-core" profiling_modules: &profiling_modules "dd-java-agent/agent-profiling" -default_system_tests_commit: &default_system_tests_commit 1ef00a34ad1f83ae999887e510ef1ea1c27b151b +default_system_tests_commit: &default_system_tests_commit 9be8b2793d687c7d9b39f3265fef27b5ec91910c parameters: nightly: