Skip to content

Implement the new klog and logfmt structured log parser#359

Merged
kyasbal merged 4 commits into
GoogleCloudPlatform:mainfrom
kyasbal:feat/improve-klog-parser
Nov 13, 2025
Merged

Implement the new klog and logfmt structured log parser#359
kyasbal merged 4 commits into
GoogleCloudPlatform:mainfrom
kyasbal:feat/improve-klog-parser

Conversation

@kyasbal

@kyasbal kyasbal commented Nov 13, 2025

Copy link
Copy Markdown
Member

This is a change part of #357.
This PR introduces new string formatted structured logging parsers.

The previous implementations were renamed to legacyklog and it had problems:

  • The implementation didn't return any error when it fails to parse field thus it couldn't identify if that was error of parsing or just receiving empty string
  • The implementation used string and + operator to concatenate them. This is inefficient.
  • The implementation was named like KLogParser but it actually pases KLog format or Logfmt format.

The new implementation introduced in this PR uses strings.Builder to parse and uses sync.Pool to use them efficiently.

@kyasbal kyasbal marked this pull request as ready for review November 13, 2025 02:33
@kyasbal

kyasbal commented Nov 13, 2025

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces new, more performant parsers for klog and logfmt structured logs, moving the old implementation to a legacyklog file. The new parsers leverage sync.Pool and strings.Builder for efficiency. The changes are well-structured, with common types extracted into a new type.go file and comprehensive tests including benchmarks.

My review includes a few suggestions to improve maintainability and robustness:

  • Decoupling the legacyklog parser from the logfmt parser.
  • Ensuring consistent and safe usage of sync.Pool.
  • Improving test file organization.

Comment thread pkg/core/inspection/logutil/klog.go Outdated
Comment thread pkg/core/inspection/logutil/klog_test.go Outdated
Comment thread pkg/core/inspection/logutil/legacyklog.go
@kyasbal kyasbal changed the base branch from epic/issue-357 to main November 13, 2025 05:07
@kyasbal kyasbal merged commit f0da075 into GoogleCloudPlatform:main Nov 13, 2025
7 of 8 checks passed
@kyasbal kyasbal deleted the feat/improve-klog-parser branch November 13, 2025 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants