MDEV-18386: Add server_audit_timestamp_format to customize audit log timestamps by abhishek593 · Pull Request #4633 · MariaDB/server

abhishek593 · 2026-02-08T20:18:11Z

This PR adds a new global system variable, server_audit_timestamp_format, to the MariaDB Audit Plugin.

Key changes:

Users can now specify a custom strftime format string for audit log timestamps.
Default behavior is preserved if the variable is left empty.
Added a new test case server_audit_timestamp.test to verify custom formats, including timezone offsets (%z).
Updated existing audit tests to reflect the new system variable.

…timestamps

gkodinov

This is a preliminary review. Thank you for your contribution!

plugin/server_audit/server_audit.cc

gkodinov

it was a good first iteration. Thanks for working on that. Another round of comments coming up.

plugin/server_audit/server_audit.cc

…audit logging

gkodinov

LGTM after fixing the below. Please expect a final review.

gkodinov · 2026-02-13T11:15:59Z

plugin/server_audit/server_audit.cc

       NULL, NULL, 1024, 0, 0x7FFFFFFF, 1);
+static MYSQL_SYSVAR_STR(timestamp_format, timestamp_format,
+       PLUGIN_VAR_RQCMDARG,
+       "The format string the strftime() routine applies with its format argument",


I'd say: a format string used to print the timestamp into the audit log messages. The format used is the same as strftime. Default is empty, meaning use the existing hard-coded format.

gkodinov · 2026-02-13T11:17:25Z

plugin/server_audit/server_audit.cc

+      if (timestamp_format_buffer[0])
+        ts_len= strftime(ts_tmp, sizeof(ts_tmp), timestamp_format_buffer, &tm_time);
+
+      if (ts_len == 0)


just do "else" here. Easier to read.

gkodinov · 2026-02-13T11:18:45Z

plugin/server_audit/server_audit.cc

    {
-      if (!(is_active= (logger_write(logfile, message, len) == (int) len)))
+      struct tm tm_time;
+      char ts_tmp[TIMESTAMP_OUTPUT_LENGTH];


256 bytes in the stack ... this is a bit much. I'd allocate a buffer outside of the lock and pass it down. Or, better, it's going at the start of message anyway, can't you just print it there directly instead of using a temp buffer and then memcpy() into it?

Yeah, so here's what I changed. Removed the temp buffer, and directly stored the output of strftime in message buffer at the beginning, then used memmove to shift the output from the start to the end of reserved space.

gkodinov · 2026-02-13T11:27:12Z

plugin/server_audit/server_audit.cc

 /**
  Write to the log, acquiring the lock.
-*/
+ */


don't do white-space only changes please!

gkodinov · 2026-02-13T11:27:46Z

plugin/server_audit/server_audit.cc

-                      const char *userip, unsigned int userip_len,
-                      unsigned int connection_id, unsigned int port,
-                      long long query_id, const char *operation)
+                        const char *serverhost, size_t serverhost_len,


no white-space only changes please

gkodinov · 2026-02-13T11:28:19Z

plugin/server_audit/server_audit.cc

  time_t ctime;
  size_t csize;
-  char message[1024];
+  char raw_message[1024 + TIMESTAMP_OUTPUT_LENGTH];


I'd use some named const size_t for the 1k constant

gkodinov · 2026-02-13T11:29:57Z

plugin/server_audit/server_audit.cc


  (void) time(&ctime);
-  csize= log_header(message, sizeof(message)-1, &ctime,
+  csize= log_header(message, 1024-1,


why do you need to do this? Can't you just pass the actual length as you used to do? And then do the calculation inside this func?

Previously message was a fixed size array (1024). In the new version, its a pointer to an offset within a large buffer, so we must explicitly pass the correct size (MAX_AUDIT_PAYLOAD_LENGTH).

gkodinov · 2026-02-13T11:30:35Z

plugin/server_audit/server_audit.cc

  time_t ctime;
  size_t csize;
-  char message[1024];
+  char raw_message[1024 + TIMESTAMP_OUTPUT_LENGTH];


same here: use a named constant for 1k

plugin/server_audit/server_audit.cc

vuvova · 2026-02-21T09:04:58Z

plugin/server_audit/server_audit.cc

+            timestamp_format);
+    CLIENT_ERROR(1, "server_audit_timestamp_format can't exceed %d characters.",
+                 MYF(ME_WARNING), TIMESTAMP_FORMAT_LENGTH - 1);
+    return;


validity checks must be done in the check callback, not in the update callbac

vuvova · 2026-02-21T09:14:31Z

plugin/server_audit/server_audit.cc

+       PLUGIN_VAR_RQCMDARG,
+       "A format string used to print the timestamp into the audit log messages "
+       "The format used is the same as strftime and the default is empty which uses "
+       "the existing hard-coded format",


Small issue: don't make it "empty = hard-coded", it's not very intuitive, simply set the default value of timestamp_format to the correct (backward compatible) value.

Big issue: don't use strftime. The server already has date-to-string formatting function DATE_FORMAT(). We should not let different parts of the server to implement formatting differently. For example, %M should always and everywhere have the same meaning.

That is, invoke server's date-to-string function. And to do that, it must be in a service. But, perhaps, you can add a new method to the timezone service and not create a new one.

I have updated the implementation and added a new method in the timezone service.

…TE_FORMAT

vuvova

Looks quite good. The service is almost correct and its usage is correct. %z should print global time zone. And I don't quite understand why you moved time->string conversion away from log_header

vuvova · 2026-02-22T16:06:05Z

mysql-test/suite/plugins/r/server_audit_timestamp.result

+select 5;
+5
+5
+set global server_audit_timestamp_format='%Y%m%d %H:%i:%s';


you can try set global server_audit_timestamp_format=default instead

vuvova · 2026-02-22T16:08:24Z

mysql-test/suite/plugins/t/server_audit_timestamp.opt

+--plugin-load-add=server_audit
+--server_audit_file_path='server_audit_timestamp.log'
+--server_audit_output_type=file
+--server_audit_events='query'


let's test setting of timestamp format on the command line too

vuvova · 2026-02-22T16:09:05Z

mysql-test/suite/plugins/t/server_audit_timestamp.test

+set global server_audit_timestamp_format='CUSTOM-DATE %Y-%m-%d';
+select 2;
+
+# Custom format 2: Including timezone (if supported by system strftime)


you can now remove references to the "system strftime" from comments

vuvova · 2026-02-22T17:20:53Z

plugin/server_audit/server_audit.cc

+#endif
+#ifndef MAX_AUDIT_QUERY_LENGTH
+#define MAX_AUDIT_QUERY_LENGTH 2048
+#endif


let's drop these #ifndef/#endif, simply define what you want. Easier to read and we don't really expect anyone to define this outside of the plugin

vuvova · 2026-02-22T17:33:20Z

sql/item_timefunc.cc

+  str.length(0);
+  make_date_time(thd ? thd : current_thd, &fmt, ltime, MYSQL_TIMESTAMP_DATETIME,
+                 thd ? thd->variables.lc_time_names :
+                       global_system_variables.lc_time_names,


The service is good. How you use it is good. It is very correct to pass NULL as the thd here:

thd_gmt_sec_to_TIME(NULL, &ltime, ts); ... thd_TIME_to_str(NULL, &ltime, timestamp_format_buffer,

which means "use global time zone and locale", because session settings should not affect the global audit log.

But make_date_time() uses session time zone for %z format. I suspect you need to modify make_date_time to take a time zone as an argument, just as it takes locale.

vuvova · 2026-02-22T17:36:06Z

plugin/server_audit/server_audit.cc

-      "%04d%02d%02d %02d:%02d:%02d,%.*s,%.*s,%.*s%s,%d,%lld,%s",
-      tm_time.tm_year+1900, tm_time.tm_mon+1, tm_time.tm_mday,
-      tm_time.tm_hour, tm_time.tm_min, tm_time.tm_sec,
+      ",%.*s,%.*s,%.*s%s,%d,%lld,%s",


I thought it'd be natural to generate the timestamp string here and print like

"%s,%.*s,%.*s,%.*s%s,%d,%lld,%s", ts_start, ...

Why did you move it to write_log() ?

abhishek593 added 2 commits February 9, 2026 01:44

MDEV-18386: Add server_audit_timestamp_format to customize audit log …

069fde7

…timestamps

MDEV-18386: Fix server_audit_timestamp tests

1d7a286

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Feb 9, 2026

gkodinov self-assigned this Feb 11, 2026

Merge branch 'main' into MDEV-18386

828f298

gkodinov requested changes Feb 11, 2026

View reviewed changes

plugin/server_audit/server_audit.cc Outdated Show resolved Hide resolved

abhishek593 added 2 commits February 11, 2026 23:39

MDEV-18386: Make server_audit_timestamp_format thread-safe and robust

ab99fdb

MDEV-18386: server_audit: Fix re-entrant deadlock in log_header()

8b9e22c

gkodinov requested changes Feb 12, 2026

View reviewed changes

abhishek593 added 2 commits February 13, 2026 02:32

MDEV-18386: Address review comments; implement optimized single-lock …

44cf2fb

…audit logging

MDEV-18386: Fix regressions

0a98e58

gkodinov approved these changes Feb 13, 2026

View reviewed changes

gkodinov assigned vuvova and unassigned gkodinov Feb 13, 2026

MDEV-18386: Addressed review comments

974a559

vuvova self-requested a review February 21, 2026 09:01

vuvova requested changes Feb 21, 2026

View reviewed changes

MDEV-18386: Addressed review comments; remove strftime in favor of DA…

9e88050

…TE_FORMAT

abhishek593 requested a review from vuvova February 22, 2026 08:31

vuvova requested changes Feb 22, 2026

View reviewed changes

vuvova reviewed Feb 22, 2026

View reviewed changes

Uh oh!

Comments

Conversation

abhishek593 commented Feb 8, 2026

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vuvova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers