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 DI internal settings reorg + telemetry + error handling #4089

Merged
merged 12 commits into from
Nov 12, 2024

Conversation

p-datadog
Copy link
Contributor

What does this PR do?
This PR contains part of #4063 that modifies files already in master.

  • Moves internal DI settings to "internal" subkey
  • Minimum send interval for the network submission worker moved to a new internal configuration setting from the constant
  • Injects telemetry object into DI components
  • Reports unhandled exceptions to telemetry in DI code already in master
  • Line probes now require a single match of the provided path suffix, if multiple paths match an exception is raised (this will be reported via UI to the user, on the assumption that user wanted a specific file instrumented and we don't know which of the multiple matching files is the correct one)
  • When probe installation fails for reasons other than the referenced target being missing (presumably not yet loaded), the failure is noted and that probe is not attempted again in the future (this PR contains part of the code for this change)
  • Improves probe notifier worker flush logic to be event-driven instead of sleeping

Motivation:

Test failures in #4063 are difficult to troubleshoot, this PR extracts code that can be merged without needing non-DI changes to help with the troubleshooting.

Change log entry

None

Additional Notes:

How to test the change?
Unit tests

@p-datadog p-datadog requested a review from a team as a code owner November 7, 2024 22:19
@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2024

Codecov Report

Attention: Patch coverage is 80.91603% with 25 lines in your changes missing coverage. Please review.

Project coverage is 97.79%. Comparing base (54a779e) to head (a7b7f11).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
lib/datadog/di/probe_notifier_worker.rb 68.42% 12 Missing ⚠️
lib/datadog/di/instrumenter.rb 56.25% 7 Missing ⚠️
lib/datadog/di/code_tracker.rb 55.55% 4 Missing ⚠️
lib/datadog/di/probe.rb 80.00% 1 Missing ⚠️
spec/datadog/di/spec_helper.rb 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4089      +/-   ##
==========================================
+ Coverage   97.74%   97.79%   +0.05%     
==========================================
  Files        1341     1341              
  Lines       80356    80408      +52     
  Branches     4030     4048      +18     
==========================================
+ Hits        78541    78637      +96     
+ Misses       1815     1771      -44     

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

@pr-commenter
Copy link

pr-commenter bot commented Nov 8, 2024

Benchmarks

Benchmark execution time: 2024-11-12 14:43:10

Comparing candidate commit a7b7f11 in PR branch di-settings-internal with baseline commit 18960b5 in branch master.

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

@p-datadog p-datadog merged commit e28cb05 into master Nov 12, 2024
262 of 263 checks passed
@p-datadog p-datadog deleted the di-settings-internal branch November 12, 2024 15:36
@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
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