Skip to content

fix panic error on containerd id finder task#355

Merged
kyasbal merged 2 commits into
GoogleCloudPlatform:mainfrom
kyasbal:fix/containerid-finder-crash
Nov 11, 2025
Merged

fix panic error on containerd id finder task#355
kyasbal merged 2 commits into
GoogleCloudPlatform:mainfrom
kyasbal:fix/containerid-finder-crash

Conversation

@kyasbal

@kyasbal kyasbal commented Nov 11, 2025

Copy link
Copy Markdown
Member

The logutil.ExtractKLogField can return an empty string without error. This can cause panic on the ID finder.

@kyasbal kyasbal added bug Something isn't working backend:parser labels Nov 11, 2025
@kyasbal

kyasbal commented Nov 11, 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 correctly addresses a panic caused by an empty string return from logutil.ExtractKLogField. The fix prevents the panic by adding a check and returning an error. The inclusion of new test cases to cover this scenario is a great addition. I have a couple of suggestions to improve the error handling to make it more robust by preserving the original error information, and a minor suggestion to improve the clarity of a test case description.

Comment thread pkg/task/inspection/googlecloudlogk8snode/impl/containerd_task.go Outdated
Comment thread pkg/task/inspection/googlecloudlogk8snode/impl/containerd_task.go Outdated
Comment thread pkg/task/inspection/googlecloudlogk8snode/impl/containerd_task_test.go Outdated
@kyasbal

kyasbal commented Nov 11, 2025

Copy link
Copy Markdown
Member Author

/gemini summary

@kyasbal kyasbal marked this pull request as ready for review November 11, 2025 08:18
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

This pull request addresses a critical panic error within the containerd ID finder task by refining the error handling logic when extracting log fields. The changes ensure that both extraction errors and empty message scenarios are explicitly caught and reported with specific error messages, preventing unexpected crashes. The update also includes new test coverage to validate the improved robustness of these functions.

Highlights

  • Panic Error Fix: Resolved a panic error in the containerd ID finder task that occurred when logutil.ExtractKLogField returned an empty string without an error.
  • Improved Error Handling: Refactored error handling in findPodSandboxIDInfo and findContainerIDInfo to explicitly differentiate between extraction errors and cases where the main message is empty, returning more descriptive errors.
  • New Test Cases: Added new test cases for processPodSandboxIDDiscoveryForLog and processContainerIDDiscoveryForLog to cover scenarios with empty log messages, enhancing test coverage for robustness.
Changelog
  • pkg/task/inspection/googlecloudlogk8snode/impl/containerd_task.go
    • Modified findPodSandboxIDInfo to separate error checking for logutil.ExtractKLogField results, handling err != nil and msg == "" distinctly with specific fmt.Errorf messages.
    • Modified findContainerIDInfo with similar separated error handling logic for logutil.ExtractKLogField results.
  • pkg/task/inspection/googlecloudlogk8snode/impl/containerd_task_test.go
    • Added TestProcessPodSandboxIDDiscoveryForLog to test the processPodSandboxIDDiscoveryForLog function, including a test case for an empty message.
    • Added TestProcessContainerIDDiscoveryForLog to test the processContainerIDDiscoveryForLog function, including a test case for an empty message.
Activity
  • kyasbal requested a Gemini review.
  • gemini-code-assist[bot] provided review comments suggesting to separate error checks for err != nil and msg == "" in findPodSandboxIDInfo and findContainerIDInfo for clarity and robustness.
  • gemini-code-assist[bot] suggested renaming a test case description from "log message without RunPodSandbox prefix" to "empty log message" for accuracy.
  • kyasbal requested a Gemini summary.

@kyasbal kyasbal merged commit f3d881d into GoogleCloudPlatform:main Nov 11, 2025
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:parser bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants