views:

225

answers:

3

Does anyone see a performance issue with my logon Trigger?

I'm trying to reduce the overhead and prevent any performance issues before I push this trigger to my production SQL Server.

I currently have the logon trigger working on my Development sql server. I let it run over the past weekend and it put 50,000+ rows into my audit log table. I noticed that 95% of the records where for the logon 'NT AUTHORITY/SYSTEM'. So I decided to filter anything with 'NT AUTHORITY%' and just not insert those records. My thinking is if I do filter these 'NT AUTHORITY' records that the amount of resources I'll save on those inserts will make up for the cost of the IF statement check. I also have been watching Prefmon and don't see anything unusual while the trigger is enabled, but then again my Development server dosn't see the same amount of activity as production.

USE [MASTER]
GO

CREATE TRIGGER AuditServerAuthentication
ON ALL SERVER
WITH EXECUTE AS SELF
FOR LOGON
AS BEGIN

DECLARE @event XML, @Logon_Name VARCHAR(100)
SET @Event = EVENTDATA()
SET @Logon_Name =  CAST(@event.query('/EVENT_INSTANCE/LoginName/text()') AS VARCHAR(100))

IF @Logon_Name NOT LIKE 'NT AUTHORITY%'
BEGIN
    INSERT INTO Auditing.Audit.Authentication_Log 
     (Post_Time,Event_Type,Login_Name,Client_Host,Application_Name,Event_Data)
    VALUES
     (
      CAST(CAST(@event.query('/EVENT_INSTANCE/PostTime/text()') AS VARCHAR(64)) AS DATETIME),
      CAST(@event.query('/EVENT_INSTANCE/EventType/text()') AS VARCHAR(100)),
      CAST(@event.query('/EVENT_INSTANCE/LoginName/text()') AS VARCHAR(100)),
      CAST(@event.query('/EVENT_INSTANCE/ClientHost/text()') AS VARCHAR(100)),
      APP_NAME(),
      @Event
     )
END
END
GO
A: 

It doesn't look like a costly IF statement at all to me (it's not like you are selecting anything from the database) and, as you say, would be far less costly than performing an INSERT that is isn't necessary 95% of the time. However, I should add I am just a database programmer and not a DBA, so am open to being corrected here.

I am, though, slightly curious as to why you are doing this? Doesn't SQL Server already have a built-in mechanism for Login Auditing that you can use?

Dan Diplo
The "built-in mechanism for Login Auditing" isn't easy to sort through and doesn't provide all the info to build custom reports with. DBAs like being able to select from tables to get our answers. :)
DBAndrew
A: 

There's nothing immediately obvious. It's effectively an IF that protects an INSERT. The only thing I would validate is how expensive the XML parsing is - I've not used it in SQL Server yet, so it's an unknown to me.

Admittedly, it would seem odd for Microsoft to supply an easy way to get metadata (EVENTDATA()) yet make it expensive to parse, yet stranger things have happened...

Chris J
+1  A: 

I use a very similar trigger in my servers, and i haven't experience any performance issues. The production DB gets about 10 logins per second. This creates a huge amount of data over time which translates to bigger backups, etc.

For some servers i created a table with the users that logins shouldn't be logged, with this is also possible to refuse logins according to working hours

The difference with my trigger is that I've created a DB for auditing purposes, in which i created some stored procedures that i call in the trigger. The trigger looks like this

alter TRIGGER [tr_AU_LogonLog] ON ALL SERVER
WITH EXECUTE AS 'AUDITUSER'
FOR LOGON
AS
BEGIN
    DECLARE
        @data XML
      , @rc INT
    SET @data = EVENTDATA()
    EXEC @rc = AuditDB.dbo.LogonLog @data
END ;

The production DB gets about 10 logins per second. This creates a huge amount of data over time which translates to bigger backups, etc.

EDIT: oh i forgot, its recommended if you create a specific user for the trigger, execute as self can be dangerous in some scenarios.

EDIT2: There's some useful information about the execute as statement here. oh and be careful while implementing triggers, you can accidentally lock yourself out, i recommend keeping a connection open just in case :)

Alan FL