I am still learning PowerShell and I would like your opinion on my log writing function. It creates a log entry with timestamp and message passed thru a parameter Message or thru pipeline, and saves the log entry to log file, to report log file, and writes the same entry to console. In Configuration.cfg file paths to report log and permanent log file are contained, and option to turn on or off whether a report log and permanent log should be written. If Configuration.cfg file is absent it loads the default values. Depending on the OperationResult parameter, log entry can be written with or without a timestamp. Format of the timestamp is "yyyy.MM.dd. HH:mm:ss:fff", and this function adds " - " after timestamp and before the main message.
function Write-Log {
param (
[Parameter(Position = 0, ValueFromPipelineByPropertyName)]
[ValidateSet('Success', 'Fail', 'Partial', 'Info', 'None')]
[String]
$OperationResult = 'None',
[Parameter(Position = 1, Mandatory, ValueFromPipeline, ValueFromPipelineByPropertyName)]
[String]
$Message
)
begin {
if (Test-Path -Path '.\Configuration.cfg') {
$Configuration = Get-Content '.\Configuration.cfg' | ConvertFrom-StringData
$LogFile = $Configuration.LogFile
$ReportFile = $Configuration.ReportFile
$WriteLog = $Configuration.WriteLog -eq 'true'
$SendReport = $Configuration.SendReport -eq 'true'
}
else {
$LogFile = '.\Log.log'
$ReportFile = '.\Report.log'
$WriteLog = $true
$SendReport = $true
}
if (-not (Test-Path -Path $LogFile)) {
New-Item -Path $LogFile -ItemType File
}
if (-not (Test-Path -Path $ReportFile)) {
New-Item -Path $ReportFile -ItemType File
}
}
process {
$Timestamp = Get-Date -Format 'yyyy.MM.dd. HH:mm:ss:fff'
$LogEntry = $Timestamp + " - " + $Message
switch ($OperationResult) {
'Success' {
$ForegroundColor = 'Green'
break
}
'Fail' {
$ForegroundColor = 'Red'
break
}
'Partial' {
$ForegroundColor = 'Yellow'
break
}
'Info' {
$ForegroundColor = 'Cyan'
break
}
'None' {
$ForegroundColor = 'White'
$LogEntry = $Message
}
}
Write-Host $LogEntry -ForegroundColor $ForegroundColor -BackgroundColor Black
if ($WriteLog) {
Add-content -Path $LogFile -Value $LogEntry
}
if ($SendReport) {
Add-content -Path $ReportFile -Value $LogEntry
}
}
}
2 Answers 2
I don't see anything wrong or debatable in this so far. Just a couple nitpicks:
I would use parameter splatting to make the color part more compact. For example, from my own log function:
switch ($Severity) { Debug { $colors = @{ForegroundColor="DarkGray" } } Info { $colors = @{ } } Warning { $colors = @{ForegroundColor="Black" ; BackgroundColor="Yellow" } } Error { $colors = @{ForegroundColor="White" ; BackgroundColor="DarkRed" } } Critical { $colors = @{ForegroundColor="DarkYellow" ; BackgroundColor="DarkRed" } } } Write-Host @colors "$($Severity): $Message"
Maybe you could go even further with a hash of hashs and do Write-Host @colors["error"]
or something like that.
To create a file only if the file doesn't already exist, you can call
New-Item
directly, it will throw an exception if the file already exists which you can just ignore with a try-catch or with-ErrorAction SilentlyContinue
, saving a couple lines.Why not make all the configuration fetched from the config file optional parameters, with the values in the file as default values ? That would help if you ever have to use the function on the fly, in a shell or as a debug tool.
Very personal opinion, but I would personally put the default value of
$OperationResult
toInfo
, as this is the most common case for me and I'd appreciate just whipping out a simpleWrite-Log "mymessage"
without extra typing.Last one, this one depends on your environment, but
Write-Log
is also a function in the officiel VMWare vSphere module that is quite widely used. I wouldn't change any code because of that but remember to document that if you ever have to distribute your code.
Overall I think your cmdlet is pretty readable and produces a log that is easy enough to parse, so it ticks all the important boxes for me.
I have arrived at a new solution for my log and transcript PowerShell function.
function Write-Log {
[CmdletBinding()]
param (
[Parameter(Position = 0, Mandatory, ValueFromPipeline, ValueFromPipelineByPropertyName)]
[string]
$Message,
[Parameter(Mandatory = $false)]
[switch]
$NoTimestamp = $false
)
begin {
if (Test-Path -Path '.\Settings.cfg') {
$Settings = Get-Content '.\Settings.cfg' | ConvertFrom-StringData
$LogFile = $Settings.LogFile
$ReportFile = $Settings.ReportFile
$WriteTranscript = $Settings.WriteTranscript -eq "true"
$WriteLog = $Settings.WriteLog -eq "true"
$SendReport = $Settings.SendReport -eq "true"
}
else {
$LogFile = '.\Log.log'
$ReportFile = '.\Report.log'
$WriteTranscript = $true
$WriteLog = $true
$SendReport = $true
}
if (-not (Test-Path -Path $LogFile)) {
New-Item -Path $LogFile -ItemType File
}
if ((-not (Test-Path -Path $ReportFile)) -and $SendReport) {
New-Item -Path $ReportFile -ItemType File
}
}
process {
if (-not($NoTimestamp)) {
$Timestamp = Get-Date -Format "yyyy.MM.dd. HH:mm:ss:fff"
$LogEntry = $Timestamp + " - " + $Message
}
else {
$LogEntry = $Message
}
if ($WriteTranscript) {
Write-Verbose $LogEntry -Verbose
}
if ($WriteLog) {
Add-content -Path $LogFile -Value $LogEntry
}
if ($SendReport) {
Add-content -Path $ReportFile -Value $LogEntry
}
}
}
$PSScriptRoot
. [2] it looks like you ALWAYS write the two files. if so, why bother with the two $Vars? [3] you seem to ALWAYS write to the screen. i would make that an option that was off by default. your function IS namedWrite-Log
... [grin] [4] i would add details in the function about the structure of the CFG file. [5] i would also add Comment Base Help. \$\endgroup\$