Conversation
Dockerfile
Outdated
| # for ddtrace.patch to discover instrumationation packages. | ||
| RUN find ./python/lib/$runtime/site-packages -name \*.py | grep -v ddtrace/contrib | xargs rm -rf | ||
| RUN find ./python/lib/$runtime/site-packages/ddtrace/contrib -name \*.py | grep -v __init__ | xargs rm -rf | ||
| # RUN find ./python/lib/$runtime/site-packages/ddtrace/contrib -name \*.py | grep -v __init__ | xargs rm -rf |
There was a problem hiding this comment.
Commenting out this line will have the impact of increasing the layer size significantly. We shouldn't do this.
There was a problem hiding this comment.
It will also have the impact of increasing cold start time.
There was a problem hiding this comment.
I'm aware of the related drawbacks. Just want to first commenting it out to see it's related as the integration tests passed.
The specific error complained about logging module. So maybe we just keep the logging's py files?
Dockerfile
Outdated
| RUN find ./python/lib/$runtime/site-packages/ddtrace/contrib -name \*.py | grep -v __init__ | xargs rm -rf | ||
| RUN find ./python/lib/$runtime/site-packages/ddtrace/contrib -name "*.py" \ | ||
| -not -path "./python/lib/$runtime/site-packages/ddtrace/contrib/internal/logging/*" \ | ||
| | grep -v __init__ | xargs rm -rf |
There was a problem hiding this comment.
Unfortunately, this isn't the right fix. To better understand the problem, take a look at this line change in this PR: https://github.com/DataDog/dd-trace-py/pull/12153/files#diff-6a70144455f9fea41df44b3e578cfaa81ae9c5297e41cce9ef02a5b719013817R265
pyproject.toml
Outdated
| datadog = ">=0.51.0,<1.0.0" | ||
| wrapt = "^1.11.2" | ||
| ddtrace = ">=2.20.0" | ||
| ddtrace = "==2.21.3" |
There was a problem hiding this comment.
Hmm, one concern here is that this change will also impact customers not using the layer.
I'm wondering if instead of changing the compatibility requirements here, we should just force an install of the specific ddtrace version in the Dockerfile.
There was a problem hiding this comment.
So basically, here
datadog-lambda-python/Dockerfile
Line 17 in b64be3b
RUN pip install . ddtrace==2.21.3 -t ./python/lib/$runtime/site-packagesThere was a problem hiding this comment.
Actually, I'm fine with however you wanna handle this @joeyzhao2018 since we're just gonna revert it back right away afterwards.
There was a problem hiding this comment.
The docker file is a better place i agree.
joeyzhao2018
force-pushed
the
release/ddtrace2.21.3
branch
from
0316755 to
3b35b0b
Compare
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| # Install datadog_lambda and dependencies from local | ||
| COPY . . | ||
| RUN pip install . -t ./python/lib/$runtime/site-packages | ||
| RUN pip install . ddtrace==2.21.3 -t ./python/lib/$runtime/site-packages |
There was a problem hiding this comment.
⚪ Code Quality Violation
| RUN pip install . ddtrace==2.21.3 -t ./python/lib/$runtime/site-packages | |
| RUN pip install --no-cache-dir . ddtrace==2.21.3 -t ./python/lib/$runtime/site-packages |
use --no-cache-dir to avoid caching (...read more)
This rule states that Dockerfiles should not use a cache when installing packages. When building Docker images, Docker has a built-in caching mechanism that reuses instructions from previous builds, which can speed up the build process. However, when installing packages, this can lead to outdated packages being used, which might have security vulnerabilities or bugs.
It is important to avoid using a cache when installing packages because it ensures that the latest version of a package is always used. This reduces the risk of security vulnerabilities and bugs, and ensures that your application has the most up-to-date and secure dependencies.
When installing packages with pip in a Dockerfile, use the --no-cache-dir option. This tells pip not to use a cache when installing packages, which ensures that the latest version of the package is always used. For example, instead of writing RUN pip install django, write RUN pip install --no-cache-dir django.
2 participants
What does this PR do?
Release with ddtrace@2.21.3
Motivation
Include a fix for payload tagging for dynamodb
Testing Guidelines
Additional Notes
Types of Changes
Check all that apply