Skip to content

resource renaming: fix failing resource renaming test for java#6444

Merged
florentinl merged 1 commit into
mainfrom
florentin.labelle/APPSEC-61406/fix-flaky-resource-renaming-test
Mar 6, 2026
Merged

resource renaming: fix failing resource renaming test for java#6444
florentinl merged 1 commit into
mainfrom
florentin.labelle/APPSEC-61406/fix-flaky-resource-renaming-test

Conversation

@florentinl
Copy link
Copy Markdown
Contributor

@florentinl florentinl commented Mar 6, 2026

Motivation

After monitoring the test for a few days, I noticed that it is still only failing for two Java flavors (play and ratpack).

The reason is that they create multiple web spans in their trace tree and stats points where double counted.

            {
              "Name": "akka-http.request",
              "Service": "weblog",
              "Resource": "GET /resource_renaming/api/users/{param:int}",
              "Type": "web",
              "HTTPStatusCode": 200,
              "Synthetics": false,
              "IsTraceRoot": 1,
              "SpanKind": "server",
              "HTTPMethod": "GET",
              "HTTPEndpoint": "/resource_renaming/api/users/{param:int}",
              "Hits": 5,
              "TopLevelHits": 5,
            },
            {
              "Name": "play.request",
              "Service": "weblog",
              "Resource": "GET /resource_renaming/api/users/{param:int}",
              "Type": "web",
              "HTTPStatusCode": 0,
              "Synthetics": false,
              "IsTraceRoot": 2,
              "SpanKind": "server",
              "HTTPMethod": "GET",
              "HTTPEndpoint": "/resource_renaming/api/users/{param:int}",
              "Hits": 5,
              "TopLevelHits": 0,
            },

Switching to TopLevelHits instead of Hits should fix the issue.

This allows us to enable the test again.

Changes

  • use TopLevelHits instead of Hits

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • Anything but tests/ or manifests/ is modified ? I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added, removed or renamed?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

CODEOWNERS have been resolved as:

manifests/java.yml                                                      @DataDog/asm-java @DataDog/apm-java
tests/test_resource_renaming.py                                         @DataDog/system-tests-core

@florentinl florentinl marked this pull request as ready for review March 6, 2026 10:28
@florentinl florentinl requested review from a team as code owners March 6, 2026 10:28
Copy link
Copy Markdown
Member

@jandro996 jandro996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks!

@florentinl florentinl merged commit 04175b6 into main Mar 6, 2026
436 checks passed
@florentinl florentinl deleted the florentin.labelle/APPSEC-61406/fix-flaky-resource-renaming-test branch March 6, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants