Skip to content

Add log lines from (devel) Postgres make check output - #842

Open
msakrejda wants to merge 16 commits into
mainfrom
add-log-line-classifications-from-make-check-log-output
Open

Add log lines from (devel) Postgres make check output#842
msakrejda wants to merge 16 commits into
mainfrom
add-log-line-classifications-from-make-check-log-output

Conversation

@msakrejda

@msakrejda msakrejda commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

See individual commits:

  • Improve log line matching for existing categories
  • Add SKIPPING_MAINTENANCE_PERMISSION_DENIED
  • Add VALUE_OUT_OF_RANGE
  • Add OBJECT_DOES_NOT_EXIST
  • Expand CHECK_CONSTRAINT_VIOLATION to catch some partition errors
  • Add ROW_LEVEL_SECURITY_VIOLATION
  • Add CANNOT_MODIFY_VIEW and WITH_CHECK_OPTION_VIOLATION
  • Add SQL_JSON_ERROR
  • Add PARTITION_ERROR
  • Add OBJECT_ALREADY_EXISTS and WRONG_OBJECT_TYPE
  • Recategorize a bit: prefer specific categories

Note that SQL_JSON_ERROR and PARTITION_ERROR are coarser-grained than a lot of the existing event types, but I think this is preferable to both keeping these as UNKNOWN and a proliferation of event types.

@msakrejda
msakrejda requested a review from a team July 23, 2026 22:13

@keiko713 keiko713 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, it was easy to follow per commit. Just to double check - we'll need to create a follow up PR in docs to explain about new/updated categories?

@msakrejda

Copy link
Copy Markdown
Contributor Author

@keiko713 yeah, and in the app to actually process them. Thanks for the review!

Comment thread protobuf/compact_log_snapshot.proto
Comment thread protobuf/compact_log_snapshot.proto
Comment thread logs/analyze.go Outdated
Comment thread logs/analyze.go Outdated
Comment thread logs/analyze.go Outdated
Comment thread logs/analyze.go Outdated
Comment thread logs/analyze.go
"ROW triggers with transition tables", "TO must specify", "FROM must specify",
},
regexp: regexp.MustCompile(`^(?:` +
`partitions?\b.+` +

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also have a bunch of trailing .* that we can maybe avoid?

Comment thread logs/analyze.go
`|could not find jsonpath variable "[^"]*"` +
`|syntax error at end of jsonpath input(?: at character \d+)?` +
`|no SQL/JSON item found for specified path(?: of column "[^"]*")?` +
`|JSON path expression .+` +

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same in this set of expressions

@msakrejda
msakrejda marked this pull request as draft August 6, 2026 16:39

@msakrejda msakrejda left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated most of the introduced expressions to avoid a trailing .* or .+, and rewrote the match struct comment to note our philosophy about this (another set of eyes would be welcome there to make sure I captured that correctly).

Three log event types introduced in this PR are hard to tighten: permissionDenied, partitionError, and sqlJsonError. They cover a lot of ground. We could drop them or let them slide for now. These are only an issue if an unrelated log line containing PII is somehow stitched to the end of the line being captured without a joining newline, but that's not impossible if we have a bad enough stitching issue.

In addition, cannotDrop and connectionAuthorized are existing cases of the same over-broad capture. I can follow up on them separately.

@msakrejda
msakrejda marked this pull request as ready for review August 13, 2026 04:28
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.

3 participants