Update PSAdapters to automatically convert streams to DSC traces#1379
Update PSAdapters to automatically convert streams to DSC traces#1379SteveL-MSFT wants to merge 15 commits intoPowerShell:mainfrom
Conversation
bb39c85 to
b6f52f0
Compare
There was a problem hiding this comment.
Pull request overview
This pull request implements automatic conversion of PowerShell streams to DSC traces, addressing issue #423. The core change introduces an event-driven mechanism that captures PowerShell output streams (Error, Warning, Information, Verbose, Debug) and converts them to corresponding DSC trace levels (error, warn, info/trace, debug, trace).
Changes:
- Refactored powershell.resource.ps1 to use async PowerShell execution with Register-ObjectEvent for stream capture
- Removed Write-DscTrace function from psDscAdapter modules and replaced with native PowerShell stream cmdlets (Write-Error, Write-Warning, etc.)
- Updated modules to set ErrorActionPreference='Stop' and use -Verbose/-Debug flags consistently
- Added StreamResource test class and comprehensive streaming tests to validate the conversion
- Simplified powershell.dsc.yaml example by removing nested WindowsPowerShell adapter structure
- Updated existing tests to accommodate JSON-formatted trace output
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| adapters/powershell/psDscAdapter/powershell.resource.ps1 | Major refactoring to implement async PowerShell execution with event-based stream capture and conversion to DSC traces |
| adapters/powershell/psDscAdapter/win_psDscAdapter.psm1 | Removed custom Write-DscTrace function, replaced with native Write-Error/Warning/Verbose/Debug, added ErrorActionPreference='Stop' |
| adapters/powershell/psDscAdapter/psDscAdapter.psm1 | Same changes as win_psDscAdapter.psm1 - removed Write-DscTrace, using native streams |
| adapters/powershell/Tests/powershellgroup.resource.tests.ps1 | Added comprehensive streaming tests for all trace levels, updated ClearCache test to suppress stderr |
| adapters/powershell/Tests/powershellgroup.config.tests.ps1 | Updated tests to capture trace output for better diagnostics, changed assertions for JSON-formatted traces |
| adapters/powershell/Tests/win_powershell_cache.tests.ps1 | Changed FileContentMatchExactly to -BeLike to accommodate JSON-formatted trace messages |
| adapters/powershell/Tests/TestClassResource/0.0.1/TestClassResource.psm1 | Added StreamResource test class that emits various stream types for testing |
| adapters/powershell/Tests/TestClassResource/0.0.1/TestClassResource.psd1 | Exported StreamResource in DscResourcesToExport array |
| dsc/examples/powershell.dsc.yaml | Simplified example by removing nested WindowsPowerShell adapter structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
A couple of notes from a design/spec perspective:
|
|
I’m not satisfied with the detection if the resource failed, so perhaps treat all records in error stream as non-terminating and only have unhandled exception as failure? |
|
I think unhandled exceptions as failure makes sense. This would still bubble up the error messages from those commands in existing resources without causing the operation for a resource to fail (and thus halt a configuration operation). Then resource authors can clean up their implementation to avoid surfacing erroneous error messages or we can define an option to only surface unhandled exceptions or whatever makes sense. |
|
I'll make these changes:
|
Is it possible to emit error messages without failing an operation? I thought that whether a non-adapted resource emitted error messages was separate from whether we considered an operation to have failed (e.g. emitting errors but exiting If you can emit error-level messages with exit code If the presence of error messages causes DSC to consider a resource operation as a failure even when the exit code is Otherwise the proposed changes all look good to me. |
|
@michaeltlombardi yes, error messages are not considered fatal, only the exit code. I guess I was trying to cover for "badly written" resources too much that will spew errors implicitly. However, it's probably better to trace every ErrorRecord as an error trace and leave it to resource authors to clean up their code. Since both Error and Warning are, by default, displayed, it doesn't change much to shift non-terminating errors as warnings. So new plan:
|
PR Summary
Automatically convert PS streams to DSC traces:
Write-HostandWrite-Informationis INFOWrite-Warningis WARNWrite-Erroris ERRORWrite-Debugis TRACE (since we can't be certain raw information isn't written) and must be explicitlyWrite-Debug -DebugWrite-Verboseis DEBUG (seemed too noisy to be INFO) and must be explicitlyWrite-Verbose -VerboseThe main changes:
powershell.resource.ps1in a scriptblock that gets executed in a runspace so streams can be captured$VerbosePreferenceand$DebugPreferencetosilentlycontinueas it was generating too much noise in the traces, docs should state that either resources set those preferences variables tocontinueor explicitly useWrite-Verbose -VerboseorWrite-Debug -DebugHadErrorsistrueif anything is written to stderr, it just gets traced but ignored. Instead, a script is seen as failed if theErrorstream had any records. This means that resources need to use-ErrorAction Ignoreinstead of-ErrorAction SilentlyContinue, this change was made throughout the adapterWrite-DscTracein adapter and support modules have it write to the PS streams instead$env:DSC_TRACE_LEVELto determine which streams to actually listen toStreamResourceclass resource to validate conversion-Becausewith the tracelog to help identify why they brokepowershell.dsc.yamlexample to use PS resources implicitly instead of explicitlyPR Context
Fix #423