Skip to content

Commit

Permalink
feat(onboarding): early exit conditions in lib-injection (#9323)
Browse files Browse the repository at this point in the history
This pull request adds "guardrails" to the "library injection" process.
These are early exit conditions from the instrumentation process
intended to avoid sending any traces when undefined behavior is likely.
The code makes this determination on the basis of software versions
present in the application environment, both of Python packages and the
Python runtime itself.

The biggest risk here is that instrumentation is disabled when it's not
intended to be. I think existing tests in `tests/lib-injection` cover
this pretty well. There's a new test added that verifies instrumentation
was cancelled when an unsupported package version is present.

Contains changes from #9418
Related RFC: "[RFC] One Step Guardrails"

## Checklist

- [x] minimum package version checks
- [x] Testing
- [x] replace envvars with inject_force
- [x] figure out what to use instead of pkg_resources
- [x] replace local file path with `DD_TELEMETRY_FORWARDER_PATH`
- [x] Change(s) are motivated and described in the PR description
- [x] Testing strategy is described if automated tests are not included
in the PR
- [x] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

---------

Co-authored-by: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
Co-authored-by: Emmett Butler <emmett.butler321@gmail.com>
(cherry picked from commit 0c38e09)
  • Loading branch information
ZStriker19 authored and github-actions[bot] committed Jun 11, 2024
1 parent 360b469 commit 34bdd36
Show file tree
Hide file tree
Showing 17 changed files with 851 additions and 42 deletions.
53 changes: 51 additions & 2 deletions .github/workflows/lib-injection.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:
repository: 'DataDog/system-tests'

- name: Install runner
uses: ./.github/actions/install_runner
uses: ./.github/actions/install_runner

- name: Run K8s Lib Injection Tests
run: ./run.sh K8S_LIB_INJECTION_BASIC
Expand Down Expand Up @@ -79,7 +79,7 @@ jobs:
'dd-lib-python-init-test-django-uvicorn',
'dd-lib-python-init-test-django-no-perms',
'dd-lib-python-init-test-django-pre-installed',
'dd-lib-python-init-test-django-unsupported-python',
'dd-lib-python-init-test-django-unsupported-package-force',
]
fail-fast: false
steps:
Expand Down Expand Up @@ -125,3 +125,52 @@ jobs:
if: success() || failure()
run: |
docker compose logs
test_unit_no_instrumentation:
runs-on: ubuntu-latest
strategy:
matrix:
variant: [
'dd-lib-python-init-test-django-unsupported-python',
'dd-lib-python-init-test-django-unsupported-package',
]
fail-fast: false
steps:
- uses: actions/checkout@v4
- name: Build and run the app
run: |
SRC="$(pwd)"
cd lib-injection
export DDTRACE_PYTHON_VERSION="v2.6.3"
export APP_CONTEXT="${SRC}/tests/lib-injection/${{matrix.variant}}"
export TEMP_DIR="${SRC}/tmp/ddtrace"
mkdir -p "${TEMP_DIR}"
# Give the temp dir permissions, by default the docker user doesn't have permissions
# to write to the filesystem.
chmod 777 $TEMP_DIR
# Start the lib_inject to get the files copied. This avoids a race condition with the startup of the
# application.
docker compose up --build lib_inject
docker compose up --build -d
# Wait for the app to start
sleep 60
docker compose logs
- name: Check Permissions on ddtrace pkgs
run: |
cd lib-injection
# Ensure /datadog-lib/ddtrace_pkgs is a valid directory that is not empty
docker compose run lib_inject find /datadog-init/ddtrace_pkgs -maxdepth 0 -empty | wc -l && if [ $? -ne 0 ]; then exit 1; fi
# Ensure files are not world writeable
docker compose run lib_inject find /datadog-init/ddtrace_pkgs ! -perm /o+w | wc -l && if [ $? -ne 0 ]; then exit 1; fi
# Ensure all users have read and execute permissions to files stored in /datadog-lib/ddtrace_pkgs
docker compose run lib_inject find /datadog-init/ddtrace_pkgs ! -perm u=rwx,o=rx | wc -l && if [ $? -ne 0 ]; then exit 1; fi
- name: Test the app
run: |
curl http://localhost:18080
sleep 1 # wait for traces to be sent
- name: Print traces
run: curl http://localhost:8126/test/traces
- name: Check test agent received no trace
run: |
N=$(curl http://localhost:8126/test/traces | jq -r -e 'length')
[[ $N == "0" ]]
1 change: 1 addition & 0 deletions .gitlab/build-oci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ fi
echo -n $PYTHON_PACKAGE_VERSION > auto_inject-python.version
cp ../lib-injection/sitecustomize.py $BUILD_DIR/
cp auto_inject-python.version $BUILD_DIR/version
cp ../min_compatible_versions.csv $BUILD_DIR/
chmod -R o-w $BUILD_DIR
chmod -R g-w $BUILD_DIR

Expand Down
1 change: 1 addition & 0 deletions lib-injection/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@ RUN chown -R datadog:datadog /datadog-init/ddtrace_pkgs
RUN chmod -R 755 /datadog-init/ddtrace_pkgs
USER ${UID}
WORKDIR /datadog-init
ADD min_compatible_versions.csv /datadog-init/min_compatible_versions.csv
ADD sitecustomize.py /datadog-init/sitecustomize.py
ADD copy-lib.sh /datadog-init/copy-lib.sh
1 change: 1 addition & 0 deletions lib-injection/copy-lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
# This script is used by the admission controller to install the library from the
# init container into the application container.
cp sitecustomize.py "$1/sitecustomize.py"
cp min_compatible_versions.csv "$1/min_compatible_versions.csv"
cp -r ddtrace_pkgs "$1/ddtrace_pkgs"
2 changes: 2 additions & 0 deletions lib-injection/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ services:
environment:
- PYTHONPATH=/datadog-lib
- DD_TRACE_AGENT_URL=http://testagent:8126
- DD_TELEMETRY_FORWARDER_PATH=
volumes:
- ${TEMP_DIR:-/tmp/ddtrace_test}:/datadog-lib

Expand All @@ -45,5 +46,6 @@ services:
- PYTHONPATH=/datadog-lib
- DD_TRACE_AGENT_URL=http://testagent:8126
- DD_TRACE_DEBUG=1
- DD_TELEMETRY_FORWARDER_PATH=
volumes:
- ${TEMP_DIR:-/tmp/ddtrace_test}:/datadog-lib
188 changes: 188 additions & 0 deletions lib-injection/min_compatible_versions.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
This file was generated by scripts/min_compatible_versions.py
pkg_name,min_version
Flask-Cache,~=0.13.1
Jinja2,~=2.11.0
SQLAlchemy,==2.0.22
WebTest,0
Werkzeug,<1.0
ai21,0
aiobotocore,~=1.4.2
aiofiles,0
aiohttp,~=3.7
aiohttp_jinja2,~=1.5.0
aiomysql,~=0.1.0
aiopg,~=0.16.0
aiosqlite,0
algoliasearch,~=2.5
anyio,>=3.4.0
aredis,0
asgiref,~=3.0
astunparse,0
async_generator,~=1.10
asyncpg,~=0.22.0
asynctest,==0.13.0
attrs,>=20
austin-python,~=1.0
blinker,0
boto3,0
botocore,~=1.13
bottle,>=0.12
bytecode,0
cassandra-driver,~=3.24.0
cattrs,<23.1.1
celery,~=4.4
cfn-lint,~=0.53.1
channels,~=3.0
cherrypy,>=17
click,==7.1.2
cohere,==4.57
confluent-kafka,~=1.9.2
coverage,0
cryptography,<39
daphne,0
databases,0
datadog-lambda,>=4.66.0
ddsketch,>=3.0.0
django,>=2.2
django-pylibmc,>=0.6
django-q,0
django-redis,>=4.5
django_hosts,~=4.0
djangorestframework,>=3.11
docker,0
dogpile.cache,~=0.9
dramatiq,0
elasticsearch,~=7.13.0
elasticsearch1,~=1.10.0
elasticsearch2,~=2.5.0
elasticsearch5,~=5.5.0
elasticsearch6,~=6.8.0
elasticsearch7,~=7.13.0
elasticsearch7[async],0
elasticsearch8,~=8.0.1
elasticsearch[async],0
envier,==0.5.1
exceptiongroup,0
falcon,~=3.0
fastapi,~=0.64.0
flask,~=0.12.0
flask-caching,~=1.10.0
flask-login,~=0.6.2
gevent,~=20.12.0
git+https://github.com/gnufede/pytest-memray.git@24a3c0735db99eedf57fb36c573680f9bab7cd73,0
googleapis-common-protos,0
graphene,~=3.0.0
graphql-core,~=3.2.0
graphql-relay,0
greenlet,~=1.0.0
grpcio,~=1.34.0
gunicorn,==20.0.4
gunicorn[gevent],0
httpretty,<1.1
httpx,~=0.17.0
huggingface-hub,0
hypothesis,<6.45.1
importlib-metadata,0
importlib_metadata,<5.0
itsdangerous,<2.0
jinja2,~=2.11.0
kombu,>=4.2.0
langchain,==0.0.192
langchain-aws,0
langchain-community,==0.0.14
langchain-core,==0.1.52
langchain-openai,==0.1.6
langchain-pinecone,==0.1.0
langchain_experimental,==0.0.47
langsmith,==0.1.58
logbook,~=1.0.0
loguru,~=0.4.0
mako,~=1.1.0
mariadb,~=1.0.0
markupsafe,<2.0
mock,0
molten,>=1.0
mongoengine,~=0.23
more_itertools,<8.11.0
moto,>=1.0
moto[all],<5.0
msgpack,~=1.0.0
mysql-connector-python,==8.0.5
mysqlclient,~=2.0
numexpr,0
openai,==0.26.5
openai[datalib],==1.30.1
"openai[embeddings,datalib]",==0.27.2
opensearch-py,0
opensearch-py[async],0
opensearch-py[requests],~=1.1.0
opentelemetry-api,>=1
opentelemetry-instrumentation-flask,<=0.37b0
opentracing,>=2.0.0
peewee,0
pillow,0
pinecone-client,==2.2.4
pony,0
protobuf,>=3
psutil,0
psycopg,~=3.0.18
psycopg2-binary,~=2.8.0
py-cpuinfo,~=8.0.0
pycryptodome,0
pyfakefs,0
pylibmc,~=1.6.2
pymemcache,~=3.4.2
pymongo,~=3.11
pymysql,~=0.10
pynamodb,~=5.0
pyodbc,~=4.0.31
pyramid,~=1.10
pysqlite3-binary,0
pytest,~=4.0
pytest-aiohttp,0
pytest-asyncio,==0.21.1
pytest-bdd,>=4.0
pytest-benchmark,>=3.1.0
pytest-cov,==2.9.0
pytest-django,==3.10.0
pytest-mock,==2.0.0
pytest-randomly,0
pytest-sanic,~=1.6.2
python-consul,>=1.1
python-json-logger,==2.0.7
python-memcached,0
redis,~=2.0
redis-py-cluster,>=2.0
reno,0
requests,~=2.20.0
requests-mock,>=1.4
responses,~=0.16.0
rich,0
rq,~=1.8.0
ruamel.yaml,0
sanic,~=20.12
sanic-testing,~=0.8.3
scikit-learn,==1.0.2
simplejson,0
six,==1.12.0
snowflake-connector-python,~=2.3.0
sqlalchemy,~=1.2.18
starlette,~=0.14.0
structlog,~=20.2.0
tests/contrib/pyramid/pserve_app,0
tiktoken,0
tornado,~=4.5.0
tortoise-orm,0
typing-extensions,0
typing_extensions,0
urllib3,~=1.0
uwsgi,0
vcrpy,==4.2.1
vertica-python,>=0.6.0
websockets,<11.0
webtest,0
werkzeug,<1.0
wheel,0
xmltodict,>=0.12
yaaredis,~=2.0.0
yarl,~=1.0
Loading

0 comments on commit 34bdd36

Please sign in to comment.