From 2b67f9f3118bfe2075551ea54eacf4a38e292003 Mon Sep 17 00:00:00 2001 From: Nicolas Schweitzer Date: Thu, 19 Dec 2024 14:47:54 +0100 Subject: [PATCH] feat(pkg_size): Delete old code and round the diff (#32207) --- .gitlab/.ci-linters.yml | 2 - .gitlab/JOBOWNERS | 2 +- .gitlab/pkg_metrics/pkg_metrics.yml | 106 +------------------------- tasks/libs/package/size.py | 46 ++++------- tasks/libs/package/utils.py | 6 +- tasks/package.py | 44 ----------- tasks/unit_tests/package_lib_tests.py | 20 +++++ 7 files changed, 42 insertions(+), 184 deletions(-) diff --git a/.gitlab/.ci-linters.yml b/.gitlab/.ci-linters.yml index 247e9f7e36318..fd1af4a9868c5 100644 --- a/.gitlab/.ci-linters.yml +++ b/.gitlab/.ci-linters.yml @@ -57,8 +57,6 @@ job-owners: - build_otel_agent_binary_arm64 - build_otel_agent_binary_x64 - cancel-prev-pipelines - - check_pkg_size-amd64-a7 - - check_pkg_size-arm64-a7 - close_failing_tests_stale_issues - compute_gitlab_ci_config - deploy_cluster_agent_cloudfoundry diff --git a/.gitlab/JOBOWNERS b/.gitlab/JOBOWNERS index e03f28a2d537d..2678bd7a5de6d 100644 --- a/.gitlab/JOBOWNERS +++ b/.gitlab/JOBOWNERS @@ -67,7 +67,7 @@ iot_agent_suse* @DataDog/agent-delivery dogstatsd_suse* @DataDog/agent-delivery agent_oci* @DataDog/agent-delivery installer_oci* @DataDog/agent-delivery -new_check_pkg_size @DataDog/agent-delivery +check_pkg_size @DataDog/agent-delivery # Testing package deploy deploy_deb_testing* @DataDog/agent-delivery diff --git a/.gitlab/pkg_metrics/pkg_metrics.yml b/.gitlab/pkg_metrics/pkg_metrics.yml index ceed774b62f41..ed09fa8ed1ee2 100644 --- a/.gitlab/pkg_metrics/pkg_metrics.yml +++ b/.gitlab/pkg_metrics/pkg_metrics.yml @@ -76,118 +76,14 @@ send_pkg_size: - inv package.send-size --flavor "agent" --package-os "suse" --package-path $OMNIBUS_PACKAGE_DIR_SUSE/datadog-agent-7.*.aarch64.rpm --major-version "7" --git-ref "${CI_COMMIT_REF_SLUG}" --bucket-branch "${BUCKET_BRANCH}" --arch arm64 -.check_pkg_size: - stage: pkg_metrics - image: registry.ddbuild.io/ci/datadog-agent-buildimages/deb_x64$DATADOG_AGENT_BUILDIMAGES_SUFFIX:$DATADOG_AGENT_BUILDIMAGES - tags: ["arch:amd64"] - script: - - ls -l $OMNIBUS_PACKAGE_DIR - - if [[ "${ARCH}" == "amd64" ]]; then ls -l $OMNIBUS_PACKAGE_DIR_SUSE; fi - - - export failures=0 - - export last_stable=$(inv release.get-release-json-value "last_stable::${MAJOR_VERSION}" --no-worktree) - # Get stable packages from S3 buckets, send new package sizes & compare stable and new package sizes - # The loop assumes that all flavors start with "da", which is currently the case - # We want to run all package size comparisons before failing, so we set +e while doing the comparisons - # to get the error codes without exiting the shell. - - | - if [[ "${ARCH}" == "amd64" ]]; then ARCH_RPM_EXT="x86_64"; else ARCH_RPM_EXT="aarch64"; fi - for flavor in ${FLAVORS}; do - - if [[ "${ARCH}" == "amd64" && "$flavor" != "datadog-heroku-agent" ]]; then - mkdir -p "/tmp/stable/${flavor}/suse" - curl -sSL "https://s3.amazonaws.com/yum.datadoghq.com/suse/stable/${MAJOR_VERSION}/${ARCH_RPM_EXT}/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm" -o "/tmp/stable/${flavor}/suse/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm" - set +e - inv package.compare-size --package-type "${flavor} suse rpm" --last-stable "${last_stable}" --threshold "${max_sizes[${flavor}]}" --new-package "$OMNIBUS_PACKAGE_DIR_SUSE/${flavor}-${MAJOR_VERSION}.*.${ARCH_RPM_EXT}.rpm" --stable-package "/tmp/stable/${flavor}/suse/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm" - failures=$((${failures}+$?)) - set -e - fi - - mkdir -p "/tmp/stable/${flavor}" - - curl -sSL "https://s3.amazonaws.com/apt.datadoghq.com/pool/d/da/${flavor}_${last_stable}-1_${ARCH}.deb" -o "/tmp/stable/${flavor}/${flavor}_${last_stable}-1_${ARCH}.deb" - - set +e - inv package.compare-size --package-type "${flavor} deb" --last-stable "${last_stable}" --threshold "${max_sizes[${flavor}]}" --new-package "$OMNIBUS_PACKAGE_DIR/${flavor}_${MAJOR_VERSION}*_${ARCH}.deb" --stable-package "/tmp/stable/${flavor}/${flavor}_${last_stable}-1_${ARCH}.deb" - failures=$((${failures}+$?)) - set -e - - if [[ "$flavor" != "datadog-heroku-agent" && ( "${ARCH}" == "amd64" || "$flavor" != "datadog-dogstatsd") ]]; then - # We don't build RPM packages for the heroku flavor - curl -sSL "https://s3.amazonaws.com/yum.datadoghq.com/stable/${MAJOR_VERSION}/${ARCH_RPM_EXT}/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm" -o "/tmp/stable/${flavor}/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm" - set +e - inv package.compare-size --package-type "${flavor} rpm" --last-stable "${last_stable}" --threshold "${max_sizes[${flavor}]}" --new-package "$OMNIBUS_PACKAGE_DIR/${flavor}-${MAJOR_VERSION}.*.${ARCH_RPM_EXT}.rpm" --stable-package "/tmp/stable/${flavor}/${flavor}-${last_stable}-1.${ARCH_RPM_EXT}.rpm" - failures=$((${failures}+$?)) - set -e - fi - done - - # Make the job fail if at least one package is above threshold - - if [ "${failures}" -ne "0" ]; then false; fi - -check_pkg_size-amd64-a7: - extends: .check_pkg_size - rules: - - !reference [.except_mergequeue] - - when: on_success - needs: - - agent_deb-x64-a7 - - iot_agent_deb-x64 - - dogstatsd_deb-x64 - - agent_heroku_deb-x64-a7 - - agent_rpm-x64-a7 - - iot_agent_rpm-x64 - - dogstatsd_rpm-x64 - - agent_suse-x64-a7 - - dogstatsd_suse-x64 - - iot_agent_suse-x64 - variables: - MAJOR_VERSION: 7 - FLAVORS: "datadog-agent datadog-iot-agent datadog-dogstatsd datadog-heroku-agent" - ARCH: "amd64" - before_script: - # FIXME: ["datadog-agent"]="140000000" and ["datadog-heroku-agent"]="140000000" should - # be replaced by "50000000" - # "70000000" is needed as of now because of multiple large additions in 7.45 - - | - declare -Ar max_sizes=( - ["datadog-agent"]="140000000" - ["datadog-iot-agent"]="10000000" - ["datadog-dogstatsd"]="10000000" - ["datadog-heroku-agent"]="70000000" - ) - -check_pkg_size-arm64-a7: - extends: .check_pkg_size - rules: !reference [.on_all_builds] - needs: - - agent_deb-arm64-a7 - - iot_agent_deb-arm64 - - dogstatsd_deb-arm64 - - agent_rpm-arm64-a7 - - iot_agent_rpm-arm64 - variables: - MAJOR_VERSION: 7 - FLAVORS: "datadog-agent datadog-iot-agent datadog-dogstatsd" - ARCH: "arm64" - before_script: - # FIXME: ["datadog-agent"]="140000000" should be replaced by "70000000" - # "140000000" is needed as of now because of multiple large additions in 7.45 - - | - declare -Ar max_sizes=( - ["datadog-agent"]="140000000" - ["datadog-iot-agent"]="10000000" - ["datadog-dogstatsd"]="10000000" - ) -new_check_pkg_size: +check_pkg_size: stage: pkg_metrics image: registry.ddbuild.io/ci/datadog-agent-buildimages/deb_x64$DATADOG_AGENT_BUILDIMAGES_SUFFIX:$DATADOG_AGENT_BUILDIMAGES tags: ["arch:amd64"] rules: - !reference [.except_mergequeue] - when: on_success - allow_failure: true needs: - agent_deb-x64-a7 - iot_agent_deb-x64 diff --git a/tasks/libs/package/size.py b/tasks/libs/package/size.py index 4743dfcd906de..2323d40fdb56b 100644 --- a/tasks/libs/package/size.py +++ b/tasks/libs/package/size.py @@ -37,22 +37,22 @@ # The below template contains the relative increase threshold for each package type PACKAGE_SIZE_TEMPLATE = { 'amd64': { - 'datadog-agent': {'deb': 140 * pow(10, 6)}, - 'datadog-iot-agent': {'deb': 10 * pow(10, 6)}, - 'datadog-dogstatsd': {'deb': 10 * pow(10, 6)}, - 'datadog-heroku-agent': {'deb': 70 * pow(10, 6)}, + 'datadog-agent': {'deb': int(140e6)}, + 'datadog-iot-agent': {'deb': int(10e6)}, + 'datadog-dogstatsd': {'deb': int(10e6)}, + 'datadog-heroku-agent': {'deb': int(70e6)}, }, 'x86_64': { - 'datadog-agent': {'rpm': 140 * pow(10, 6), 'suse': 140 * pow(10, 6)}, - 'datadog-iot-agent': {'rpm': 10 * pow(10, 6), 'suse': 10 * pow(10, 6)}, - 'datadog-dogstatsd': {'rpm': 10 * pow(10, 6), 'suse': 10 * pow(10, 6)}, + 'datadog-agent': {'rpm': int(140e6), 'suse': int(140e6)}, + 'datadog-iot-agent': {'rpm': int(10e6), 'suse': int(10e6)}, + 'datadog-dogstatsd': {'rpm': int(10e6), 'suse': int(10e6)}, }, 'arm64': { - 'datadog-agent': {'deb': 140 * pow(10, 6)}, - 'datadog-iot-agent': {'deb': 10 * pow(10, 6)}, - 'datadog-dogstatsd': {'deb': 10 * pow(10, 6)}, + 'datadog-agent': {'deb': int(140e6)}, + 'datadog-iot-agent': {'deb': int(10e6)}, + 'datadog-dogstatsd': {'deb': int(10e6)}, }, - 'aarch64': {'datadog-agent': {'rpm': 140 * pow(10, 6)}, 'datadog-iot-agent': {'rpm': 10 * pow(10, 6)}}, + 'aarch64': {'datadog-agent': {'rpm': int(140e6)}, 'datadog-iot-agent': {'rpm': int(10e6)}}, } @@ -177,25 +177,11 @@ def compare(ctx, package_sizes, ancestor, pkg_size): return pkg_size -def mb(value): - return f"{value / 1000000:.2f}MB" - - def _get_uncompressed_size(ctx, package, os_name): if os_name == 'deb': - return _get_deb_uncompressed_size(ctx, package) + return ( + int(ctx.run(f'dpkg-deb --info {package} | grep Installed-Size | cut -d : -f 2 | xargs', hide=True).stdout) + * 1024 + ) else: - return _get_rpm_uncompressed_size(ctx, package) - - -def _get_deb_uncompressed_size(ctx, package): - # the size returned by dpkg is a number of bytes divided by 1024 - # so we multiply it back to get the same unit as RPM or stat - return ( - int(ctx.run(f'dpkg-deb --info {package} | grep Installed-Size | cut -d : -f 2 | xargs', hide=True).stdout) - * 1024 - ) - - -def _get_rpm_uncompressed_size(ctx, package): - return int(ctx.run(f'rpm -qip {package} | grep Size | cut -d : -f 2 | xargs', hide=True).stdout) + return int(ctx.run(f'rpm -qip {package} | grep Size | cut -d : -f 2 | xargs', hide=True).stdout) diff --git a/tasks/libs/package/utils.py b/tasks/libs/package/utils.py index ec8363fcfa134..8f43eaa9b4e33 100644 --- a/tasks/libs/package/utils.py +++ b/tasks/libs/package/utils.py @@ -20,6 +20,7 @@ def __init__(self, arch, flavor, os_name, threshold): self.size = 0 self.ancestor_size = 0 self.diff = 0 + self.mb_diff = 0 self.threshold = threshold self.emoji = "✅" @@ -48,14 +49,15 @@ def compare(self, size, ancestor_size): self.size = size self.ancestor_size = ancestor_size self.diff = self.size - self.ancestor_size + self.mb_diff = float(f"{self.diff / pow(10, 6):.2f}") if self.ko(): self.emoji = "❌" - elif self.diff > 0: + elif self.mb_diff > 0: self.emoji = "⚠️" @staticmethod def mb(value): - return f"{value / 1000000:.2f}MB" + return f"{value / 1e6:.2f}MB" def log(self): return f"{self.emoji} - {self.name} size {self.mb(self.size)}: {self.mb(self.diff)} diff[{self.diff}] with previous {self.mb(self.ancestor_size)} (max: {self.mb(self.threshold)})" diff --git a/tasks/package.py b/tasks/package.py index f2d49130152dc..3a831a24225fc 100644 --- a/tasks/package.py +++ b/tasks/package.py @@ -8,15 +8,12 @@ from tasks.libs.common.git import get_default_branch from tasks.libs.package.size import ( PACKAGE_SIZE_TEMPLATE, - _get_deb_uncompressed_size, - _get_rpm_uncompressed_size, compare, compute_package_size_metrics, ) from tasks.libs.package.utils import ( PackageSize, display_message, - find_package, get_ancestor, list_packages, retrieve_package_sizes, @@ -61,47 +58,6 @@ def check_size(ctx, filename: str = 'package_sizes.json', dry_run: bool = False) raise Exit(code=1) -@task -def compare_size(ctx, new_package, stable_package, package_type, last_stable, threshold): - mb = 1000000 - - if package_type.endswith('deb'): - new_package_size = _get_deb_uncompressed_size(ctx, find_package(new_package)) - stable_package_size = _get_deb_uncompressed_size(ctx, find_package(stable_package)) - else: - new_package_size = _get_rpm_uncompressed_size(ctx, find_package(new_package)) - stable_package_size = _get_rpm_uncompressed_size(ctx, find_package(stable_package)) - - threshold = int(threshold) - - diff = new_package_size - stable_package_size - - # For printing purposes - new_package_size_mb = new_package_size / mb - stable_package_size_mb = stable_package_size / mb - threshold_mb = threshold / mb - diff_mb = diff / mb - - if diff > threshold: - print( - color_message( - f"""{package_type} size increase is too large: - New package size is {new_package_size_mb:.2f}MB - Stable package ({last_stable}) size is {stable_package_size_mb:.2f}MB - Diff is {diff_mb:.2f}MB > {threshold_mb:.2f}MB (max allowed diff)""", - "red", - ) - ) - raise Exit(code=1) - - print( - f"""{package_type} size increase is OK: - New package size is {new_package_size_mb:.2f}MB - Stable package ({last_stable}) size is {stable_package_size_mb:.2f}MB - Diff is {diff_mb:.2f}MB (max allowed diff: {threshold_mb:.2f}MB)""" - ) - - @task def send_size( ctx, diff --git a/tasks/unit_tests/package_lib_tests.py b/tasks/unit_tests/package_lib_tests.py index 984e4cc4f871c..72f3c6c190252 100644 --- a/tasks/unit_tests/package_lib_tests.py +++ b/tasks/unit_tests/package_lib_tests.py @@ -238,6 +238,26 @@ def test_on_branch_warning(self, mock_print): f"⚠️ - {flavor}-{arch}-{os_name} size 69.00MB: 1.00MB diff[1000000] with previous 68.00MB (max: 70.00MB)" ) + @patch.dict( + 'os.environ', + {'OMNIBUS_PACKAGE_DIR_SUSE': 'tasks/unit_tests/testdata/packages', 'CI_COMMIT_REF_NAME': 'pikachu'}, + ) + @patch('builtins.print') + def test_on_branch_ok_small_diff(self, mock_print): + flavor, arch, os_name = 'datadog-agent', 'aarch64', 'suse' + s = PackageSize(arch, flavor, os_name, 70000000) + c = MockContext( + run={ + 'git merge-base HEAD origin/main': Result('25'), + f"rpm -qip {self.pkg_root}/{flavor}-7.{arch}.rpm | grep Size | cut -d : -f 2 | xargs": Result(68004999), + } + ) + res = compare(c, self.package_sizes, '25', s) + self.assertEqual(res.markdown(), "|datadog-agent-aarch64-suse|0.00MB|✅|68.00MB|68.00MB|70.00MB|") + mock_print.assert_called_with( + f"✅ - {flavor}-{arch}-{os_name} size 68.00MB: 0.00MB diff[4999] with previous 68.00MB (max: 70.00MB)" + ) + @patch.dict( 'os.environ', {'OMNIBUS_PACKAGE_DIR': 'tasks/unit_tests/testdata/packages', 'CI_COMMIT_REF_NAME': 'pikachu'} )