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

[Debugger] Refactoring #3616

Merged
merged 13 commits into from
Dec 10, 2024
Merged

[Debugger] Refactoring #3616

merged 13 commits into from
Dec 10, 2024

Conversation

shurivich
Copy link
Contributor

@shurivich shurivich commented Dec 4, 2024

Motivation

Makes the debugger system tests easier to understand for new developers.
Fix bugs and ensures the tests work more reliably.

Changes

  1. Unified test flow
  • setup: prepare probes -> send RC -> wait for probes to be installed -> send library requests.
  • assertion: collect all relevant data (snapshots, spans, metrics) -> assert RC has no errors -> assert all probes are installed -> validate library responses -> perform scenario-specific validations.
  1. Expanded Data Collection
    Collecting all spans, snapshots, and metrics related to the debugger.

  2. Refactored Methods
    Moved and standardized "wait for" and "collect" methods to base utility, instead of test specific.

  3. Probe Enrichment Refactor
    Moved enrichment of probes fields (types, language, etc.) from the RC module to the debugger utility.

  4. Exception Replay

  • fixed inconsistencies in tests
  • disabled checks for "exception id" temporarily
  • removed one recursion test (will restore it later).
  1. Approval Assertion Enhancements
  • improved assertion mechanism for better coverage
  • moved read/write logic to utility - now reusable for other test scenarios.
  1. Merge all snapshot test scenarios into a single scenario (tests are still separated)

@shurivich shurivich self-assigned this Dec 4, 2024
@shurivich shurivich force-pushed the shurivich/improve_usability branch 2 times, most recently from 66c2b4d to c6b9d83 Compare December 4, 2024 10:26
@shurivich shurivich marked this pull request as ready for review December 5, 2024 12:38
@shurivich shurivich requested review from a team as code owners December 5, 2024 12:38
@shurivich shurivich requested review from emmettbutler, sabrenner, smola, jandro996 and dmehala and removed request for a team December 5, 2024 12:38
tests/debugger/test_debugger_pii.py Outdated Show resolved Hide resolved
Copy link

@shatzi shatzi left a comment

Choose a reason for hiding this comment

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

very nice!

tests/debugger/test_debugger_exception_replay.py Outdated Show resolved Hide resolved
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.

So far, ALGTM. You can rebase your branch to fix the issue in open telemetry.

I've also added some lint tules, so you may have to fix one or two lints after the rebase.

@shurivich shurivich force-pushed the shurivich/improve_usability branch from 7df28e5 to 7a64c5b Compare December 9, 2024 12:19
@shurivich
Copy link
Contributor Author

So far, ALGTM. You can rebase your branch to fix the issue in open telemetry.

I've also added some lint tules, so you may have to fix one or two lints after the rebase.

No conflicts, but still decided to rebase to double check everything is okay. It's huge PR, a lot of changes. Want to be sure nothing is going to be broken.

@shurivich shurivich merged commit 5b0f557 into main Dec 10, 2024
429 of 431 checks passed
@shurivich shurivich deleted the shurivich/improve_usability branch December 10, 2024 07:20
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.

5 participants