chore: update the webhook log logic #36
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates webhook logging logic to reduce noise and makes changes to the reconciliation flow, including event recording order and resource application logic.
- Reduced logging verbosity by using
V(1)log levels for routine operations like successful webhook sends and conflict retries - Changed resource application strategy to use
CreateIfNotExistbeforeReconcileResourceto avoid conflicts - Moved webhook event recording to occur before LastChangeID updates to ensure events are sent before changes are marked as processed
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/webrecorder/recorder.go | Changed successful webhook event logs to use V(1) verbosity level to reduce noise |
| internal/controller/common/kubeutil/apply.go | Refactored ApplyObjectV2 to call CreateIfNotExist first, then ReconcileResource for existing resources, with improved operation result tracking |
| internal/controller/app/applicationdefinition_controller.go | Added fast-path optimization for stable Running apps, moved event recording before LastChangeID updates, added resource-level webhook events, reduced log verbosity for routine operations, and updated health check logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| switch applyResult.Operation { | ||
| case "created": | ||
| eventMessage = fmt.Sprintf("Created %s: %s", gvk.Kind, objKey.Name) | ||
| eventReason = "ResourceCreated" | ||
| stepName = fmt.Sprintf("Sync%s", gvk.Kind) // e.g., SyncConfigMap, SyncService | ||
| case "configured", "updated": | ||
| eventMessage = fmt.Sprintf("Updated %s: %s", gvk.Kind, objKey.Name) | ||
| eventReason = "ResourceUpdated" | ||
| stepName = fmt.Sprintf("Sync%s", gvk.Kind) // e.g., SyncConfigMap, SyncService | ||
| default: |
There was a problem hiding this comment.
The switch statement is comparing applyResult.Operation (which is of type controllerutil.OperationResult) against string literals "created", "configured", and "updated". This will never match because you're comparing a typed constant against strings.
You should compare against the actual constants like controllerutil.OperationResultCreated and controllerutil.OperationResultUpdated instead of string literals.
| // --- 2. Check Application-Level Health (if K8s resource is healthy) --- | ||
| compLogger.V(1).Info("K8s resource is healthy, proceeding to application-level health check") | ||
|
|
||
| return true, false, firstCheckErr | ||
| // Mark this component as healthy | ||
| compStatus.Health = true | ||
| compStatus.Message = "Component is ready and healthy" | ||
| compLogger.V(1).Info("Component health check passed") |
There was a problem hiding this comment.
The comment on line 826 states "Check Application-Level Health (if K8s resource is healthy)", but the code that follows (lines 829-832) doesn't actually perform any application-level health check. It simply marks the component as healthy after the K8s resource health check passes.
Either implement the application-level health check as the comment suggests, or update the comment to accurately reflect that only K8s resource health is being checked.
3 participants
No description provided.