Skip to content

Release with [email protected] #565

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 4 commits into from
Mar 4, 2025
Merged

Release with [email protected] #565

merged 4 commits into from
Mar 4, 2025

Conversation

joeyzhao2018
Copy link
Contributor

@joeyzhao2018 joeyzhao2018 commented Feb 27, 2025

What does this PR do?

Release with [email protected]

Motivation

Include a fix for payload tagging for dynamodb

Testing Guidelines

Additional Notes

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

@joeyzhao2018 joeyzhao2018 requested a review from a team as a code owner February 27, 2025 15:06
Dockerfile Outdated
@@ -50,7 +50,7 @@ RUN PYTHONNODEBUGRANGES=1 python -OO -m compileall -b ./python/lib/$runtime/site
# remove all .py files except ddtrace/contrib/*/__init__.py which are necessary
# 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting out this line will have the impact of increasing the layer size significantly. We shouldn't do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will also have the impact of increasing cold start time.

Copy link
Contributor Author

@joeyzhao2018 joeyzhao2018 Feb 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@@ -28,7 +28,7 @@ classifiers = [
python = ">=3.8.0,<4"
datadog = ">=0.51.0,<1.0.0"
wrapt = "^1.11.2"
ddtrace = ">=2.20.0"
ddtrace = "==2.21.3"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically, here

RUN pip install . -t ./python/lib/$runtime/site-packages
, we'd change this to something like

RUN pip install . ddtrace==2.21.3 -t ./python/lib/$runtime/site-packages

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm fine with however you wanna handle this @joeyzhao2018 since we're just gonna revert it back right away afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docker file is a better place i agree.

@joeyzhao2018 joeyzhao2018 force-pushed the release/ddtrace2.21.3 branch from 0316755 to 3b35b0b Compare March 3, 2025 16:08
@@ -14,7 +14,7 @@ ENV PATH=/root/.cargo/bin:$PATH

# 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Quality Violation

Suggested change
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.

View in Datadog  Leave us feedback  Documentation

@joeyzhao2018 joeyzhao2018 merged commit fe3385f into main Mar 4, 2025
59 checks passed
@joeyzhao2018 joeyzhao2018 deleted the release/ddtrace2.21.3 branch March 4, 2025 18:47
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.

2 participants