Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DEBUG-3483] add: tests for probe budgets #3910

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tylfin
Copy link
Contributor

@tylfin tylfin commented Jan 28, 2025

Motivation

This pull request includes several changes to the debugger tests and utilities, focusing on method refactoring, adding new test probes, and enhancing test coverage for probe budgets. The most important changes include adding new probe definitions, refactoring method calls, and updating test setup and validation logic.

Changes

  • Added new probe definitions in tests/debugger/probes/probe_log_method_budgets.json and tests/debugger/probes/probe_snapshot_log_line_budgets.json to support logging and snapshot capturing for budget-related tests. [1] [2]
  • Updated _setup method in tests/debugger/test_debugger_probe_snapshot.py to include timing logic and line probe setup.
  • Added new test methods and validations for probe snapshots with budgets in tests/debugger/test_debugger_probe_snapshot.py.
  • Introduced method_and_language_to_line_number in tests/debugger/utils.py to centralize line number mappings.
  • Added a new endpoint /budgets/{loops} in utils/build/docker/java/spring-boot/src/main/java/com/datadoghq/system_tests/springboot/debugger/DebuggerController.java and utils/build/docker/python/flask/debugger_controller.py for budget-related tests. [1] [2]
  • Introduced a new feature marker debugger_probe_budgets in utils/_features.py to manage probe budget tests.

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes (if something not related to your task is failing, you can ignore it)
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner. We're working on refining the codeowners file quickly.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • CI is green, or failing jobs are not related to this change (and you are 100% sure about this statement)
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added (or removed)?

@tylfin tylfin force-pushed the tyler.finethy/debugger-probe-budgets branch 7 times, most recently from 825b521 to 2f52816 Compare February 3, 2025 18:43
@tylfin tylfin force-pushed the tyler.finethy/debugger-probe-budgets branch 7 times, most recently from 1264468 to 76183ee Compare February 10, 2025 16:43
@tylfin tylfin force-pushed the tyler.finethy/debugger-probe-budgets branch from 76183ee to 9e789dd Compare February 10, 2025 18:19
@tylfin tylfin changed the title add: tests for probe budgets [DEBUG-3483] add: tests for probe budgets Feb 10, 2025
@tylfin tylfin marked this pull request as ready for review February 11, 2025 13:32
@tylfin tylfin requested review from a team as code owners February 11, 2025 13:32
@tylfin tylfin requested review from nsrip-dd, erikayasuda, ygree, ValentinZakharov and sezen-datadog and removed request for a team February 11, 2025 13:32
@tylfin tylfin requested review from shurivich and a team and removed request for ygree, ValentinZakharov and sezen-datadog February 11, 2025 13:32
@tylfin
Copy link
Contributor Author

tylfin commented Feb 11, 2025

@shurivich This is ready for review, there's some issues with the trigger probes for both Python and Java so that's currently disabled for all languages, but I'll re-enable when the problems are enabled. For now it gives us a rough idea that standard rate-limiting for enriched snapshots are working within a threshold.

@tylfin tylfin requested a review from cbeauchesne February 11, 2025 19:39
@tylfin tylfin force-pushed the tyler.finethy/debugger-probe-budgets branch from 70cb56c to f4eb484 Compare February 11, 2025 19:51
Copy link
Collaborator

@cbeauchesne cbeauchesne left a comment

Choose a reason for hiding this comment

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

Failing on CI (failure on java build step are not related, you can rebase your branch to fix them)

@tylfin tylfin force-pushed the tyler.finethy/debugger-probe-budgets branch 2 times, most recently from e3f59b7 to 97e8ca5 Compare February 13, 2025 14:00
fix: generalize line mapping and add java test
fix: filter non-spring-boot weblogs
fix: add session_id tag to get correct budgets
add: link to feature-parity dashboard

fix: check captures instead of presence of snapshot only
@tylfin tylfin force-pushed the tyler.finethy/debugger-probe-budgets branch from 97e8ca5 to b4dfb8b Compare February 13, 2025 18:25
@tylfin tylfin requested a review from cbeauchesne February 13, 2025 21:46
Copy link
Contributor

@shurivich shurivich left a comment

Choose a reason for hiding this comment

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

LGTM with some small comments.

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.

3 participants