improve: logging for resource filter cache #3167
+212
−135
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,19 +83,13 @@ PostExecutionControl<P> handleDispatch(ExecutionScope<P> executionScope, Context | |
| throws Exception { | ||
| P originalResource = executionScope.getResource(); | ||
| var resourceForExecution = cloneResource(originalResource); | ||
| log.debug("Handling dispatch"); | ||
|
|
||
| final var markedForDeletion = originalResource.isMarkedForDeletion(); | ||
| if (!triggerOnAllEvents() | ||
| && markedForDeletion | ||
| && shouldNotDispatchToCleanupWhenMarkedForDeletion(originalResource)) { | ||
| log.debug("Skipping cleanup because finalizer(s) don't allow processing yet"); | ||
| return PostExecutionControl.defaultDispatch(); | ||
| } | ||
| // context can be provided only for testing purposes | ||
|
|
@@ -165,11 +159,7 @@ private PostExecutionControl<P> reconcileExecution( | |
| P originalResource, | ||
| Context<P> context) | ||
| throws Exception { | ||
| log.debug("Reconciling resource execution scope: {}", executionScope); | ||
|
|
||
| UpdateControl<P> updateControl = controller.reconcile(resourceForExecution, context); | ||
|
|
||
|
|
@@ -246,9 +236,8 @@ public boolean isLastAttempt() { | |
| exceptionLevel = Level.DEBUG; | ||
| failedMessage = " due to conflict"; | ||
| log.info( | ||
| "ErrorStatusUpdateControl.patchStatus failed due to a conflict, but the next" | ||
| + " reconciliation is imminent"); | ||
| } else { | ||
| exceptionLevel = Level.WARN; | ||
| failedMessage = ", but will be retried soon,"; | ||
|
|
@@ -312,10 +301,7 @@ private void updatePostExecutionControlWithReschedule( | |
| private PostExecutionControl<P> handleCleanup( | ||
| P resourceForExecution, Context<P> context, ExecutionScope<P> executionScope) { | ||
| if (log.isDebugEnabled()) { | ||
| log.debug("Executing delete for resource"); | ||
| } | ||
| DeleteControl deleteControl = controller.cleanup(resourceForExecution, context); | ||
| final var useFinalizer = controller.useFinalizer(); | ||
|
|
@@ -329,10 +315,7 @@ private PostExecutionControl<P> handleCleanup( | |
| } | ||
| } | ||
| log.debug( | ||
| "Skipping finalizer remove for resource. Delete control: {}, uses finalizer: {}", | ||
| deleteControl, | ||
| useFinalizer); | ||
|
Comment on lines
317
to
320
There was a problem hiding this comment. There is a double space before "finalizer" in the log message. It should be "Delete control: {}, uses finalizer: {}" with a single space. |
||
| PostExecutionControl<P> postExecutionControl = PostExecutionControl.defaultDispatch(); | ||
|
|
@@ -342,11 +325,7 @@ private PostExecutionControl<P> handleCleanup( | |
|
|
||
| private P patchResource(Context<P> context, P resource, P originalResource) { | ||
| if (log.isDebugEnabled()) { | ||
| log.debug("Updating resource; SSA: {}", useSSA); | ||
| } | ||
| log.trace("Resource before update: {}", resource); | ||
|
|
||
|
|
@@ -384,10 +363,7 @@ public CustomResourceFacade(ControllerConfiguration<R> configuration, Cloner clo | |
|
|
||
| public R patchResource(Context<R> context, R resource, R originalResource) { | ||
| if (log.isDebugEnabled()) { | ||
| log.debug("Trying to replace resource"); | ||
| } | ||
| if (useSSA) { | ||
| return context.resourceOperations().serverSideApplyPrimary(resource); | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The MDC key names defined here do not match the keys used in the log4j2.xml pattern layouts. The log4j2.xml files reference keys like
informer.name,informer.event.action,informer.event.resource.name, etc., but the code defineseventsource.name,eventsource.event.action,eventsource.event.resource.name, etc. This mismatch will cause the MDC values not to appear in the logs as expected.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.
This looks like an hallucination but I'm not super familiar with how MDC is used so 🤷
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.
Actually, these should be documented in the appropriate doc section, along with the way to enable / disable it (which I don't think is documented).
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.
Yes, will add this to the docs, although this is not that interesting for the user, since this happens outside the reconciliation, but should be in the docs at least mentioned.