4
\$\begingroup\$

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
        }
    }
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ [1] i would avoid the use of relative paths. that can bite you when the OS & PoSh disagree about the current dir ... and also when the current dir is not what you were expecting. take a look at $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 named Write-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\$
    – Lee_Dailey
    Aug 11, 2020 at 18:08
  • 1
    \$\begingroup\$ If you are writing to the console and want a "transcript" of activity then you could just use Start-Transcript. Dont forget to use Stop-Transcript to stop it - I would put that in a Finally block. \$\endgroup\$
    – Kate
    Aug 11, 2020 at 19:40

2 Answers 2

2
\$\begingroup\$

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 to Info, as this is the most common case for me and I'd appreciate just whipping out a simple Write-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.

\$\endgroup\$
0
\$\begingroup\$

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
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.