Skip to content

chore(lib-injection): change ssi code to use safe instrumentation configuration #13617

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

Merged
merged 118 commits into from
Jun 13, 2025

Conversation

wconti27
Copy link
Contributor

@wconti27 wconti27 commented Jun 6, 2025

Motivation

Updates SSI lib injection to only compare installed package versions against ddtrace dependencies. Removes any checks against unrelated libraries / integration libraries. Adds environment variable DD_TRACE_SAFE_INSTRUMENTATION_ENABLED=true which ensures we only patch compatible instrumentation packages.

Removes min_compatible_version.csv as well since it is no longer needed.

Changes:

  • The main SSI code that was updated is located in lib-injection/sources/sitecustomize.py.
  • min_compatible_versions.csv files were removed, since the SSI sitecustomize code no longer compares this table against the packages / versions installed on the injected application.
  • requirements.csv was added, which only includes the base ddtrace dependencies, and is compared against the application's installed packages, to ensure no conflicts. Any conflicts here result in library injection being aborted.
  • Adds a script to generate requirements.csv, and a workflow to check if the file would have changed, running on each PR and will require users to update the csv file if changes were detected.
  • Tests added:
    • Lib injection test suite that dynamically build a traced application, installing different sets of dependencies and asserting that certain dependencies were / were not patched dependent on their version and dd-trace-py's compatible versions for the relevant integration.

Checklist

  • PR author has checked that all the criteria below are met
  • The PR description includes an overview of the change
  • The PR description articulates the motivation for the change
  • The change includes tests OR the PR description describes a testing strategy
  • The PR description notes risks associated with the change, if any
  • Newly-added code is easy to change
  • The change follows the library release note guidelines
  • The change includes or references documentation updates if necessary
  • Backport labels are set (if applicable)

Reviewer Checklist

  • Reviewer has checked that all the criteria below are met
  • Title is accurate
  • All changes are related to the pull request's stated goal
  • Avoids breaking API changes
  • Testing strategy adequately addresses listed risks
  • Newly-added code is easy to change
  • Release note makes sense to a user of the library
  • If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment
  • Backport labels are set in a manner that is consistent with the release branch maintenance policy

@wconti27 wconti27 requested a review from a team as a code owner June 11, 2025 19:24
@wconti27 wconti27 requested a review from quinna-h June 11, 2025 19:24
@emmettbutler emmettbutler self-requested a review June 11, 2025 19:50
@wconti27 wconti27 enabled auto-merge (squash) June 13, 2025 17:25
…ation-patching-e2035b51fafb793b.yaml

Co-authored-by: wantsui <wan.tsui@datadoghq.com>
@wconti27 wconti27 merged commit 1f87a5e into main Jun 13, 2025
819 checks passed
@wconti27 wconti27 deleted the conti/improve-ssi-guardrails branch June 13, 2025 18:52
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.

3 participants