-
Notifications
You must be signed in to change notification settings - Fork 59
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
Build more regular expressions at boot time #604
Conversation
4045b50
to
637d623
Compare
637d623
to
cf94c4a
Compare
TIL: despite Go being a compiled language, these variables are actually defined when the program boots, not when it's compiled https://stackoverflow.com/a/72185182 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for the cleanup.
// We ignore failures here since we want the per-backend stitching logic | ||
// that runs later on (and any other parsing errors will just be ignored). | ||
// Note that we need to restore the original trailing newlines since | ||
// AnalyzeStreamInGroups expects them and they are not present in the GCP | ||
// log stream. | ||
logLine, _ := parser.ParseLine(in.Content + "\n") | ||
logLine, _ := server.GetLogParser().ParseLine(in.Content + "\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No big deal, but if we get a nil panic here, it now won't be clear if server is nil or GetLogParser()
returned nil (or in
is nil, but that was the case before).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since in
is coming from a channel and we check ok
, I think it's impossible for in
to be nil. But even if it was, lines of code above this would have already panicked by accessing in
.
For support ticket 5773, I noticed that some places in the code we compile regular expressions at runtime for regular expressions that don't change. This PR fixes that, which should reduce CPU/memory usage.