Promote MSBuildBinaryLog Detector to prod - #1873
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate findings affect state isolation, compatibility, telemetry, and integration tests.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (4)
What changed in this PR
Promotes MSBuildBinaryLogComponentDetector to production for .NET/NuGet detection, replacing legacy detectors while retaining fallback coverage.
Changes:
- Removes legacy detector registrations and obsolete experiment configuration.
- Adds fallback, compatibility, and parse-failure telemetry coverage.
- Updates detector documentation and integration tests.
Review findings:
- Critical (1 vote): Clear per-scan state when cancellation or exceptions bypass cleanup.
- Moderate (2 votes): Ensure unreadable or corrupt binlogs emit promised parse-failure telemetry.
- Moderate (1 vote): Preserve the
DotNetdetector category. - Moderate (2 votes): Add aliases for retired
DotNetandNuGetProjectCentricfilters. - Critical (2 votes): Update integration-test allowlists for retired detector IDs.
| File | Summary |
|---|---|
test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/LinuxApplicationLayerExperimentTests.cs |
Updates experiment detector expectations. |
test/Microsoft.ComponentDetection.Detectors.Tests/nuget/MSBuildBinaryLogComponentDetectorTests.cs |
Covers production, fallback, and telemetry behavior. |
test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildBinaryLogFallbackCompatibilityTests.cs |
Updates NuGet fallback compatibility tests. |
test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildBinaryLogDotNetFallbackCompatibilityTests.cs |
Updates .NET fallback compatibility tests. |
test/Microsoft.ComponentDetection.Detectors.Tests/ComponentDetectorTests.cs |
Verifies detector registration and replacement coverage. |
src/Microsoft.ComponentDetection.Orchestrator/Extensions/ServiceCollectionExtensions.cs |
Replaces legacy registrations with the promoted detector. |
src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/MSBuildBinaryLogExperiment.cs |
Removes obsolete experiment configuration. |
src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/LinuxApplicationLayerExperiment.cs |
Updates experiment detector membership. |
src/Microsoft.ComponentDetection.Detectors/nuget/NuGetProjectModelProjectCentricComponentDetector.cs |
Removes the superseded NuGet detector. |
src/Microsoft.ComponentDetection.Detectors/nuget/MSBuildBinaryLogComponentDetector.cs |
Promotes the detector and updates fallback and telemetry behavior. |
src/Microsoft.ComponentDetection.Detectors/nuget/LockFileUtilities.cs |
Updates utility ownership documentation. |
src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetProjectInfoProvider.cs |
Updates fallback ownership documentation. |
src/Microsoft.ComponentDetection.Detectors/dotnet/DotNetComponentDetector.cs |
Removes the superseded .NET detector. |
README.md |
Updates the .NET detector documentation link. |
docs/detectors/README.md |
Marks the replacement detector as stable. |
docs/detectors/nuget.md |
Documents production, fallback, and telemetry behavior. |
docs/detectors/dotnet.md |
Removes superseded detector documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Ryan Brandenburg (ryanbrandenburg)
left a comment
There was a problem hiding this comment.
Code looks fine, though the tests seem to have noticed some differences in your compatibility tests.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Resolve cancellation-state cleanup and legacy DotNet category filtering compatibility.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
Resolved since last review (1)
…ub.com/microsoft/component-detection into users/grvillic/PromoteMSBinLogDetector
|
👋 Hi! It looks like you modified some files in the
If none of the above scenarios apply, feel free to ignore this comment 🙂 |


Context
We have validated telemetry and confirmed in sample of repositories generating bin logs that NuGet dependencies are being classified as dev dependency.
Additionally, we reviewed performance across the board and dependencies found and it does meet expectations and replaces DotNet and NugetProjectCentric detector, as it finds the exact same list of dependencies but with richer metadata that allows to find more dev dependencies and specific project types of .NET SDK libraries/executables that otherwise we couldn't find. When no binlog is generated, the detector will fall back to preexisting detection coverage.