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-2334 Ruby DI system tests #3516

Merged
merged 3 commits into from
Nov 21, 2024
Merged

DEBUG-2334 Ruby DI system tests #3516

merged 3 commits into from
Nov 21, 2024

Conversation

p-datadog
Copy link
Contributor

Motivation

Initial implementation of Dynamic Instrumentation in Ruby

Changes

  • Adds DebuggerController to rails70 weblog implementing log_probe, mix_probe and pii methods
  • Adds Ruby to the test code that substitutes actual controller names into placeholders
  • Marks tests that initial Ruby DI implementation does not implement as @missing_feature
  • Adds pii_line test, which is a modification of the pii test using a line probe since Ruby does not currently implement local variable capture for method probes (this requires using method entry/exit trace points). Alters the PII verification code to read snapshot from line or method location based on the test being executed

In order to run these tests, the changes in DataDog/dd-trace-rb#4098 are required which are pending debugging of failing non-DI tests.

Since DI is still not available in any released version of Ruby library, none of the manifests are changed in this PR.

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)?

@p-datadog p-datadog requested review from a team as code owners November 20, 2024 22:19
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.

Framework usage : all good

But you may need a review from someone familiar with the feature

@p-datadog p-datadog merged commit 86f272e into main Nov 21, 2024
398 of 399 checks passed
@p-datadog p-datadog deleted the ruby-di branch November 21, 2024 23:04
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.

4 participants