Skip to content

Commit 3d2b9bf

Browse files
committed
perf(diff): skip unchanged artifacts when no output reads them
Cached diff-scan responses embed every unchanged artifact at roughly 1 KB each. On a large dependency tree that is nearly the whole response — measured at ~11 MB for a tree with ~10k unchanged packages — downloaded, deserialised into Package objects and then discarded on every pull request. omit_unchanged is honored by the API (unlike omit_license_details, which cached responses ignore), so request it whenever no enabled output reads that half of the comparison. Verified against the live API through the SDK: 192 artifacts -> 57. Every consumer is behind an opt-in flag, so the gate is centralised in Core._requires_unchanged_artifacts with the reasoning recorded there: - --strict-blocking blocks on pre-existing issues via diff.unchanged_alerts - --enable-gitlab-security includes them in the dependency scanning report - --generate-license enumerates diff.packages, which must list every dependency - --legal-format fossa reports all currently-present issues Diff.to_dict serialises them too but has no callers. When cli_config is absent the caller is unknown, so the full payload is kept. Tests parametrise over every flag in that list so a new reader of diff.unchanged_alerts or diff.packages cannot be added without also updating the gate. The completion log reports omit_unchanged so it is visible whether the optimisation engaged on a given run. Ref: CE-379
1 parent 40b141e commit 3d2b9bf

3 files changed

Lines changed: 113 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,16 @@
2222
smaller local fallback pattern set. Manifest results from `--sub-path` routing
2323
are reused during scan creation.
2424

25+
### Changed: scan comparisons no longer fetch unused artifacts
26+
27+
- Scan comparisons now ask the API to omit unchanged artifacts unless an enabled
28+
output actually reads them (`--strict-blocking`, `--enable-gitlab-security`,
29+
`--generate-license`, or `--legal-format fossa`). Cached comparison responses
30+
embed every unchanged artifact at roughly 1 KB each, so on a large dependency
31+
tree this was most of the response — over 10 MB for a tree of ~10k unchanged
32+
packages — downloaded and parsed on every pull request even when nothing read
33+
it. Behavior is unchanged for any run that uses those outputs.
34+
2535
### Changed: scan comparison timing is easier to attribute
2636

2737
- Lowered the diff-scan poll ceiling from 30s to 10s. A finished comparison is no

socketsecurity/core/__init__.py

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,14 +1608,13 @@ def get_diff_scan_artifacts(
16081608
#
16091609
# Verified against the live API: passing omit_license_details alongside
16101610
# cached=true leaves the license fields in the response, but omit_unchanged
1611-
# IS honored and drops the unchanged artifacts entirely (~1.1 KB each). Not
1612-
# sent here because unchanged artifacts are not only used by
1613-
# --strict-blocking: create_security_comment_gitlab and the FOSSA compat
1614-
# issue list both read diff.unchanged_alerts unconditionally, so omitting
1615-
# them would silently shrink those outputs. Gating it correctly across all
1616-
# three consumers is worth doing - on a tree with ~10k unchanged artifacts
1617-
# it is over 10 MB of response - but it needs its own change.
1611+
# IS honored and drops the unchanged artifacts entirely (~1.1 KB each), so
1612+
# it is requested whenever no enabled output reads them. See
1613+
# _requires_unchanged_artifacts.
16181614
poll_params = {"cached": "true"}
1615+
omit_unchanged = not self._requires_unchanged_artifacts()
1616+
if omit_unchanged:
1617+
poll_params["omit_unchanged"] = "true"
16191618
poll_start = time.monotonic()
16201619
deadline = poll_start + DIFF_SCAN_POLL_TIMEOUT_SECONDS
16211620
interval = DIFF_SCAN_POLL_INITIAL_INTERVAL_SECONDS
@@ -1648,7 +1647,8 @@ def get_diff_scan_artifacts(
16481647
log.info(
16491648
"Diff scan comparison ready in "
16501649
f"{time.monotonic() - poll_start:.2f}s: id={diff_scan_id}, "
1651-
f"polls={polls}, wait_before_final_poll={last_interval:.0f}s"
1650+
f"polls={polls}, wait_before_final_poll={last_interval:.0f}s, "
1651+
f"omit_unchanged={str(omit_unchanged).lower()}"
16521652
)
16531653
break
16541654
if time.monotonic() >= deadline:
@@ -1666,6 +1666,39 @@ def get_diff_scan_artifacts(
16661666
for key in ("added", "removed", "unchanged", "replaced", "updated")
16671667
})
16681668

1669+
def _requires_unchanged_artifacts(self) -> bool:
1670+
"""Whether any enabled output reads the unchanged half of a comparison.
1671+
1672+
A cached diff-scan response embeds every unchanged artifact at roughly 1 KB
1673+
each, so on a large dependency tree they are almost the entire payload
1674+
(~11 MB for a tree of ~10k unchanged packages) even though most runs never
1675+
look at them. Every consumer is behind an opt-in flag:
1676+
1677+
- ``--strict-blocking`` reads ``diff.unchanged_alerts`` to block on
1678+
pre-existing issues (socketcli, output, alert_selection, slack plugin).
1679+
- ``--enable-gitlab-security`` includes them in the GitLab dependency
1680+
scanning report (Messages.create_security_comment_gitlab).
1681+
- ``--generate-license`` enumerates ``diff.packages``, which must list every
1682+
dependency, not just the changed ones.
1683+
- ``--legal-format fossa`` reports all currently-present issues, matching
1684+
FOSSA's point-in-time snapshot semantics.
1685+
1686+
``Diff.to_dict`` also serializes them but has no callers. When cli_config is
1687+
absent the caller is unknown, so the full payload is kept.
1688+
1689+
Keep this in sync with those consumers; test_unchanged_artifacts_gating
1690+
pins the list.
1691+
"""
1692+
config = self.cli_config
1693+
if config is None:
1694+
return True
1695+
return bool(
1696+
getattr(config, "strict_blocking", False)
1697+
or getattr(config, "enable_gitlab_security", False)
1698+
or getattr(config, "generate_license", False)
1699+
or getattr(config, "legal_format", "socket") == "fossa"
1700+
)
1701+
16691702
def get_added_and_removed_packages(
16701703
self,
16711704
head_full_scan_id: str,

tests/core/test_diff_scan_polling.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,3 +153,65 @@ def test_max_poll_interval_bounds_dead_time_for_ci_budgets():
153153
core_module.DIFF_SCAN_POLL_INITIAL_INTERVAL_SECONDS
154154
<= core_module.DIFF_SCAN_POLL_MAX_INTERVAL_SECONDS
155155
)
156+
157+
158+
UNCHANGED_ARTIFACT_CONSUMERS = [
159+
# flag name, value that makes the flag active
160+
("strict_blocking", True),
161+
("enable_gitlab_security", True),
162+
("generate_license", True),
163+
("legal_format", "fossa"),
164+
]
165+
166+
167+
@pytest.mark.parametrize(("flag", "value"), UNCHANGED_ARTIFACT_CONSUMERS)
168+
def test_unchanged_artifacts_gating(core, diff_scan_get_response, flag, value):
169+
"""Any output that reads unchanged artifacts must keep them in the response.
170+
171+
This pins the consumer list in Core._requires_unchanged_artifacts: adding a new
172+
reader of diff.unchanged_alerts or diff.packages without adding it here (and to
173+
that method) would silently ship an empty result to that output.
174+
"""
175+
from types import SimpleNamespace
176+
177+
defaults = {name: (False if name != "legal_format" else "socket")
178+
for name, _ in UNCHANGED_ARTIFACT_CONSUMERS}
179+
core.cli_config = SimpleNamespace(**{**defaults, flag: value})
180+
core.sdk.diffscans.get.side_effect = None
181+
core.sdk.diffscans.get.return_value = diff_scan_get_response
182+
183+
core.get_diff_scan_artifacts("head", "new")
184+
185+
params = core.sdk.diffscans.get.call_args.kwargs["params"]
186+
assert "omit_unchanged" not in params, f"{flag}={value} still needs unchanged artifacts"
187+
188+
189+
def test_unchanged_artifacts_omitted_when_no_output_reads_them(core, diff_scan_get_response):
190+
"""With no such flag set, the ~1 KB-per-artifact unchanged half is not fetched."""
191+
from types import SimpleNamespace
192+
193+
core.cli_config = SimpleNamespace(
194+
strict_blocking=False,
195+
enable_gitlab_security=False,
196+
generate_license=False,
197+
legal_format="socket",
198+
)
199+
core.sdk.diffscans.get.side_effect = None
200+
core.sdk.diffscans.get.return_value = diff_scan_get_response
201+
202+
core.get_diff_scan_artifacts("head", "new")
203+
204+
params = core.sdk.diffscans.get.call_args.kwargs["params"]
205+
assert params["cached"] == "true"
206+
assert params["omit_unchanged"] == "true"
207+
208+
209+
def test_unknown_caller_keeps_full_payload(core, diff_scan_get_response):
210+
"""cli_config is optional; without it, do not assume unchanged is unused."""
211+
core.cli_config = None
212+
core.sdk.diffscans.get.side_effect = None
213+
core.sdk.diffscans.get.return_value = diff_scan_get_response
214+
215+
core.get_diff_scan_artifacts("head", "new")
216+
217+
assert "omit_unchanged" not in core.sdk.diffscans.get.call_args.kwargs["params"]

0 commit comments

Comments
 (0)