Skip to content
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

feat: add optional logging for expander stages #2162

Conversation

xiaoweim
Copy link
Collaborator

@xiaoweim xiaoweim commented Jun 26, 2024

This PR adds the option to enable logging for each expander stage for better debug ability.

Log message start with expanderDebugLog---${Kind}/${Namespace}/${Name}---version: ${Version}

Example log:

2024-06-29T00:35:34Z	DEBUG	events	expanderDebugLog---PConfig/team-a/team-a-config---version: 1	{"type": "Normal", "object": {"kind":"PConfig","namespace":"team-a","name":"team-a-config","uid":"9d674656-1db3-4d54-b5eb-f51bf90d8732","apiVersion":"facade.foocorp.com/v1alpha1","resourceVersion":"679"}, "reason": "Running expander stage 0: common"}
2024-06-29T00:35:34Z	DEBUG	events	expanderDebugLog---PConfig/team-a/team-a-config---version: 1---request: config:"apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: common-config\n  namespace: {{ pconfigs.metadata.namespace }}\n  labels:\n    createdby: \"composition-namespaceconfigmap\"\ndata:\n  key1: value1" context:"{\"apiVersion\":\"composition.google.com/v1alpha1\",\"kind\":\"Context\",\"metadata\":{\"creationTimestamp\":\"2024-06-29T00:35:33Z\",\"generation\":1,\"managedFields\":[{\"apiVersion\":\"composition.google.com/v1alpha1\",\"fieldsType\":\"FieldsV1\",\"fieldsV1\":{\"f:spec\":{\".\":{},\"f:project\":{}}},\"manager\":\"testcases.test\",\"operation\":\"Update\",\"time\":\"2024-06-29T00:35:33Z\"}],\"name\":\"context\",\"namespace\":\"team-a\",\"resourceVersion\":\"678\",\"uid\":\"aafd1a60-c07c-4d2e-95e7-059be18ef8b9\"},\"spec\":{\"project\":\"proj-a\"}}" facade:"{\"apiVersion\":\"facade.foocorp.com/v1alpha1\",\"kind\":\"PConfig\",\"metadata\":{\"annotations\":{\"composition-expander-debug-logs\":\"true\"},\"creationTimestamp\":\"2024-06-29T00:35:33Z\",\"generation\":1,\"managedFields\":[{\"apiVersion\":\"facade.foocorp.com/v1alpha1\",\"fieldsType\":\"FieldsV1\",\"fieldsV1\":{\"f:metadata\":{\"f:annotations\":{\".\":{},\"f:composition-expander-debug-logs\":{}}},\"f:spec\":{\".\":{},\"f:project\":{}}},\"manager\":\"testcases.test\",\"operation\":\"Update\",\"time\":\"2024-06-29T00:35:33Z\"}],\"name\":\"team-a-config\",\"namespace\":\"team-a\",\"resourceVersion\":\"679\",\"uid\":\"9d674656-1db3-4d54-b5eb-f51bf90d8732\"},\"spec\":{\"project\":\"proj-a\"}}" value:"{}" resource:"pconfigs", result: error:{} type:MANIFESTS manifests:"apiVersion: v1\nkind: ConfigMap\nmetadata:\n  name: common-config\n  namespace: team-a\n  labels:\n    createdby: \"composition-namespaceconfigmap\"\ndata:\n  key1: value1"	{"type": "Normal", "object": {"kind":"PConfig","namespace":"team-a","name":"team-a-config","uid":"9d674656-1db3-4d54-b5eb-f51bf90d8732","apiVersion":"facade.foocorp.com/v1alpha1","resourceVersion":"679"}, "reason": "Expander stage common evaluation completed"}
2024-06-29T01:02:23Z	DEBUG	events	expanderDebugLog---PConfig/team-a/team-a-config---version: 1---resource count: 1	{"type": "Normal", "object": {"kind":"PConfig","namespace":"team-a","name":"team-a-config","uid":"431e53bf-1c9c-4ee2-873d-bdf4028c98db","apiVersion":"facade.foocorp.com/v1alpha1","resourceVersion":"678"}, "reason": "Finished expander stage 0: common"}

@xiaoweim xiaoweim force-pushed the add_expander_stage_logging branch 3 times, most recently from dd67398 to 095ffd2 Compare June 27, 2024 06:41
@xiaoweim xiaoweim changed the title WIP: add optional logging for expander stages feat: add optional logging for expander stages Jun 27, 2024
@xiaoweim
Copy link
Collaborator Author

/assign @barney-s @cheftako

@xiaoweim xiaoweim force-pushed the add_expander_stage_logging branch from 095ffd2 to c1cad1e Compare June 27, 2024 21:18
@xiaoweim xiaoweim force-pushed the add_expander_stage_logging branch 3 times, most recently from 34b77f9 to b64a07a Compare June 28, 2024 00:29
@xiaoweim xiaoweim force-pushed the add_expander_stage_logging branch from b64a07a to baf56d1 Compare June 29, 2024 01:08
@xiaoweim xiaoweim force-pushed the add_expander_stage_logging branch from 4a4e006 to 970d833 Compare June 29, 2024 01:21
Copy link
Collaborator

@barney-s barney-s left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@@ -517,6 +540,10 @@ func (r *ExpanderReconciler) evaluateAndSavePlan(ctx context.Context, logger log
err = fmt.Errorf("Evaluate Failed: %s", result.Error.Message)
return values, updated, "EvaluateStatusFailed", err
}
if expanderDebugLogEnabled {
logger.Info(expanderDebugLog(cr) + fmt.Sprintf("---sent expander request: %v, received results: %v", evaluateRequest, result))
r.Recorder.Event(cr, "Normal", fmt.Sprintf("Expander stage %s evaluation completed", expander.Name), expanderDebugLog(cr)+fmt.Sprintf("---request: %v, result: %v", evaluateRequest, result))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if there is a limit on what can be sent to an event. Nevertheless lets try it.

@google-oss-prow google-oss-prow bot added the lgtm label Jul 2, 2024
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: barney-s

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 8331376 into GoogleCloudPlatform:master Jul 2, 2024
7 checks passed
@yuwenma yuwenma added this to the 1.120 milestone Oct 25, 2024
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.

4 participants