-
Notifications
You must be signed in to change notification settings - Fork 71
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
Support health monitoring mode for NPD #479
Conversation
/gcbrun |
4 similar comments
/gcbrun |
/gcbrun |
/gcbrun |
/gcbrun |
/gcbrun |
1 similar comment
/gcbrun |
/gcbrun |
/gcbrun |
/gcbrun |
/gcbrun |
/gcbrun |
/gcbrun |
1 similar comment
/gcbrun |
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.
Great work! Mostly minor comments except about the struct to json encoding.
launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config.go
Outdated
Show resolved
Hide resolved
launcher/internal/healthmonitoring/nodeproblemdetector/systemstats_config.go
Show resolved
Hide resolved
launcher/launcher/main.go
Outdated
if launchSpec.MonitoringEnabled == spec.All { | ||
logger.Printf("All health monitoring metrics enabled") | ||
if err := nodeproblemdetector.EnableAllConfig(); err != nil { | ||
logger.Printf("failed to enable full monitoring config: %v", err) |
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 it's logged, this should be capitalized "Failed to ...
/gcbrun |
/gcbrun |
Supports the full list of system stat metrics that can be collected by NPD for health monitoring. The full configuration will be enabled if allowed by the policy and spec.
Breaking Changes
spec.GetLaunchPolicy(imageLabels map[string]string)
tospec.GetLaunchPolicy(imageLabels map[string]string, logger *log.Logger)