Skip to content

Conversation

@JamieMagee
Copy link
Member

@JamieMagee JamieMagee commented Jan 15, 2026

Updates the LinuxApplicationLayerExperiment to correctly measure net-new application packages found exclusively inside containers (e.g., packages installed via RUN npm add lodash).

Changes

Experiment Configuration:

  • Control Group: File-based detectors + LinuxContainerDetector (system packages)
  • Experiment Group: File-based detectors + LinuxApplicationLayerDetector (system + application packages)

This setup ensures:

  • Components from manifest files (e.g., package.json) cancel out in both groups
  • System packages detected by LinuxContainerDetector cancel out with those from LinuxApplicationLayerDetector
  • The diff shows only application packages installed inside containers that weren't in manifest files

ShouldRecord Logic:

  • Returns false when LinuxContainerDetector finds 0 components (no containers scanned)
  • Returns true for file-based detectors to avoid prematurely removing the experiment before container detectors run

Copilot AI review requested due to automatic review settings January 15, 2026 22:28
@JamieMagee JamieMagee requested a review from a team as a code owner January 15, 2026 22:28
@JamieMagee JamieMagee requested a review from pauld-msft January 15, 2026 22:28
@JamieMagee JamieMagee force-pushed the users/jamagee/fix-linux-application-layer-experiment branch from 1c68024 to 37fa219 Compare January 15, 2026 22:29
@github-actions
Copy link

github-actions bot commented Jan 15, 2026

👋 Hi! It looks like you modified some files in the Detectors folder.
You may need to bump the detector versions if any of the following scenarios apply:

  • The detector detects more or fewer components than before
  • The detector generates different parent/child graph relationships than before
  • The detector generates different devDependencies values than before

If none of the above scenarios apply, feel free to ignore this comment 🙂

@codecov
Copy link

codecov bot commented Jan 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.1%. Comparing base (0071e78) to head (612af46).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1616   +/-   ##
=====================================
  Coverage   90.1%   90.1%           
=====================================
  Files        435     435           
  Lines      37358   37376   +18     
  Branches    2309    2310    +1     
=====================================
+ Hits       33667   33684   +17     
  Misses      3217    3217           
- Partials     474     475    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes npm, pip, and NuGet component detectors from the control group of the LinuxApplicationLayerExperiment. These detectors were incorrectly included in the experiment's control group, causing the experiment to incorrectly report that these components were being removed when they are not actually planned for removal.

Changes:

  • Simplified the IsInControlGroup method to only include LinuxContainerDetector (excluding LinuxApplicationLayerDetector)
  • Removed unnecessary using statements for npm, pip, and NuGet detector namespaces
  • Removed test cases that verified npm, pip, and NuGet detectors were in the control/experiment groups

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/LinuxApplicationLayerExperiment.cs Simplified control group logic to only include Linux container detector; removed unused imports
test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs Removed test cases for npm, pip, and NuGet detectors; removed unused imports
Comments suppressed due to low confidence (1)

src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/LinuxApplicationLayerExperiment.cs:9

  • The XML documentation comment is now outdated. It states that the control group includes 'standard file-based npm and pip detectors' but these have been removed from the control group. The comment should be updated to reflect that only the Linux system package detector is in the control group.
/// Control group includes the standard file-based npm and pip detectors plus the Linux system package detector.

@grvillic
Copy link
Collaborator

I don't think we should update the experiment. This detector is introducing packages from multiple ecosystems, we need to know exactly what it found in all of them, that the other detectors couldn't report or vice versa.

@JamieMagee JamieMagee force-pushed the users/jamagee/fix-linux-application-layer-experiment branch from 37fa219 to b9bbadf Compare January 15, 2026 22:53
Copilot AI review requested due to automatic review settings January 16, 2026 00:42
@JamieMagee JamieMagee force-pushed the users/jamagee/fix-linux-application-layer-experiment branch from b9bbadf to 111b622 Compare January 16, 2026 00:42
@JamieMagee JamieMagee force-pushed the users/jamagee/fix-linux-application-layer-experiment branch from 111b622 to 612af46 Compare January 16, 2026 00:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@JamieMagee JamieMagee merged commit b956920 into main Jan 16, 2026
28 checks passed
@JamieMagee JamieMagee deleted the users/jamagee/fix-linux-application-layer-experiment branch January 16, 2026 05:18
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.

3 participants