[PATCH] Include application_name in "connection authorized" log message
Good afternoon, all.
In trying to troubleshoot the source of a recent connection storm, I was
frustrated to find that the app name was not included in the connection
messages. It is there in the log_line_prefix on the disconnection messages
but I would prefer it be directly visible with the connection itself. With
some guidance from Stephen Frost I've put together this patch which does
that.
It adds a new application_name field to the Port struct, stores the
application name there when processing the startup packet, and then reads
from there when logging the "connection authorized" message.
Thanks,
Don.
--
Don Seiler
www.seiler.us
Attachments:
appname_log.patchapplication/octet-stream; name=appname_log.patchDownload+9-7
Greetings,
* Don Seiler (don@seiler.us) wrote:
In trying to troubleshoot the source of a recent connection storm, I was
frustrated to find that the app name was not included in the connection
messages. It is there in the log_line_prefix on the disconnection messages
but I would prefer it be directly visible with the connection itself. With
some guidance from Stephen Frost I've put together this patch which does
that.
Yeah, I tend to agree that it'd be extremely useful to have this
included in the 'connection authorized' message.
It adds a new application_name field to the Port struct, stores the
application name there when processing the startup packet, and then reads
from there when logging the "connection authorized" message.
Right, to have this included in the 'connection authorized' message, we
need to have it be captured early on, prior to generic GUC handling, and
stored momentairly to be used by the 'connection authorized' message.
Using Port for that seems reasonable (and we already store other things
like user and database there).
Taking a look at the patch itself...
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 7698cd1f88..088ef346a8 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -135,6 +135,7 @@ typedef struct Port
*/
char *database_name;
char *user_name;
+ char *application_name;
char *cmdline_options;
List *guc_options;We should have some comments here about how this is the "startup packet
application_name" and that it's specifically used for the "connection
authorized" message and that it shouldn't really be used
post-connection-startup (the GUC should be used instead, as applications
can change it post-startup).
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index a4b53b33cd..8e75c80728 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -2094,6 +2094,10 @@ retry1:
pstrdup(nameptr));
port->guc_options = lappend(port->guc_options,
pstrdup(valptr));
+
+ /* Copy application_name to port as well */
+ if (strcmp(nameptr, "application_name") == 0)
+ port->application_name = pstrdup(valptr);
}
offset = valoffset + strlen(valptr) + 1;
}That comment feels a bit lacking- this is a pretty special case which
should be explained.
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 09e0df290d..86db7630d5 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -266,8 +266,8 @@ PerformAuthentication(Port *port)
#ifdef USE_SSL
if (port->ssl_in_use)
ereport(LOG,
- (errmsg("connection authorized: user=%s database=%s SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)",
- port->user_name, port->database_name,
+ (errmsg("connection authorized: user=%s database=%s application=%s SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)",
+ port->user_name, port->database_name, port->application_name
be_tls_get_version(port),
be_tls_get_cipher(port),
be_tls_get_cipher_bits(port),
@@ -275,8 +275,8 @@ PerformAuthentication(Port *port)
else
#endif
ereport(LOG,
- (errmsg("connection authorized: user=%s database=%s",
- port->user_name, port->database_name)));
+ (errmsg("connection authorized: user=%s database=%s application=%s",
+ port->user_name, port->database_name, port->application_name)));
}
}You definitely need to be handling the case where application_name is
*not* passed in more cleanly. I don't think we should output anything
different from what we do today in those cases.
Also, these don't need to be / shouldn't be 3 seperate patches/commits,
and there should be a sensible commit message which explains what the
change is entirely.
If you could update the patch accordingly and re-send, that'd be
awesome. :)
Thanks!
Stephen
On Wed, Jun 20, 2018 at 2:45 PM, Stephen Frost <sfrost@snowman.net> wrote:
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 7698cd1f88..088ef346a8 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -135,6 +135,7 @@ typedef struct Port */ char *database_name; char *user_name; + char *application_name; char *cmdline_options; List *guc_options;We should have some comments here about how this is the "startup packet
application_name" and that it's specifically used for the "connection
authorized" message and that it shouldn't really be used
post-connection-startup (the GUC should be used instead, as applications
can change it post-startup).
Makes sense. I've now moved that variable declaration underneath the others
with a small comment block above it.
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/ postmaster.c index a4b53b33cd..8e75c80728 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -2094,6 +2094,10 @@ retry1:pstrdup(nameptr));
port->guc_options =
lappend(port->guc_options,pstrdup(valptr)); + + /* Copy application_name to port as well */ + if (strcmp(nameptr, "application_name") == 0) + port->application_name = pstrdup(valptr); } offset = valoffset + strlen(valptr) + 1; }That comment feels a bit lacking- this is a pretty special case which
should be explained.
I've added longer comment explaining again why we are doing this here.
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/ postinit.c index 09e0df290d..86db7630d5 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -266,8 +266,8 @@ PerformAuthentication(Port *port) #ifdef USE_SSL if (port->ssl_in_use) ereport(LOG, - (errmsg("connection authorized: user=%s database=%s SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)", - port->user_name, port->database_name, + (errmsg("connection authorized: user=%s database=%s application=%s SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)", + port->user_name, port->database_name, port->application_namebe_tls_get_version(port),
be_tls_get_cipher(port),
be_tls_get_cipher_bits(port), @@ -275,8 +275,8 @@ PerformAuthentication(Port *port) else #endif ereport(LOG, - (errmsg("connection authorized: user=%s database=%s", - port->user_name, port->database_name))); + (errmsg("connection authorized: user=%s database=%s application=%s", + port->user_name, port->database_name, port->application_name))); } }You definitely need to be handling the case where application_name is
*not* passed in more cleanly. I don't think we should output anything
different from what we do today in those cases.
I've added some conditional logic similar to the ternary operator usage
in src/backend/catalog/pg_collation.c:92. If application_name is set, we'll
include "application=%s" in the log message, otherwise we make no mention
of it now (output will be identical to what we currently do).
Also, these don't need to be / shouldn't be 3 seperate patches/commits,
and there should be a sensible commit message which explains what the
change is entirely.
After much head scratching/banging on both our parts yesterday, I've
finally figured this out. Thanks again for your patience and time.
If you could update the patch accordingly and re-send, that'd be
awesome. :)
See attached.
--
Don Seiler
www.seiler.us
Attachments:
0001-Changes-to-add-application_name-to-Port-struct-so-we.patchapplication/octet-stream; name=0001-Changes-to-add-application_name-to-Port-struct-so-we.patchDownload+28-4
On Wed, Jun 20, 2018 at 3:45 PM, Stephen Frost <sfrost@snowman.net> wrote:
* Don Seiler (don@seiler.us) wrote:
In trying to troubleshoot the source of a recent connection storm, I was
frustrated to find that the app name was not included in the connection
messages. It is there in the log_line_prefix on the disconnection messages
but I would prefer it be directly visible with the connection itself. With
some guidance from Stephen Frost I've put together this patch which does
that.Yeah, I tend to agree that it'd be extremely useful to have this
included in the 'connection authorized' message.
I don't get it. It seems like a bad idea to me to copy information
that can already be logged using log_line_prefix into the message
itself. If we start doing that, we'll end up with duplicated
information all over the place, and even worse, it won't be
consistent, because different people will want different things
duplicated into different messages.
Am I missing something?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Greetings,
* Robert Haas (robertmhaas@gmail.com) wrote:
On Wed, Jun 20, 2018 at 3:45 PM, Stephen Frost <sfrost@snowman.net> wrote:
* Don Seiler (don@seiler.us) wrote:
In trying to troubleshoot the source of a recent connection storm, I was
frustrated to find that the app name was not included in the connection
messages. It is there in the log_line_prefix on the disconnection messages
but I would prefer it be directly visible with the connection itself. With
some guidance from Stephen Frost I've put together this patch which does
that.Yeah, I tend to agree that it'd be extremely useful to have this
included in the 'connection authorized' message.I don't get it. It seems like a bad idea to me to copy information
that can already be logged using log_line_prefix into the message
itself. If we start doing that, we'll end up with duplicated
information all over the place, and even worse, it won't be
consistent, because different people will want different things
duplicated into different messages.Am I missing something?
The issue here is exactly that at the point where we emit the
'connection authorized' message, we haven't processed generic GUCs from
the startup packet yet and therefore application_name isn't set as a
GUC and, as a result, isn't included in the 'connection authorized'
message, even if it's specified in log_line_prefix.
There's no way, today, to get the application name included in the
'connection authorized' message, which certainly seems unfortunate and a
bit surprising, hence this patch to fix that.
Thanks!
Stephen
On Fri, Jun 22, 2018 at 11:17 AM, Stephen Frost <sfrost@snowman.net> wrote:
The issue here is exactly that at the point where we emit the
'connection authorized' message, we haven't processed generic GUCs from
the startup packet yet and therefore application_name isn't set as a
GUC and, as a result, isn't included in the 'connection authorized'
message, even if it's specified in log_line_prefix.There's no way, today, to get the application name included in the
'connection authorized' message, which certainly seems unfortunate and a
bit surprising, hence this patch to fix that.
OK, that makes more sense, but I'm still skeptical of adding a special
case particularly for application_name.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Greetings,
* Robert Haas (robertmhaas@gmail.com) wrote:
On Fri, Jun 22, 2018 at 11:17 AM, Stephen Frost <sfrost@snowman.net> wrote:
The issue here is exactly that at the point where we emit the
'connection authorized' message, we haven't processed generic GUCs from
the startup packet yet and therefore application_name isn't set as a
GUC and, as a result, isn't included in the 'connection authorized'
message, even if it's specified in log_line_prefix.There's no way, today, to get the application name included in the
'connection authorized' message, which certainly seems unfortunate and a
bit surprising, hence this patch to fix that.OK, that makes more sense, but I'm still skeptical of adding a special
case particularly for application_name.
I'd argue that application_name, when available, makes as much sense to
have in the connection authorization message as the other hard-coded
values (user, database), and those actually do get set and included in
the log_line_prefix when connection authorized is logged, if they're
asked for. There certainly wasn't too much concern raised over adding
the SSL information to that same message, nor complaints from what I
recall.
I did also look at the other items available through log_line_prefix and
didn't see anything else that really felt like it should be included.
Thanks!
Stephen
On 2018-06-22 12:16:18 -0400, Robert Haas wrote:
On Fri, Jun 22, 2018 at 11:17 AM, Stephen Frost <sfrost@snowman.net> wrote:
The issue here is exactly that at the point where we emit the
'connection authorized' message, we haven't processed generic GUCs from
the startup packet yet and therefore application_name isn't set as a
GUC and, as a result, isn't included in the 'connection authorized'
message, even if it's specified in log_line_prefix.There's no way, today, to get the application name included in the
'connection authorized' message, which certainly seems unfortunate and a
bit surprising, hence this patch to fix that.OK, that makes more sense, but I'm still skeptical of adding a special
case particularly for application_name.
I think a fair argument could be made that you'd want to have
application_name logged exactly once, not in every line. Just to cope
with log volume. With decent log analysis tools once is enough.
Greetings,
Andres Freund
On 2018-Jun-22, Andres Freund wrote:
On 2018-06-22 12:16:18 -0400, Robert Haas wrote:
OK, that makes more sense, but I'm still skeptical of adding a special
case particularly for application_name.I think a fair argument could be made that you'd want to have
application_name logged exactly once, not in every line. Just to cope
with log volume. With decent log analysis tools once is enough.
Seems harder than it sounds ... because if the user turns off
log_connections then it's not longer in the log. And what about the
application changing it after the fact? One idea would be to have a log
line designed specifically to be printed once at connection start (if
not log_connections) and then once immediately after it changes. Am I
the only one for whom this sounds like overengineering?
I think the idea is nice, but I'm not sure about feasibility.
I further think that the idea in the OP is sound enough.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-06-22 15:26:06 -0400, Alvaro Herrera wrote:
On 2018-Jun-22, Andres Freund wrote:
On 2018-06-22 12:16:18 -0400, Robert Haas wrote:
OK, that makes more sense, but I'm still skeptical of adding a special
case particularly for application_name.I think a fair argument could be made that you'd want to have
application_name logged exactly once, not in every line. Just to cope
with log volume. With decent log analysis tools once is enough.Seems harder than it sounds ... because if the user turns off
log_connections then it's not longer in the log.
That's superuser only, so I really don't quite buy that argument.
And what about the application changing it after the fact?
I don't think that matters in a lot of scenarios. Poolers are the big
exception to that, obviously.
One idea would be to have a log line designed specifically to be
printed once at connection start (if not log_connections) and then
once immediately after it changes. Am I the only one for whom this
sounds like overengineering?
Yea. I think on balance, I don't buy that it's worth the cost. But I
don't think it's a clear cut "you don't need this".
Greetings,
Andres Freund
On 2018-Jun-22, Andres Freund wrote:
On 2018-06-22 15:26:06 -0400, Alvaro Herrera wrote:
On 2018-Jun-22, Andres Freund wrote:
On 2018-06-22 12:16:18 -0400, Robert Haas wrote:
OK, that makes more sense, but I'm still skeptical of adding a special
case particularly for application_name.I think a fair argument could be made that you'd want to have
application_name logged exactly once, not in every line. Just to cope
with log volume. With decent log analysis tools once is enough.Seems harder than it sounds ... because if the user turns off
log_connections then it's not longer in the log.That's superuser only, so I really don't quite buy that argument.
I meant if the DBA disables it in postgresql.conf then the info is
nowhere.
One idea would be to have a log line designed specifically to be
printed once at connection start (if not log_connections) and then
once immediately after it changes. Am I the only one for whom this
sounds like overengineering?Yea. I think on balance, I don't buy that it's worth the cost. But I
don't think it's a clear cut "you don't need this".
Yeah, that's true, particularly for the case of the connection pooler ...
(I'm anxious to see where the Odyssey thing goes, because pgbouncer at
this point doesn't seem to be cutting it anymore. If odyssey takes off,
we could start listening more from them on what they need.)
For the time being, I think adding it to the log_connections line is a
good change, so +1.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-06-22 15:50:06 -0400, Alvaro Herrera wrote:
On 2018-Jun-22, Andres Freund wrote:
On 2018-06-22 15:26:06 -0400, Alvaro Herrera wrote:
On 2018-Jun-22, Andres Freund wrote:
I think a fair argument could be made that you'd want to have
application_name logged exactly once, not in every line. Just to cope
with log volume. With decent log analysis tools once is enough.Seems harder than it sounds ... because if the user turns off
log_connections then it's not longer in the log.That's superuser only, so I really don't quite buy that argument.
I meant if the DBA disables it in postgresql.conf then the info is
nowhere.
I'm not following - application_name isn't logged anywhere by default
currently either.
Greetings,
Andres Freund
On 2018-Jun-22, Andres Freund wrote:
On 2018-06-22 15:50:06 -0400, Alvaro Herrera wrote:
On 2018-Jun-22, Andres Freund wrote:
On 2018-06-22 15:26:06 -0400, Alvaro Herrera wrote:
On 2018-Jun-22, Andres Freund wrote:
I think a fair argument could be made that you'd want to have
application_name logged exactly once, not in every line. Just to cope
with log volume. With decent log analysis tools once is enough.Seems harder than it sounds ... because if the user turns off
log_connections then it's not longer in the log.That's superuser only, so I really don't quite buy that argument.
I meant if the DBA disables it in postgresql.conf then the info is
nowhere.I'm not following - application_name isn't logged anywhere by default
currently either.
Right. I +1'd the proposal to have it in the log_connections line.
My mind was wandering about doing more than that (or doing something
different), but let's not derail the thread.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Le 21/06/2018 à 16:21, Don Seiler a écrit :
On Wed, Jun 20, 2018 at 2:45 PM, Stephen Frost <sfrost@snowman.net
<mailto:sfrost@snowman.net>> wrote:diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 7698cd1f88..088ef346a8 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -135,6 +135,7 @@ typedef struct Port */ char *database_name; char *user_name; + char *application_name; char *cmdline_options; List *guc_options;We should have some comments here about how this is the "startup
packet
application_name" and that it's specifically used for the "connection
authorized" message and that it shouldn't really be used
post-connection-startup (the GUC should be used instead, as
applications
can change it post-startup).Makes sense. I've now moved that variable declaration underneath the
others with a small comment block above it.diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index a4b53b33cd..8e75c80728 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -2094,6 +2094,10 @@ retry1: pstrdup(nameptr)); port->guc_options = lappend(port->guc_options, pstrdup(valptr)); + + /* Copy application_name to port as well */ + if (strcmp(nameptr, "application_name") == 0) + port->application_name = pstrdup(valptr); } offset = valoffset + strlen(valptr) + 1; }That comment feels a bit lacking- this is a pretty special case which
should be explained.I've added longer comment explaining again why we are doing this here.
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 09e0df290d..86db7630d5 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -266,8 +266,8 @@ PerformAuthentication(Port *port) #ifdef USE_SSL if (port->ssl_in_use) ereport(LOG, - (errmsg("connection authorized: user=%s database=%s SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)", - port->user_name, port->database_name, + (errmsg("connection authorized: user=%s database=%s application=%s SSL enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)", + port->user_name, port->database_name, port->application_name be_tls_get_version(port), be_tls_get_cipher(port), be_tls_get_cipher_bits(port), @@ -275,8 +275,8 @@ PerformAuthentication(Port *port) else #endif ereport(LOG, - (errmsg("connection authorized: user=%s database=%s", - port->user_name, port->database_name))); + (errmsg("connection authorized: user=%s database=%s application=%s", + port->user_name, port->database_name, port->application_name))); } }You definitely need to be handling the case where application_name is
*not* passed in more cleanly. I don't think we should output anything
different from what we do today in those cases.I've added some conditional logic similar to the ternary operator
usage in src/backend/catalog/pg_collation.c:92. If application_name is
set, we'll include "application=%s" in the log message, otherwise we
make no mention of it now (output will be identical to what we
currently do).Also, these don't need to be / shouldn't be 3 seperate
patches/commits,
and there should be a sensible commit message which explains what the
change is entirely.After much head scratching/banging on both our parts yesterday, I've
finally figured this out. Thanks again for your patience and time.If you could update the patch accordingly and re-send, that'd be
awesome. :)See attached.
--
Don Seiler
www.seiler.us <http://www.seiler.us>
Hi,
I've reviewed this patch, log entry is the following when
"log_connection = on" ajnd application_name is set:
2018-06-25 17:01:30.079 CEST [22002] LOG: connection authorized:
user=postgres database=postgres application=psql
when application_name is not set:
2018-06-25 17:04:22.653 CEST [22076] LOG: connection authorized:
user=dbuser database=test
I would prefer the following output to conform to elog.c output:
2018-06-25 17:32:46.854 CEST [23265] LOG: connection authorized:
user=postgres database=postgres application=[unknown]
I think this will make changes to src/backend/utils/init/postinit.c code
more readable with less duplicated code when written as follow:
diff --git a/src/backend/utils/init/postinit.c
b/src/backend/utils/init/postinit.c
index 09e0df290d..0b3323eaf2 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -263,11 +263,16 @@ PerformAuthentication(Port *port)
}
else
{
+ const char *appname = port->application_name;
+
+ if (appname == NULL || *appname == '\0')
+ appname = _("[unknown]");
+
#ifdef USE_SSL
if (port->ssl_in_use)
ereport(LOG,
- (errmsg("connection authorized: user=%s database=%s SSL enabled
(protocol=%s, cipher=%s, bits=%d, compression=%s)",
- port->user_name, port->database_name,
+ (errmsg("connection authorized: user=%s database=%s application=%s SSL
enabled (protocol=%s, cipher=%s, bits=%d, compression=%s)",
+ port->user_name, port->database_name, appname,
be_tls_get_version(port),
be_tls_get_cipher(port),
be_tls_get_cipher_bits(port),
@@ -275,8 +280,8 @@ PerformAuthentication(Port *port)
else
#endif
ereport(LOG,
- (errmsg("connection authorized: user=%s database=%s",
- port->user_name, port->database_name)));
+ (errmsg("connection authorized: user=%s database=%s application=%s",
+ port->user_name, port->database_name, appname)));
}
}--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
On Mon, Jun 25, 2018 at 10:50 AM, Gilles Darold <gilles.darold@dalibo.com>
wrote:
I would prefer the following output to conform to elog.c output:
2018-06-25 17:32:46.854 CEST [23265] LOG: connection authorized:
user=postgres database=postgres application=[unknown]
Hello Gilles,
The different cases were for Stephen's request earlier in this thread. I'm
happy to write it either way and agree it would be nice to not have a lot
of duplicate code. Do you want me to submit another patch?
Don.
--
Don Seiler
www.seiler.us
Greetings
On Mon, Jun 25, 2018 at 15:57 Don Seiler <don@seiler.us> wrote:
On Mon, Jun 25, 2018 at 10:50 AM, Gilles Darold <gilles.darold@dalibo.com>
wrote:I would prefer the following output to conform to elog.c output:
2018-06-25 17:32:46.854 CEST [23265] LOG: connection authorized:
user=postgres database=postgres application=[unknown]Hello Gilles,
The different cases were for Stephen's request earlier in this thread. I'm
happy to write it either way and agree it would be nice to not have a lot
of duplicate code. Do you want me to submit another patch?
I think it’s better to not change the line when application name isn’t
being set, as the latest patch has. That also matches how we handle the SSL
info- it’s only shown if ssl is in place on the connection.
Thanks!
Stephen
Show quoted text
Le 25/06/2018 à 21:57, Don Seiler a écrit :
On Mon, Jun 25, 2018 at 10:50 AM, Gilles Darold
<gilles.darold@dalibo.com <mailto:gilles.darold@dalibo.com>> wrote:I would prefer the following output to conform to elog.c output:
2018-06-25 17:32:46.854 CEST [23265] LOG: connection
authorized: user=postgres database=postgres application=[unknown]Hello Gilles,
The different cases were for Stephen's request earlier in this thread.
I'm happy to write it either way and agree it would be nice to not
have a lot of duplicate code. Do you want me to submit another patch?
Yes please send a new patch this will be easiest to manage with a single
patch. I will give it a new test and change the status to "ready for
committer". I will also prepare a patch to pgbadger to support the change.
Best regards,
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
Le 25/06/2018 à 22:32, Stephen Frost a écrit :
Greetings
On Mon, Jun 25, 2018 at 15:57 Don Seiler <don@seiler.us
<mailto:don@seiler.us>> wrote:On Mon, Jun 25, 2018 at 10:50 AM, Gilles Darold
<gilles.darold@dalibo.com <mailto:gilles.darold@dalibo.com>> wrote:I would prefer the following output to conform to elog.c output:
2018-06-25 17:32:46.854 CEST [23265] LOG: connection
authorized: user=postgres database=postgres application=[unknown]Hello Gilles,
The different cases were for Stephen's request earlier in this
thread. I'm happy to write it either way and agree it would be
nice to not have a lot of duplicate code. Do you want me to submit
another patch?I think it’s better to not change the line when application name isn’t
being set, as the latest patch has. That also matches how we handle
the SSL info- it’s only shown if ssl is in place on the connection.
Oh ok, sorry Stephen I've not seen your anwser before my last reply. I
would prefer to have the information that the application_name is
unknown but this is just to conform with the log parser and obviously
the actual patch is enough, I change the status to "ready for
committer", nothing need to be change.
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
On 21.06.18 16:21, Don Seiler wrote:
- (errmsg("connection authorized: user=%s database=%s", - port->user_name, port->database_name))); + (errmsg("connection authorized: user=%s database=%s application=%s", + port->user_name, port->database_name, port->application_name)));
Why is it "application" and not "application_name"?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Jul 2, 2018 at 2:13 AM, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
On 21.06.18 16:21, Don Seiler wrote:
- (errmsg("connection authorized: user=%s database=%s", - port->user_name, port->database_name))); + (errmsg("connection authorized: user=%s database=%s application=%s", + port->user_name, port->database_name, port->application_name)));Why is it "application" and not "application_name"?
I was trying to be consistent since we don't use "user_name" or
"database_name" as labels even though those are the variable names.
--
Don Seiler
www.seiler.us
