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 dynamic instrumentation probe manager + instrumentation integration tests #4097

Merged
merged 14 commits into from
Nov 12, 2024

Conversation

p-datadog
Copy link
Member

@p-datadog p-datadog commented Nov 11, 2024

What does this PR do?

  • Adds a probe manager class whose function is to maintain the list of probes received from remote config and their state (pending/instrumented/failed).
  • Probe manager drives Instrumenter (already in master) to perform instrumentation.
  • Probe manager keeps track of when instrumentation fails for reasons other than the target is not yet loaded, adds such probes to the list of failed probes and does not attempt to instrument them again.
  • Integration tests for instrumentation of the supported cases (method/line probes, basic/enriched, target is loaded/is loaded later) are included and comprise majority of the diff
  • DI Component is also part of this PR, it is needed to link Code Tracker and globally installed trace points for code loading and class definition to Probe Manager to install the pending probes

Motivation:

Initial DI implementation

Change log entry
None

Additional Notes:

This PR is extracted from #4063. It still does not enable DI for customers.

This PR is on top of #4088 and #4089, those PRs need to be merged first and then I will rebase this PR on master.

sig files are all stubs, I can edit them after all of the DI is in master but doing so currently causes churn that is not helping release DI on time.

How to test the change?
Unit and integration tests included.

@p-datadog p-datadog requested a review from a team as a code owner November 11, 2024 13:00
lib/datadog/di/serializer.rb Outdated Show resolved Hide resolved
spec/datadog/di/serializer_helper.rb Outdated Show resolved Hide resolved
spec/datadog/di/serializer_helper.rb Outdated Show resolved Hide resolved
lib/datadog/di/serializer.rb Outdated Show resolved Hide resolved
lib/datadog/di/code_tracker.rb Outdated Show resolved Hide resolved
spec/datadog/di/serializer_spec.rb Outdated Show resolved Hide resolved
lib/datadog/di/probe_manager.rb Outdated Show resolved Hide resolved
spec/datadog/di/serializer_spec.rb Outdated Show resolved Hide resolved
@pr-commenter
Copy link

pr-commenter bot commented Nov 11, 2024

Benchmarks

Benchmark execution time: 2024-11-12 16:49:05

Comparing candidate commit 2386443 in PR branch di-instrumentation with baseline commit e28cb05 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.28152% with 39 lines in your changes missing coverage. Please review.

Project coverage is 97.77%. Comparing base (54a779e) to head (2386443).
Report is 20 commits behind head on master.

Files with missing lines Patch % Lines
lib/datadog/di/probe_manager.rb 79.59% 20 Missing ⚠️
lib/datadog/di/component.rb 85.71% 8 Missing ⚠️
...pec/datadog/di/integration/instrumentation_spec.rb 98.20% 5 Missing ⚠️
spec/datadog/di/probe_manager_spec.rb 98.13% 3 Missing ⚠️
lib/datadog/di.rb 83.33% 2 Missing ⚠️
lib/datadog/di/code_tracker.rb 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4097      +/-   ##
==========================================
+ Coverage   97.74%   97.77%   +0.02%     
==========================================
  Files        1341     1350       +9     
  Lines       80356    81052     +696     
  Branches     4030     4081      +51     
==========================================
+ Hits        78541    79245     +704     
+ Misses       1815     1807       -8     

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

@p-datadog p-datadog merged commit 766bf02 into master Nov 12, 2024
262 of 263 checks passed
@p-datadog p-datadog deleted the di-instrumentation branch November 12, 2024 16:51
@github-actions github-actions bot added this to the 2.7.0 milestone Nov 12, 2024
@p-datadog p-datadog added the dev/internal Other internal work that does not need to be included in the changelog label Nov 13, 2024
@p-datadog p-datadog added 2.0 and removed 2.0 labels Nov 13, 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