-
Notifications
You must be signed in to change notification settings - Fork 413
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
chore(auto-inject): add package instrumentation skipping and denylist #9685
base: main
Are you sure you want to change the base?
chore(auto-inject): add package instrumentation skipping and denylist #9685
Conversation
…rced injection pkg test
Datadog ReportBranch report: ✅ 0 Failed, 8459 Passed, 14412 Skipped, 1h 58m 14.07s Total duration (6.17s time saved) |
…and_add_default_path
BenchmarksBenchmark execution time: 2024-11-08 20:25:29 Comparing candidate commit cf4069d in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 387 metrics, 2 unstable metrics. scenario:iast_aspects-replace_aspect
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9685 +/- ##
===========================================
- Coverage 73.93% 10.54% -63.39%
===========================================
Files 1402 1368 -34
Lines 130420 127984 -2436
===========================================
- Hits 96426 13502 -82924
- Misses 33994 114482 +80488 ☔ View full report in Codecov by Sentry. |
|
d5c32f4
to
fa0ec02
Compare
lib-injection/sitecustomize.py
Outdated
@@ -248,6 +261,11 @@ def _inject(): | |||
_log("site-packages path is %r" % site_pkgs_path, level="debug") | |||
if not os.path.exists(site_pkgs_path): | |||
_log("ddtrace site-packages not found in %r, aborting" % site_pkgs_path, level="error") | |||
telemetry_data.append( | |||
create_count_metric("library_entrypoint.error", ["error_type:" + "site-packages-not-found"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Code Quality Violation
use f-string or .format to format strings (...read more)
Concatenation of multiple strings is not efficient and make the code hard to read and understand.
Instead of concatenating multiple strings, use an f-string or a format string.
Learn More
…and_add_default_path
…and_add_default_path
…and_add_default_path
…and_add_default_path
…and_add_default_path
…and_add_default_path
@@ -296,6 +311,11 @@ def _inject(): | |||
_log("site-packages path is %r" % site_pkgs_path, level="debug") | |||
if not os.path.exists(site_pkgs_path): | |||
_log("ddtrace site-packages not found in %r, aborting" % site_pkgs_path, level="error") | |||
telemetry_data.append( | |||
create_count_metric("library_entrypoint.error", ["error_type:" + "site-packages-not-found"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Code Quality Violation
use f-string or .format to format strings (...read more)
Concatenation of multiple strings is not efficient and make the code hard to read and understand.
Instead of concatenating multiple strings, use an f-string or a format string.
Learn More
…and_add_default_path
@@ -296,6 +311,11 @@ def _inject(): | |||
_log("site-packages path is %r" % site_pkgs_path, level="debug") | |||
if not os.path.exists(site_pkgs_path): | |||
_log("ddtrace site-packages not found in %r, aborting" % site_pkgs_path, level="error") | |||
telemetry_data.append( | |||
create_count_metric("library_entrypoint.error", ["error_type:" + "site-packages-not-found"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Code Quality Violation
use f-string or .format to format strings (...read more)
Concatenation of multiple strings is not efficient and make the code hard to read and understand.
Instead of concatenating multiple strings, use an f-string or a format string.
Learn More
…and_add_default_path
…and_add_default_path
Currently the python tracer will abort instrumentation entirely if a package that's outside of the supported range is detected. Instead it should follow the behavior of other tracers and simply skip instrumenting that particular package, and then instrument the rest of the supported packages.
In addition, the Python tracer should skip instrumentation if uwsgi is detected since it's only compatible with uwsgi under specific conditions: https://ddtrace.readthedocs.io/en/stable/advanced_usage.html?highlight=uwsgi#uwsgi
Checklist
changelog/no-changelog
is set@DataDog/apm-tees
.Reviewer Checklist