Hello, I have attached the fourth version of the patch.
Regards, Sergey.
From 0303df03496ec9aafd6e69fa798177ad06a85bee Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov <sergey.dudola...@gmail.com> Date: Tue, 8 Nov 2022 18:56:26 +0100 Subject: [PATCH] Introduce 'log_connection_messages' This patch removes 'log_connections' and 'log_disconnections' in favor of 'log_connection_messages', thereby introducing incompatibility Author: Sergey Dudoladov Reviewed-by: Justin Pryzby, Jacob Champion Discussion: https://www.postgresql.org/message-id/flat/CAA8Fd-qCB96uwfgMKrzfNs32mqqysi53yZFNVaRNJ6xDthZEgA%40mail.gmail.com --- doc/src/sgml/config.sgml | 90 +++++++++++++------ src/backend/commands/variable.c | 81 +++++++++++++++++ src/backend/libpq/auth.c | 5 +- src/backend/postmaster/postmaster.c | 5 +- src/backend/tcop/postgres.c | 11 ++- src/backend/utils/init/postinit.c | 2 +- src/backend/utils/misc/guc_tables.c | 30 +++---- src/backend/utils/misc/postgresql.conf.sample | 5 +- src/include/postgres.h | 9 ++ src/include/postmaster/postmaster.h | 1 - src/include/utils/guc_hooks.h | 2 + src/test/authentication/t/001_password.pl | 2 +- src/test/authentication/t/003_peer.pl | 2 +- src/test/kerberos/t/001_auth.pl | 2 +- src/test/ldap/t/001_auth.pl | 2 +- src/test/ldap/t/002_bindpasswd.pl | 2 +- src/test/recovery/t/013_crash_restart.pl | 2 +- src/test/recovery/t/022_crash_temp_files.pl | 2 +- src/test/recovery/t/032_relfilenode_reuse.pl | 2 +- src/test/ssl/t/SSL/Server.pm | 2 +- src/tools/ci/pg_ci_base.conf | 3 +- 21 files changed, 187 insertions(+), 75 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 2073bafa1f..42c0a1f3db 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -140,7 +140,7 @@ An example of what this file might look like is: <programlisting> # This is a comment -log_connections = yes +log_connection_messages = all log_destination = 'syslog' search_path = '"$user", public' shared_buffers = 128MB @@ -335,7 +335,7 @@ UPDATE pg_settings SET setting = reset_val WHERE name = 'configuration_parameter passed to the <command>postgres</command> command via the <option>-c</option> command-line parameter. For example, <programlisting> -postgres -c log_connections=yes -c log_destination='syslog' +postgres -c log_connection_messages=all -c log_destination='syslog' </programlisting> Settings provided in this way override those set via <filename>postgresql.conf</filename> or <command>ALTER SYSTEM</command>, @@ -7124,23 +7124,75 @@ local0.* /var/log/postgresql </listitem> </varlistentry> - <varlistentry id="guc-log-connections" xreflabel="log_connections"> - <term><varname>log_connections</varname> (<type>boolean</type>) + <varlistentry id="guc-log-connection-messages" xreflabel="log_connection_messages"> + <term><varname>log_connection_messages</varname> (<type>string</type>) <indexterm> - <primary><varname>log_connections</varname> configuration parameter</primary> + <primary><varname>log_connection_messages</varname> configuration parameter</primary> </indexterm> </term> <listitem> <para> - Causes each attempted connection to the server to be logged, - as well as successful completion of both client authentication (if - necessary) and authorization. + Causes the specified stages of each connection attempt to be logged. + Example: <literal>authorized,disconnected</literal>. + The default is the empty string, which means that nothing is logged. Only superusers and users with the appropriate <literal>SET</literal> privilege can change this parameter at session start, and it cannot be changed at all within a session. - The default is <literal>off</literal>. </para> + <table id="connection-messages"> + <title>Connection messages</title> + <tgroup cols="2"> + <colspec colname="col1" colwidth="1*"/> + <colspec colname="col2" colwidth="2*"/> + <thead> + <row> + <entry>Name</entry> + <entry>Description</entry> + </row> + </thead> + <tbody> + <row> + <entry><literal>received</literal></entry> + <entry>Logs receipt of a connection. At this point a connection has been + received, but no further work has been done: PostgreSQL is about to start + interacting with a client.</entry> + </row> + + <row> + <entry><literal>authenticated</literal></entry> + <entry>Logs the original identity used by an authentication method + to identify a user. In most cases, the identity string matches the + PostgreSQL username, but some third-party authentication methods may + alter the original user identifier before the server stores it. Failed + authentication is always logged regardless of the value of this setting. + </entry> + </row> + + <row> + <entry><literal>authorized</literal></entry> + <entry>Logs successful completion of authorization. At this point the + connection has been fully established. + </entry> + </row> + + <row> + <entry><literal>disconnected</literal></entry> + <entry>Logs session termination. The log output provides information + similar to <literal>authorized</literal>, plus the duration of the session. + </entry> + </row> + + <row> + <entry><literal>all</literal></entry> + <entry>A convenience alias. If present, it must be the only value in the + list.</entry> + </row> + + </tbody> + </tgroup> + </table> + <note> <para> Some client programs, like <application>psql</application>, attempt @@ -7152,26 +7204,6 @@ local0.* /var/log/postgresql </listitem> </varlistentry> - <varlistentry id="guc-log-disconnections" xreflabel="log_disconnections"> - <term><varname>log_disconnections</varname> (<type>boolean</type>) - <indexterm> - <primary><varname>log_disconnections</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - Causes session terminations to be logged. The log output - provides information similar to <varname>log_connections</varname>, - plus the duration of the session. - Only superusers and users with the appropriate <literal>SET</literal> - privilege can change this parameter at session start, - and it cannot be changed at all within a session. - The default is <literal>off</literal>. - </para> - </listitem> - </varlistentry> - - <varlistentry id="guc-log-duration" xreflabel="log_duration"> <term><varname>log_duration</varname> (<type>boolean</type>) <indexterm> diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c index f0f2e07655..d7d01fb495 100644 --- a/src/backend/commands/variable.c +++ b/src/backend/commands/variable.c @@ -993,6 +993,78 @@ show_role(void) } +/* check_hook: validate new log_connection_messages value + * + * The implementation follows the check_log_destination hook. + */ +bool +check_log_connection_messages(char **newval, void **extra, GucSource source) +{ + char *rawname; + List *namelist; + ListCell *l; + int newlogconnect = 0; + int *myextra; + + if (pg_strcasecmp(*newval, "all") == 0) + { + newlogconnect |= LOG_CONNECTION_ALL; + myextra = (int *) guc_malloc(LOG, sizeof(int)); + if (!myextra) + return false; + *myextra = newlogconnect; + *extra = (void *) myextra; + return true; + } + + /* Need a modifiable copy of string */ + rawname = pstrdup(*newval); + + /* Parse string into list of identifiers */ + /* We rely on SplitIdentifierString to downcase and strip whitespace */ + if (!SplitIdentifierString(rawname, ',', &namelist)) + { + /* syntax error in name list */ + GUC_check_errdetail("List syntax is invalid."); + pfree(rawname); + list_free(namelist); + return false; + } + + foreach(l, namelist) + { + char *stage = (char *) lfirst(l); + if (pg_strcasecmp(stage, "received") == 0) + newlogconnect |= LOG_CONNECTION_RECEIVED; + else if (pg_strcasecmp(stage, "authenticated") == 0) + newlogconnect |= LOG_CONNECTION_AUTHENTICATED; + else if (pg_strcasecmp(stage, "authorized") == 0) + newlogconnect |= LOG_CONNECTION_AUTHORIZED; + else if (pg_strcasecmp(stage, "disconnected") == 0) + newlogconnect |= LOG_CONNECTION_DISCONNECTED; + else { + GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE); + GUC_check_errmsg("invalid value for parameter '%s'", stage); + GUC_check_errdetail("Valid values to use in the list are 'all', 'received', 'authenticated', 'authorized', and 'disconnected'." + " If 'all' is present, it must be the only value in the list."); + pfree(rawname); + list_free(namelist); + return false; + } + } + + pfree(rawname); + list_free(namelist); + + myextra = (int *) guc_malloc(LOG, sizeof(int)); + if (!myextra) + return false; + *myextra = newlogconnect; + *extra = (void *) myextra; + + return true; +} + /* * PATH VARIABLES * @@ -1015,6 +1087,15 @@ check_canonical_path(char **newval, void **extra, GucSource source) /* + * assign_log_connection_messages: GUC assign_hook for log_connection_messages + */ +void +assign_log_connection_messages(const char *newval, void *extra) +{ + Log_connection_messages = *((int *) extra); +} + + /* * MISCELLANEOUS */ diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index a1a826e37f..671c901814 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -332,7 +332,8 @@ auth_failed(Port *port, int status, const char *logdetail) /* * Sets the authenticated identity for the current user. The provided string * will be stored into MyClientConnectionInfo, alongside the current HBA - * method in use. The ID will be logged if log_connections is enabled. + * method in use. The ID will be logged if + * log_connection_messages contains the 'authenticated' value. * * Auth methods should call this routine exactly once, as soon as the user is * successfully authenticated, even if they have reasons to know that @@ -364,7 +365,7 @@ set_authn_id(Port *port, const char *id) MyClientConnectionInfo.authn_id = MemoryContextStrdup(TopMemoryContext, id); MyClientConnectionInfo.auth_method = port->hba->auth_method; - if (Log_connections) + if (Log_connection_messages & LOG_CONNECTION_AUTHENTICATED) { ereport(LOG, errmsg("connection authenticated: identity=\"%s\" method=%s " diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 4c49393fc5..7e79736feb 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -235,7 +235,6 @@ int PreAuthDelay = 0; int AuthenticationTimeout = 60; bool log_hostname; /* for ps display and logging */ -bool Log_connections = false; bool Db_user_namespace = false; bool enable_bonjour = false; @@ -4343,8 +4342,8 @@ BackendInitialize(Port *port) port->remote_host = strdup(remote_host); port->remote_port = strdup(remote_port); - /* And now we can issue the Log_connections message, if wanted */ - if (Log_connections) + /* And now we can issue the "connection received" message, if wanted */ + if (Log_connection_messages & LOG_CONNECTION_RECEIVED) { if (remote_port[0]) ereport(LOG, diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 01b6cc1f7d..c424e6ee7e 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -87,8 +87,8 @@ const char *debug_query_string; /* client-supplied query string */ /* Note: whereToSendOutput is initialized for the bootstrap/standalone case */ CommandDest whereToSendOutput = DestDebug; -/* flag for logging end of session */ -bool Log_disconnections = false; +/* flags for logging information about session state */ +int Log_connection_messages = 0; int log_statement = LOGSTMT_NONE; @@ -3650,8 +3650,7 @@ set_debug_options(int debug_flag, GucContext context, GucSource source) if (debug_flag >= 1 && context == PGC_POSTMASTER) { - SetConfigOption("log_connections", "true", context, source); - SetConfigOption("log_disconnections", "true", context, source); + SetConfigOption("log_connection_messages", "all", context, source); } if (debug_flag >= 2) SetConfigOption("log_statement", "all", context, source); @@ -4221,9 +4220,9 @@ PostgresMain(const char *dbname, const char *username) /* * Also set up handler to log session end; we have to wait till now to be - * sure Log_disconnections has its final value. + * sure Log_connection_messages has its final value. */ - if (IsUnderPostmaster && Log_disconnections) + if (IsUnderPostmaster && (Log_connection_messages & LOG_CONNECTION_DISCONNECTED)) on_proc_exit(log_disconnections, 0); pgstat_report_connect(MyDatabaseId); diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 53420f4974..93eeb631fe 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -250,7 +250,7 @@ PerformAuthentication(Port *port) */ disable_timeout(STATEMENT_TIMEOUT, false); - if (Log_connections) + if (Log_connection_messages & LOG_CONNECTION_AUTHORIZED) { StringInfoData logmsg; diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 8abf9bb644..952b1888de 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -90,7 +90,6 @@ #endif /* XXX these should appear in other modules' header files */ -extern bool Log_disconnections; extern int CommitDelay; extern int CommitSiblings; extern char *default_tablespace; @@ -605,6 +604,7 @@ static char *recovery_target_string; static char *recovery_target_xid_string; static char *recovery_target_name_string; static char *recovery_target_lsn_string; +static char *log_connection_messages_string; /* should be static, but commands/variable.c needs to get at this */ char *role_string; @@ -1202,24 +1202,6 @@ struct config_bool ConfigureNamesBool[] = true, NULL, NULL, NULL }, - { - {"log_connections", PGC_SU_BACKEND, LOGGING_WHAT, - gettext_noop("Logs each successful connection."), - NULL - }, - &Log_connections, - false, - NULL, NULL, NULL - }, - { - {"log_disconnections", PGC_SU_BACKEND, LOGGING_WHAT, - gettext_noop("Logs end of a session, including duration."), - NULL - }, - &Log_disconnections, - false, - NULL, NULL, NULL - }, { {"log_replication_commands", PGC_SUSET, LOGGING_WHAT, gettext_noop("Logs each replication command."), @@ -4206,6 +4188,16 @@ struct config_string ConfigureNamesString[] = check_session_authorization, assign_session_authorization, NULL }, + { + {"log_connection_messages", PGC_SU_BACKEND, LOGGING_WHAT, + gettext_noop("Lists connection stages to log."), + NULL, + GUC_LIST_INPUT + }, + &log_connection_messages_string, + "", + check_log_connection_messages, assign_log_connection_messages, NULL + }, { {"log_destination", PGC_SIGHUP, LOGGING_WHERE, gettext_noop("Sets the destination for server log output."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 11a4cf6cfb..460d79b3a3 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -21,7 +21,7 @@ # require a server shutdown and restart to take effect. # # Any parameter can also be given as a command-line option to the server, e.g., -# "postgres -c log_connections=on". Some parameters can be changed at run time +# "postgres -c log_connection_messages=all". Some parameters can be changed at run time # with the "SET" SQL command. # # Memory units: B = bytes Time units: us = microseconds @@ -557,8 +557,7 @@ # actions running at least this number # of milliseconds. #log_checkpoints = on -#log_connections = off -#log_disconnections = off +#log_connection_messages = '' #log_duration = off #log_error_verbosity = default # terse, default, or verbose messages #log_hostname = off diff --git a/src/include/postgres.h b/src/include/postgres.h index 8a028ff789..9f888f4aef 100644 --- a/src/include/postgres.h +++ b/src/include/postgres.h @@ -576,4 +576,13 @@ extern Datum Float8GetDatum(float8 X); #define NON_EXEC_STATIC static #endif +extern PGDLLIMPORT int Log_connection_messages; + +/* Bitmap for logging connection messages */ +#define LOG_CONNECTION_RECEIVED (1 << 0) +#define LOG_CONNECTION_AUTHENTICATED (1 << 1) +#define LOG_CONNECTION_AUTHORIZED (1 << 2) +#define LOG_CONNECTION_DISCONNECTED (1 << 3) +#define LOG_CONNECTION_ALL 0xFFFF + #endif /* POSTGRES_H */ diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 3b3889c58c..e1ad7e1c50 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -25,7 +25,6 @@ extern PGDLLIMPORT char *ListenAddresses; extern PGDLLIMPORT bool ClientAuthInProgress; extern PGDLLIMPORT int PreAuthDelay; extern PGDLLIMPORT int AuthenticationTimeout; -extern PGDLLIMPORT bool Log_connections; extern PGDLLIMPORT bool log_hostname; extern PGDLLIMPORT bool enable_bonjour; extern PGDLLIMPORT char *bonjour_name; diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index 2ecb9fc086..ea98ac35f0 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -69,6 +69,8 @@ extern bool check_locale_numeric(char **newval, void **extra, GucSource source); extern void assign_locale_numeric(const char *newval, void *extra); extern bool check_locale_time(char **newval, void **extra, GucSource source); extern void assign_locale_time(const char *newval, void *extra); +extern bool check_log_connection_messages(char **newval, void **extra, GucSource source); +extern void assign_log_connection_messages(const char *newval, void *extra); extern bool check_log_destination(char **newval, void **extra, GucSource source); extern void assign_log_destination(const char *newval, void *extra); diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index 0680f8b07c..7474bc8d85 100644 --- a/src/test/authentication/t/001_password.pl +++ b/src/test/authentication/t/001_password.pl @@ -63,7 +63,7 @@ sub test_conn # Initialize primary node my $node = PostgreSQL::Test::Cluster->new('primary'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connection_messages = all\n"); $node->start; # Create 3 roles with different password methods for each one. The same diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl index a6be651ea7..4ad97da2f4 100644 --- a/src/test/authentication/t/003_peer.pl +++ b/src/test/authentication/t/003_peer.pl @@ -82,7 +82,7 @@ sub find_in_log my $node = PostgreSQL::Test::Cluster->new('node'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connection_messages = all\n"); $node->start; # Set pg_hba.conf with the peer authentication. diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl index e2c928349f..8f1dc31fb3 100644 --- a/src/test/kerberos/t/001_auth.pl +++ b/src/test/kerberos/t/001_auth.pl @@ -216,7 +216,7 @@ $node->append_conf( 'postgresql.conf', qq{ listen_addresses = '$hostaddr' krb_server_keyfile = '$keytab' -log_connections = on +log_connection_messages = all lc_messages = 'C' }); $node->start; diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl index 1e027ced01..783b05d952 100644 --- a/src/test/ldap/t/001_auth.pl +++ b/src/test/ldap/t/001_auth.pl @@ -48,7 +48,7 @@ note "setting up PostgreSQL instance"; my $node = PostgreSQL::Test::Cluster->new('node'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connection_messages = all\n"); $node->start; $node->safe_psql('postgres', 'CREATE USER test0;'); diff --git a/src/test/ldap/t/002_bindpasswd.pl b/src/test/ldap/t/002_bindpasswd.pl index bcd4aa2b74..e274dbf928 100644 --- a/src/test/ldap/t/002_bindpasswd.pl +++ b/src/test/ldap/t/002_bindpasswd.pl @@ -44,7 +44,7 @@ note "setting up PostgreSQL instance"; my $node = PostgreSQL::Test::Cluster->new('node'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connection_messages = 'all'\n"); $node->start; $node->safe_psql('postgres', 'CREATE USER test0;'); diff --git a/src/test/recovery/t/013_crash_restart.pl b/src/test/recovery/t/013_crash_restart.pl index 92e7b367df..ff0bb3daf0 100644 --- a/src/test/recovery/t/013_crash_restart.pl +++ b/src/test/recovery/t/013_crash_restart.pl @@ -27,7 +27,7 @@ $node->start(); $node->safe_psql( 'postgres', q[ALTER SYSTEM SET restart_after_crash = 1; - ALTER SYSTEM SET log_connections = 1; + ALTER SYSTEM SET log_connection_messages = 'all'; SELECT pg_reload_conf();]); # Run psql, keeping session alive, so we have an alive backend to kill. diff --git a/src/test/recovery/t/022_crash_temp_files.pl b/src/test/recovery/t/022_crash_temp_files.pl index 03c8efdfb5..2a466da25b 100644 --- a/src/test/recovery/t/022_crash_temp_files.pl +++ b/src/test/recovery/t/022_crash_temp_files.pl @@ -26,7 +26,7 @@ $node->start(); $node->safe_psql( 'postgres', q[ALTER SYSTEM SET remove_temp_files_after_crash = on; - ALTER SYSTEM SET log_connections = 1; + ALTER SYSTEM SET log_connection_messages = 'all'; ALTER SYSTEM SET work_mem = '64kB'; ALTER SYSTEM SET restart_after_crash = on; SELECT pg_reload_conf();]); diff --git a/src/test/recovery/t/032_relfilenode_reuse.pl b/src/test/recovery/t/032_relfilenode_reuse.pl index 92ec510037..f4ac72ef62 100644 --- a/src/test/recovery/t/032_relfilenode_reuse.pl +++ b/src/test/recovery/t/032_relfilenode_reuse.pl @@ -11,7 +11,7 @@ $node_primary->init(allows_streaming => 1); $node_primary->append_conf( 'postgresql.conf', q[ allow_in_place_tablespaces = true -log_connections=on +log_connection_messages=all # to avoid "repairing" corruption full_page_writes=off log_min_messages=debug2 diff --git a/src/test/ssl/t/SSL/Server.pm b/src/test/ssl/t/SSL/Server.pm index b6344b936a..6cd5a46921 100644 --- a/src/test/ssl/t/SSL/Server.pm +++ b/src/test/ssl/t/SSL/Server.pm @@ -193,7 +193,7 @@ sub configure_test_server_for_ssl # enable logging etc. open my $conf, '>>', "$pgdata/postgresql.conf"; print $conf "fsync=off\n"; - print $conf "log_connections=on\n"; + print $conf "log_connection_messages=all\n"; print $conf "log_hostname=on\n"; print $conf "listen_addresses='$serverhost'\n"; print $conf "log_statement=all\n"; diff --git a/src/tools/ci/pg_ci_base.conf b/src/tools/ci/pg_ci_base.conf index d8faa9c26c..7596d515aa 100644 --- a/src/tools/ci/pg_ci_base.conf +++ b/src/tools/ci/pg_ci_base.conf @@ -8,7 +8,6 @@ max_prepared_transactions = 10 # Settings that make logs more useful log_autovacuum_min_duration = 0 log_checkpoints = true -log_connections = true -log_disconnections = true +log_connection_messages = all log_line_prefix = '%m [%p][%b] %q[%a][%v:%x] ' log_lock_waits = true -- 2.34.1
