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 Use :return/:b_return trace points to target 'end' line of methods and blocks with line probes #4109

Merged
merged 5 commits into from
Nov 14, 2024

Conversation

p-datadog
Copy link
Member

What does this PR do?
This PR adds return and b_return trace point types to the line trace point, permitting targeting end lines of blocks and methods with line probes.

Motivation:
Initial DI implementation for Ruby.

Change log entry
None

Additional Notes:
The return trace point types have the unfortunate side effect of always installing even if the target line has no executable ruby code (e.g. a comment line). This means users will not be getting notified when they try to instrument non-executable code lines. I think it is worthwhile to permit instrumenting end lines even if it means no diagnostics when trying to instrument non-instrumentable lines, in other words, it is better to make more use cases work at the expense of worse diagnostics when something that wouldn't work is attempted.

How to test the change?
Integration tests are included.

@p-datadog p-datadog requested a review from a team as a code owner November 13, 2024 15:43
@p-datadog p-datadog changed the title DEBUG-2334 Use :return trace point to target 'end' line of methods with line probes DEBUG-2334 Use :return/:b_return trace points to target 'end' line of methods and blocks with line probes Nov 13, 2024
@p-datadog p-datadog force-pushed the di-end-instrumentation branch from 8c0d362 to dd7a7dd Compare November 13, 2024 15:44
@pr-commenter
Copy link

pr-commenter bot commented Nov 13, 2024

Benchmarks

Benchmark execution time: 2024-11-13 20:26:43

Comparing candidate commit da4b985 in PR branch di-end-instrumentation with baseline commit 34d1947 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 29 metrics, 2 unstable metrics.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 94.17476% with 6 lines in your changes missing coverage. Please review.

Project coverage is 97.77%. Comparing base (7c519ab) to head (da4b985).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...egration/instrumentation_integration_test_class.rb 57.14% 3 Missing ⚠️
...pec/datadog/di/integration/instrumentation_spec.rb 97.80% 2 Missing ⚠️
lib/datadog/di/instrumenter.rb 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4109      +/-   ##
==========================================
- Coverage   97.77%   97.77%   -0.01%     
==========================================
  Files        1350     1350              
  Lines       81079    81150      +71     
  Branches     4085     4087       +2     
==========================================
+ Hits        79273    79342      +69     
- Misses       1806     1808       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@p-datadog p-datadog merged commit 6f4db56 into master Nov 14, 2024
262 of 263 checks passed
@p-datadog p-datadog deleted the di-end-instrumentation branch November 14, 2024 17:46
@github-actions github-actions bot added this to the 2.8.0 milestone Nov 14, 2024
p-datadog pushed a commit that referenced this pull request Nov 15, 2024
* origin/di-virtual: (30 commits)
  DEBUG-2334 Ruby 2 compatibility for dynamic instrumentation (#4120)
  Fix CI flakiness by adding dockerhub login
  Update filter with splitted  gemfile and task
  [🤖] Lock Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/11846139851
  Lock sqlite3
  Lock ActiveRecord
  Fix deprecation behavior
  Fix trilogy/patcher_spec
  Update 3.4_relational_db.gemfile
  Update Ruby 3.4 Gemfile
  Add 3.4 to circleci
  DEBUG-2334 Use :return/:b_return trace points to target 'end' line of methods and blocks with line probes (#4109)
  Temporarily disable asan ruby job
  Update developement guide
  Lint
  Fix release_gem_spec.rb
  Gemfile migration
  Update usage
  Appraisal group generation replacement
  DEBUG-2334 remove unused method stubs from DI test suite (#4107)
  ...
@p-datadog p-datadog added the dev/internal Other internal work that does not need to be included in the changelog label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/internal Other internal work that does not need to be included in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants