Include log text in snapshots #597
Conversation
seanlinsley
force-pushed
the
logs-include-text
branch
2 times, most recently
from
0a0b150 to
c5de10b
Compare
seanlinsley
force-pushed
the
logs-include-text
branch
from
c5de10b to
c39d6b4
Compare
| @@ -95,4 +95,4 @@ require ( | |||
| google.golang.org/grpc v1.58.3 // indirect | |||
| ) | |||
|
|
|||
| go 1.20 | |||
| go 1.21 | |||
There was a problem hiding this comment.
We upgraded to Go 1.21 in #447 but this file wasn't updated because Heroku didn't yet support it, but that's since been fixed: heroku/heroku-buildpack-go#522
Go 1.21 is needed for the slices.Replace usage in this PR.
| }, | ||
| { | ||
| filterLogSecret: "statement_text, statement_parameter, unidentified", | ||
| input: "2018-03-11 20:00:02 UTC:1.1.1.1(2):a@b:[3]:LOG: duration: 4079.697 ms execute <unnamed>: \nSELECT * FROM x WHERE y = $1 LIMIT $2\n2018-03-11 20:00:02 UTC:1.1.1.1(2):a@b:[3]:DETAIL: parameters: $1 = 'long string', $2 = '1'\n", | ||
| output: "2018-03-11 20:00:02 UTC:1.1.1.1(2):a@b:[3]:LOG: duration: 4079.697 ms execute <unnamed>: \nXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX\n2018-03-11 20:00:02 UTC:1.1.1.1(2):a@b:[3]:DETAIL: parameters: $1 = 'XXXXXXXXXXX', $2 = 'X'\n", | ||
| output: "duration: 4079.697 ms execute <unnamed>: \n[redacted]\n[redacted]\n", |
There was a problem hiding this comment.
This is a behavior change in logs/replace.go, where unidentified log lines are entirely redacted. The paramaters log line happens to have an unknown classification in this test.
| @@ -22,42 +22,32 @@ var replaceTests = []replaceTestpair{ | |||
| { | |||
| filterLogSecret: "all", | |||
| input: "2018-03-11 20:00:02 UTC:1.1.1.1(2):a@b:[3]:LOG: duration: 1242.570 ms statement: SELECT 1\n", | |||
| output: "2018-03-11 20:00:02 UTC:1.1.1.1(2):a@b:[3]:LOG: duration: 1242.570 ms statement: XXXXXXXX\n", | |||
| output: "duration: 1242.570 ms statement: [redacted]\n", | |||
There was a problem hiding this comment.
Since the output is generated from the fully processed log lines, the prefixes have been removed.
There's only one grant type now, so there's no need for a "default" grant
seanlinsley
force-pushed
the
logs-include-text
branch
from
5deb3e5 to
b86b20e
Compare
| @@ -2266,8 +2266,7 @@ func AnalyzeBackendLogLines(logLines []state.LogLine) (logLinesOut []state.LogLi | |||
| logLinesOut = append(logLinesOut, logLine) | |||
| } | |||
|
|
|||
| // Ensure no other part of the system accidentally sends log line contents, as | |||
| // they should be considered opaque from here on | |||
| // Remove log line content. Note that ReplaceSecrets adds it back after secrets have been removed. | |||
| for idx := range logLinesOut { | |||
| logLinesOut[idx].Content = "" | |||
There was a problem hiding this comment.
A future PR might remove the tempfile being written since I think it's not needed now. At that point we wouldn't need to zero-out the log text here. Though that would also require refactoring the log filter code so the byte offsets are relative to that string, instead of being relative to the entire file.
Log text is now included directly in snapshots instead of uploading the logs separately to S3.
In passing, filtered logs are now replaced with
[redacted]. Previously an exact number ofXcharacters would be used to keep the length of the log text from changing. This resulted in hard-to-read logs when the filtered content was very long.Possible followup PRs:
--dry-run-logs