There are a couple of real-bad things in your script, but before we get to that let's have a look at how you can parameterize your syslog function.
Parameterize your functions
Scriptblocks and functions in powershell support optionally typed parameter declarations in the aptly named param-block.
For the purposes if this answer, let's focus exclusively on the only thing that ever changes when you invoke the current function, namely the message. If we turn that into a parameter, we'll end up with a function definition that looks more like this:
function Send-SyslogEvent {
param(
[string]$Message
)
$Server = '1.1.1.1'
$Severity = '10'
$Facility = '22'
# ... rest of the function here
}
(I took the liberty of renaming it to PowerShell's characteristic Verb-Noun command naming convention).
There's a small performance-benefit to using parameters rather than global variables, but the real benefit here is that you're going to end up with clean and correct code, which will save you a headache for the rest.
IDisposable's
.NET is a "managed" runtime, meaning that we don't really need to worry about resource-management (allocating and freeing memory for example), but there are a few cases where we have to manage resources that are external to the runtime - such as network sockets used by an UDPClient object for example :)
Types that depend on these kinds of external resources usually implement the IDisposable interface, and the golden rule here is:
Who-ever creates a new IDisposable object should also dispose of it as soon as possible, preferably at latest when exiting the scope in which it was created.
So, when you create a new instance of UDPClient inside Send-SyslogEvent, you should also ensure that you always call $UDPClient.Dispose() before returning from Send-SyslogEvent. We can do that with a set of try/finally blocks:
function Send-SyslogEvent {
param(
[string]$Message
)
$Server = '1.1.1.1'
$Severity = '10'
$Facility = '22'
$Hostname= 'ServerSyslogEvents'
try{
$UDPCLient = New-Object System.Net.Sockets.UdpClient
$UDPCLient.Connect($Server, 514)
$Priority = ([int]$Facility * 8) + [int]$Severity
$Timestamp = Get-Date -Format "MMM dd HH:mm:ss"
$FullSyslogMessage = "<{0}>{1} {2} {3}" -f $Priority, $Timestamp, $Hostname, $Message
$Encoding = [System.Text.Encoding]::ASCII
$ByteSyslogMessage = $Encoding.GetBytes($FullSyslogMessage)
$UDPCLient.Send($ByteSyslogMessage, $ByteSyslogMessage.Length) | out-null
}
finally {
# this is the important part
if($UDPCLient){
$UDPCLient.Dispose()
}
}
}
Failing to dispose of IDisposable objects is one of the surest way to leak memory and cause resource contention in the operating system you're running on, so this is definitely a must, especially for performance-sensitive or frequently invoked code.
Re-use instances!
Now, I showed above how you should handle disposal of the UDPClient, but another thing you can do is re-use the same client - you'll be connecting to the same syslog host every single time anyway!
function Send-SyslogEvent {
param(
[Parameter(Mandatory = $true)]
[string]$Message,
[Parameter(Mandatory = $false)]
[System.Net.Sockets.UdpClient]$Client
)
$Server = '1.1.1.1'
$Severity = '10'
$Facility = '22'
$Hostname= 'ServerSyslogEvents'
try{
# check if an already connected UDPClient object was passed
if($PSBoundParameters.ContainsKey('Client') -and $Client.Available){
$UDPClient = $Client
$borrowedClient = $true
}
else{
$UDPClient = New-Object System.Net.Sockets.UdpClient
$UDPClient.Connect($Server, 514)
}
$Priority = ([int]$Facility * 8) + [int]$Severity
$Timestamp = Get-Date -Format "MMM dd HH:mm:ss"
$FullSyslogMessage = "<{0}>{1} {2} {3}" -f $Priority, $Timestamp, $Hostname, $Message
$Encoding = [System.Text.Encoding]::ASCII
$ByteSyslogMessage = $Encoding.GetBytes($FullSyslogMessage)
$UDPCLient.Send($ByteSyslogMessage, $ByteSyslogMessage.Length) | out-null
}
finally {
# this is the important part
# if we "borrowed" the client from the caller we won't dispose of it
if($UDPCLient -and -not $borrowedClient){
$UDPCLient.Dispose()
}
}
}
This last modification will allow us to create the UDPClient once and re-use it over and over again:
# ...
$SyslogClient = New-Object System.Net.Sockets.UdpClient
$SyslogClient.Connect($SyslogServer, 514)
foreach($file in $LogFiles)
{
# ... assign the relevant output from the logs to $message, or pass $_ directly:
Send-SyslogEvent -Message $message -Client $SyslogClient
# ...
}
Use a StreamReader instead of a switch!
Finally, if you want to minimize allocations while slurping the files, for example use File.OpenText() to create a StreamReader to read the file line-by-line:
$SyslogClient = New-Object System.Net.Sockets.UdpClient
$SyslogClient.Connect($SyslogServer, 514)
foreach($File in $LogFiles)
{
try{
$reader = [System.IO.File]::OpenText($File.FullName)
$msg = ''
while($null -ne ($line = $reader.ReadLine()))
{
if($line.StartsWith('START--'))
{
if($msg){
Send-SyslogEvent -Message $msg -Client $SyslogClient
}
$msg = $line
}
else
{
$msg = $msg,$line -join [System.Environment]::NewLine
}
}
if($msg){
# last block
Send-SyslogEvent -Message $msg -Client $SyslogClient
}
}
finally{
# Same as with UDPClient, remember to dispose of the reader.
if($reader){
$reader.Dispose()
}
}
}
This is likely going to be faster than the switch, although I doubt you'll see much improvement to the memory foot-print - simply because identical strings are interned in .NET (they're basically cached in a big in-memory pool).
Inspecting types for IDisposable
You can test if an object implements IDisposable with the -is operator:
PS C:\> $reader -is [System.IDisposable]
True
Or using Type.GetInterfaces(), as suggested by the TheIncorrigible1
PS C:\> [System.Net.Sockets.UdpClient].GetInterfaces()
IsPublic IsSerial Name
-------- -------- ----
True False IDisposable
I hope the above helps!
