Fix HttpEndpointPostProcessor incorrectly overwriting span resource name (#10754)

Fix HttpEndpointPostProcessor incorrectly overwriting span resource name

  Per RFC-1051, the trace resource renaming feature is scoped to enriching
  APM stats buckets with HTTPMethod/HTTPEndpoint fields and tagging service
  entry spans with http.endpoint when computed from the URL. Resource name
  renaming on the span itself is the backend's responsibility.

  The HttpEndpointPostProcessor was calling setResourceName() with the
  inferred endpoint, which caused the top-level span's resource field to
  be overwritten in the agent — behaviour not specified by the RFC.

  Remove the setResourceName() call and the now-unused
  HTTP_SERVER_RESOURCE_RENAMING priority constant. Update tests to assert
  that the resource name is never modified and that http.endpoint is
  correctly set in the tags when the route is ineligible or absent.

Merge branch 'master' into alejandro.gonzalez/fix-resourceName-bug

WIP

Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>

Author: jandro996

dd-trace-core/src/main/java/datadog/trace/core/tagprocessor/HttpEndpointPostProcessor.java

@@ -6,32 +6,28 @@
 
 import datadog.trace.api.TagMap;
 import datadog.trace.api.endpoint.EndpointResolver;
-import datadog.trace.api.normalize.HttpResourceNames;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
-import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities;
 import datadog.trace.core.DDSpanContext;
 import java.util.List;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Post-processes HTTP spans to update resource names based on inferred endpoints.
+ * Post-processes HTTP spans to resolve and tag the {@code http.endpoint} for APM trace metrics.
  *
- * <p>This processor implements the trace resource renaming feature by:
+ * <p>Implements the endpoint inference strategy from RFC-1051: tags the span with {@code
+ * http.endpoint} (computed from the URL) so that stats buckets can be aggregated per endpoint. This
+ * processor does <em>not</em> overwrite the span's resource name — resource renaming is the
+ * backend's responsibility.
  *
- * <ul>
- *   <li>Using EndpointResolver to determine the best endpoint (route or simplified URL)
- *   <li>Combining HTTP method with the endpoint to create a resource name (e.g., "GET
- *       /users/{param:int}")
- *   <li>Updating the span's resource name only when an endpoint is available
- * </ul>
- *
- * <p>The processor respects the endpoint resolution logic:
+ * <p>Resolution logic:
  *
  * <ul>
- *   <li>If alwaysSimplifiedEndpoint=true, always compute from URL
- *   <li>If http.route exists and is eligible, use it
- *   <li>Otherwise, compute simplified endpoint from URL
+ *   <li>If {@code http.route} is eligible, use it as the endpoint (no {@code http.endpoint} tag
+ *       added)
+ *   <li>Otherwise, compute a simplified endpoint from {@code http.url} and tag the span with {@code
+ *       http.endpoint}
+ *   <li>If {@code alwaysSimplifiedEndpoint=true}, always compute from URL regardless of route
  * </ul>
  */
 public class HttpEndpointPostProcessor extends TagsPostProcessor {
@@ -66,47 +62,14 @@ public void processTags(
       return;
     }
 
-    // Extract HTTP tags
-    Object httpMethodObj = unsafeTags.get(HTTP_METHOD);
-    Object httpRouteObj = unsafeTags.get(HTTP_ROUTE);
-    Object httpUrlObj = unsafeTags.get(HTTP_URL);
-
-    log.debug(
-        "Processing tags for span {}: httpMethod={}, httpRoute={}, httpUrl={}",
-        spanContext.getSpanId(),
-        httpMethodObj,
-        httpRouteObj,
-        httpUrlObj);
-
-    if (httpMethodObj == null) {
-      // Not an HTTP span, skip processing
-      log.debug("No HTTP method found, skipping HTTP endpoint post processing");
+    if (unsafeTags.getObject(HTTP_METHOD) == null) {
       return;
     }
 
     try {
-      String httpMethod = httpMethodObj.toString();
-      String httpRoute = httpRouteObj != null ? httpRouteObj.toString() : null;
-      String httpUrl = httpUrlObj != null ? httpUrlObj.toString() : null;
-
-      // Resolve endpoint using EndpointResolver
-      // Pass unsafeTags directly - it's safe to use at this point since span is finished
-      String endpoint = endpointResolver.resolveEndpoint(unsafeTags, httpRoute, httpUrl);
-
-      if (endpoint != null && !endpoint.isEmpty()) {
-        // Combine method and endpoint into resource name using cached join
-        CharSequence resourceName = HttpResourceNames.join(httpMethod, endpoint);
-        spanContext.setResourceName(
-            resourceName, ResourceNamePriorities.HTTP_SERVER_RESOURCE_RENAMING);
-
-        log.debug(
-            "Updated resource name to '{}' for span {} (method={}, route={}, url={})",
-            resourceName,
-            spanContext.getSpanId(),
-            httpMethod,
-            httpRoute,
-            httpUrl);
-      }
+      String httpRoute = unsafeTags.getString(HTTP_ROUTE);
+      String httpUrl = unsafeTags.getString(HTTP_URL);
+      endpointResolver.resolveEndpoint(unsafeTags, httpRoute, httpUrl);
     } catch (Throwable t) {
       log.debug("Error processing HTTP endpoint for span {}", spanContext.getSpanId(), t);
     }

dd-trace-core/src/test/groovy/datadog/trace/core/tagprocessor/HttpEndpointPostProcessorTest.groovy

@@ -1,47 +1,52 @@
 package datadog.trace.core.tagprocessor
 
-import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities
+import datadog.trace.api.TagMap
 import datadog.trace.bootstrap.instrumentation.api.Tags
 import datadog.trace.core.DDSpanContext
 import datadog.trace.api.endpoint.EndpointResolver
 import spock.lang.Specification
 
 class HttpEndpointPostProcessorTest extends Specification {
 
-  def "should update resource name with endpoint when enabled"() {
+  def "should not overwrite resource name when http.route is available and eligible"() {
+    // RFC-1051: the processor enriches stats buckets and tags the span with http.endpoint,
+    // but must NOT overwrite the span's resourceName — that is the backend's responsibility.
     given:
     def endpointResolver = new EndpointResolver(true, false)
     def processor = new HttpEndpointPostProcessor(endpointResolver)
     def mockContext = Mock(DDSpanContext)
-    def tags = [
+    def tags = TagMap.fromMap([
       (Tags.HTTP_METHOD): "GET",
       (Tags.HTTP_ROUTE): "/greeting",
       (Tags.HTTP_URL): "http://localhost:8080/greeting"
-    ]
+    ])
 
     when:
     processor.processTags(tags, mockContext, [])
 
     then:
-    1 * mockContext.setResourceName({ it.toString() == "GET /greeting" }, ResourceNamePriorities.HTTP_SERVER_RESOURCE_RENAMING)
+    0 * mockContext.setResourceName(_, _)
+    // http.route is eligible — no http.endpoint tag should be added
+    !tags.containsKey(Tags.HTTP_ENDPOINT)
   }
 
-  def "should compute simplified endpoint from URL when route is invalid"() {
+  def "should compute and tag http.endpoint from URL when route is invalid, without touching resourceName"() {
     given:
     def endpointResolver = new EndpointResolver(true, false)
     def processor = new HttpEndpointPostProcessor(endpointResolver)
     def mockContext = Mock(DDSpanContext)
-    def tags = [
+    def tags = TagMap.fromMap([
       (Tags.HTTP_METHOD): "GET",
-      (Tags.HTTP_ROUTE): "*",  // Invalid route
+      (Tags.HTTP_ROUTE): "*",  // catch-all — ineligible per RFC-1051
       (Tags.HTTP_URL): "http://localhost:8080/users/123/orders/456"
-    ]
+    ])
 
     when:
     processor.processTags(tags, mockContext, [])
 
     then:
-    1 * mockContext.setResourceName({ it.toString() == "GET /users/{param:int}/orders/{param:int}" }, ResourceNamePriorities.HTTP_SERVER_RESOURCE_RENAMING)
+    0 * mockContext.setResourceName(_, _)
+    tags[Tags.HTTP_ENDPOINT] == "/users/{param:int}/orders/{param:int}"
   }
 
   def "should skip non-HTTP spans"() {
@@ -58,6 +63,7 @@ class HttpEndpointPostProcessorTest extends Specification {
 
     then:
     0 * mockContext.setResourceName(_, _)
+    !tags.containsKey(Tags.HTTP_ENDPOINT)
   }
 
   def "should not process when resource renaming is disabled"() {
@@ -75,23 +81,25 @@ class HttpEndpointPostProcessorTest extends Specification {
 
     then:
     0 * mockContext.setResourceName(_, _)
+    !tags.containsKey(Tags.HTTP_ENDPOINT)
   }
 
-  def "should use simplified endpoint when alwaysSimplified is true"() {
+  def "should tag http.endpoint from URL even when alwaysSimplified is true, without touching resourceName"() {
     given:
     def endpointResolver = new EndpointResolver(true, true)
     def processor = new HttpEndpointPostProcessor(endpointResolver)
     def mockContext = Mock(DDSpanContext)
-    def tags = [
+    def tags = TagMap.fromMap([
       (Tags.HTTP_METHOD): "GET",
       (Tags.HTTP_ROUTE): "/greeting",
       (Tags.HTTP_URL): "http://localhost:8080/users/123"
-    ]
+    ])
 
     when:
     processor.processTags(tags, mockContext, [])
 
     then:
-    1 * mockContext.setResourceName({ it.toString() == "GET /users/{param:int}" }, ResourceNamePriorities.HTTP_SERVER_RESOURCE_RENAMING)
+    0 * mockContext.setResourceName(_, _)
+    tags[Tags.HTTP_ENDPOINT] == "/users/{param:int}"
   }
 }

internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/ResourceNamePriorities.java

@@ -6,7 +6,6 @@ public class ResourceNamePriorities {
   public static final byte HTTP_404 = 2;
   public static final byte HTTP_FRAMEWORK_ROUTE = 3;
   public static final byte RPC_COMMAND_NAME = 3;
-  public static final byte HTTP_SERVER_RESOURCE_RENAMING = 3;
   public static final byte HTTP_SERVER_CONFIG_PATTERN_MATCH = 4;
   public static final byte HTTP_CLIENT_CONFIG_PATTERN_MATCH = 4;
   public static final byte TAG_INTERCEPTOR = 5;