1
\$\begingroup\$

Below is some powershell to write to the event log; if the -Force parameter is specified this saves you the effort of writing additional code to first check whether the log exists.

I had to make use of Remove-Variable to allow some parameters to flow through; I'd not used that feature before / been aware of that nuance, so not sure whether this is the best approach?

cls
$myLog = 'Application'
$mySource = 'My PS Script'
$myEventId = 1 
[System.Diagnostics.EventLogEntryType]$myEntryType = [System.Diagnostics.EventLogEntryType]::Error 
$myMessage = 'This is a test message'
$myMessage2 = 'This is a test message (2)'
$myMessage3 = 'This is a test message (3)'

function Write-EventLog2
{
    [cmdletbinding()]
    param(  
        [Parameter(Position=0,Mandatory=$true,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [String]$LogName
        ,
        [Parameter(Position=1,Mandatory=$true,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [String]$Source
        ,
        [Parameter(Position=3,Mandatory=$true,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [Int32]$EventId
        ,
        [Parameter(Position=4,Mandatory=$false,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [System.Diagnostics.EventLogEntryType]$EntryType
        ,
        [Parameter(Position=5,Mandatory=$false,ValueFromPipeline=$true,ValueFromPipelineByPropertyName=$true)]
        [String]$Message
        ,
        [Parameter(Position=6,Mandatory=$false,ValueFromPipeline=$false,ValueFromPipelineByPropertyName=$true)]
        [Int16]$Category 
        ,
        [Parameter(Position=7,Mandatory=$false,ValueFromPipeline=$false,ValueFromPipelineByPropertyName=$true)]
        [String]$ComputerName 
        ,
        [Parameter(Position=8,Mandatory=$false,ValueFromPipeline=$false,ValueFromPipelineByPropertyName=$true)]
        [Byte[]]$RawData
        ,
        [Parameter(Position=9,Mandatory=$false,ValueFromPipeline=$false,ValueFromPipelineByPropertyName=$true)]
        [switch]$Force 
    )

    begin
    {
        if($Force.IsPresent)
        {
            if (! ([System.Diagnostics.EventLog]::Exists($LogName) -and [System.Diagnostics.EventLog]::SourceExists($Source) )) 
            {
                New-EventLog -LogName $LogName -Source $Source 
            }
        }
    }
    process 
    {
        if(!$EntryType) {Remove-Variable EntryType};
        if(!$Message) {Remove-Variable Message};
        if(!$Category) {Remove-Variable Category};
        if(!$ComputerName) {Remove-Variable ComputerName};
        if(!$RawData) {Remove-Variable RawData};
        Write-EventLog -LogName $LogName -Source $Source -EventId $EventId -EntryType $EntryType -Message $Message #-Category $Category -ComputerName $ComputerName -RawData $RawData
    }
    #end{} #no ending actions required
}

Write-EventLog2 $myLog $mySource $myEventId $myEntryType $myMessage -Force #ensure the log's created if doesn't exist
Write-EventLog2 $myLog $mySource $myEventId $myEntryType $myMessage2 -Force #just to demo that we don't get an error if source already exists 
Write-EventLog2 $myLog $mySource $myEventId $myEntryType $myMessage3 #this is how we'd generall call once we were certain the command had been called at least once with -Force
Show-EventLog #check that the above code produced the expected logs 
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

There is one fundamental flaw in this script, you need to run the whole script as admin to avoid errors of EventSource.Exists unless you ignore or hide them. But if you do that, then you can as well just use EventLog.WriteEntry that will create the source for you automatically.

There are few problems with your code. Let's try to go through them in the order they appear: 1) Specifying ValueFromPipeline for more than one parameter is not a good thing to do. Value from pipeline is meant to easily bind input from the pipeline that can't satisfy the properties defined by ValueFromPipelineByPropertyName. Usually you use it to bind the most significant and most volatile piece of information. For Item related cmdlets it is Path. In your case it should be the message.

2) Specifying Force as ValueFromPipelineByPropertyName might have unexpected results. In my point of view you should only bind data this way, not behavior. So binding Message, Source, ComputerName and LogName is OK. Binding Force is not.

3) Specifying position for every parameter is not necessary, the order is inferred by the order of the parameters by default. Specifying position is needed only if you don't need some of the parameters to be positional or you want to define them in order different from their actual usage.

4) Specifying Mandatory=$false is not necessary, it is the default.

5) Switch converts to Boolean implicitly, there is no need to explicitly check for IsPresent.

6) I prefer to use -not instead of !, it keeps you with just one style of operators, plus it's easier to spot.

7) Use parameter splatting to forward complicated calls cmdlets or other functions. Splatting is a technique where you populate a hashtable with keys names as parameter names, and values that are the actual values for that parameter. See example in the code below.

8) Optionally in PowerShell 3 and later you don't need to provide $true to set options of parameters.

9) Optionally set default value to the message type.

 #this is how we'd generall call once we were certain the command had been called at least once with -Force

Why not check in the begin even if the -Force is not specified then?

function Write-EventLog2
{
    [cmdletbinding()]
    param(  
        [Parameter(Mandatory,ValueFromPipelineByPropertyName)][String]$LogName,
        [Parameter(Mandatory,ValueFromPipelineByPropertyName)][String]$Source,
        [Parameter(Mandatory,ValueFromPipelineByPropertyName)][Int32]$EventId,
        [Parameter(ValueFromPipelineByPropertyName)][Diagnostics.EventLogEntryType]$EntryType,
        [Parameter(ValueFromPipeline,ValueFromPipelineByPropertyName)][String]$Message,
        [Parameter(ValueFromPipelineByPropertyName)][Int16]$Category,
        [Parameter(ValueFromPipelineByPropertyName)][String]$ComputerName,
        [Parameter(ValueFromPipelineByPropertyName)][Byte[]]$RawData,   
        [Switch]$Force 
    )

    begin
    {
        if($Force)
        {
            if (! ([System.Diagnostics.EventLog]::Exists($LogName) -and [System.Diagnostics.EventLog]::SourceExists($Source) )) 
            {
                New-EventLog -LogName $LogName -Source $Source 
            }
        }
    }
    process 
    {
        $params = @{
            #put mandatory params here directly
            LogName = $LogName
            Source = $Source
            EventId = $EventId
        }
        if($EntryType) { $params.Add('EntryType', $EntryType) }
        if($Message) { $params.Add('Message', $Message) }
        if($Category) {$params.Add('Category', $Category) }
        if($ComputerName) { $params.Add('ComputerName', $ComputerName) }
        if($null -ne $RawData) { $params.Add('RawData', $RawData) }

        #since your parameters are named the same way as Write-EventLog parameters, and you use cmdlet binding
        #you might get away with just specifying @PSBoundParameters
        Write-EventLog @params 
    }
}

Another option would be to implement a proxy function for the cmdlet, more info here

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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