Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Reproducer for bug with Custom 3 log line prefix when app contains brackets #449

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lfittl
Copy link
Member

@lfittl lfittl commented Aug 15, 2023

This needs more investigation, but parking this here to document the issue.

@lfittl lfittl changed the title Reproducer for bug with Custom 3 log line prefix when app contains brackets [WIP] Reproducer for bug with Custom 3 log line prefix when app contains brackets Aug 15, 2023
@lfittl lfittl changed the title [WIP] Reproducer for bug with Custom 3 log line prefix when app contains brackets WIP: Reproducer for bug with Custom 3 log line prefix when app contains brackets Aug 15, 2023
@msakrejda
Copy link
Contributor

msakrejda commented Aug 17, 2023

This change (allowing the app name regex to match brackets but making it non-greedy) fixes the problem and does not break the tests in the existing suite, but it needs more validation:

diff --git a/logs/parse.go b/logs/parse.go
index 15328926..28075b0c 100644
--- a/logs/parse.go
+++ b/logs/parse.go
@@ -60,7 +60,7 @@ var DbRegexp = `(\S*)`                                                       //
 var AppBeforeSpaceRegexp = `(\S*)`                                           // %a
 var AppBeforeCommaRegexp = `([^,]*)`                                         // %a
 var AppBeforeQuoteRegexp = `([^"]*)`                                         // %a
-var AppInsideBracketsRegexp = `(\[unknown\]|[^,\]]*)`                        // %a
+var AppInsideBracketsRegexp = `(\[unknown\]|.+?)`                            // %a
 var HostRegexp = `(\S*)`                                                     // %h
 var VirtualTxRegexp = `(\d+/\d+)?`                                           // %v
 var LogLineCounterRegexp = `(\d+)` 

@msakrejda
Copy link
Contributor

Also, it looks like we're currently matching empty app names. I don't think that can actually happen: if you set your application_name to an empty string, the prefix lists '[unknown'] (which we're already handling). We're also matching empty database names, host names, and user names (which I'm guessing also can't happen). I don't think that's causing problems here, but it may lead to other parsing issues. We should maybe change those *s to +?s...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants