Skip to content

Commit bb1550e

Browse files
s-b-e-n-s-o-ns-b-e-n-s-o-n
committed
🎨 style(triggers): add debug logging to every silent filter in trigger dispatch (#253)
Every early-return in the trigger notification chain now logs the specific reason at debug level, making missed notifications diagnosable without code reading: - Dispatch decision reason (e.g. excluded-from-allow-list) - changed/once/updateAvailable values when shouldHandleSimpleContainerReport rejects - Threshold details (threshold value, updateKind, semverDiff) - Trigger include/exclude context with actual label values Also extracts getMustTriggerDecision() from mustTrigger() to provide structured rejection reasons, and updates default notification templates to include container.notificationWatcherSuffix for multi-host clarity.
1 parent ba6341b commit bb1550e

File tree

3 files changed

+147
-39
lines changed

3 files changed

+147
-39
lines changed

‎app/triggers/providers/Trigger.test.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2094,6 +2094,49 @@ test('handleContainerReport should skip when update-available rule suppresses th
20942094
expect(spy).not.toHaveBeenCalled();
20952095
});
20962096

2097+
test('handleContainerReport should debug log when update-available rule suppresses this trigger', async () => {
2098+
notificationStore.isTriggerEnabledForRule.mockImplementation(
2099+
(ruleId) => ruleId !== 'update-available',
2100+
);
2101+
notificationStore.getTriggerDispatchDecisionForRule.mockReturnValue({
2102+
enabled: false,
2103+
reason: 'excluded-from-allow-list',
2104+
});
2105+
const debugSpy = vi.spyOn(log, 'debug');
2106+
2107+
await trigger.handleContainerReport({
2108+
changed: true,
2109+
container: {
2110+
watcher: 'local',
2111+
name: 'container1',
2112+
updateAvailable: true,
2113+
updateKind: { kind: 'tag', semverDiff: 'major' },
2114+
},
2115+
});
2116+
2117+
expect(debugSpy).toHaveBeenCalledWith(
2118+
'Skipping update-available notification for local_container1 (excluded-from-allow-list)',
2119+
);
2120+
});
2121+
2122+
test('handleContainerReport should debug log when simple mode skips an unchanged update', async () => {
2123+
const debugSpy = vi.spyOn(log, 'debug');
2124+
2125+
await trigger.handleContainerReport({
2126+
changed: false,
2127+
container: {
2128+
watcher: 'local',
2129+
name: 'container1',
2130+
updateAvailable: true,
2131+
updateKind: { kind: 'digest', semverDiff: 'unknown' },
2132+
},
2133+
});
2134+
2135+
expect(debugSpy).toHaveBeenCalledWith(
2136+
'Skipping update-available notification for local_container1 (changed=false, once=true, updateAvailable=true)',
2137+
);
2138+
});
2139+
20972140
test('handleContainerReportDigest should warn once when update-available routing excludes a digest trigger', async () => {
20982141
await trigger.register('trigger', 'smtp', 'gmail', {
20992142
...configurationValid,
@@ -2935,6 +2978,31 @@ test('handleContainerReport should debug log when mustTrigger returns false', as
29352978
expect(spy).not.toHaveBeenCalled();
29362979
});
29372980

2981+
test('handleContainerReport should include trigger filter context when mustTrigger returns false', async () => {
2982+
trigger.configuration = {
2983+
threshold: 'all',
2984+
mode: 'simple',
2985+
};
2986+
trigger.type = 'pushover';
2987+
trigger.name = 'mobile';
2988+
const debugSpy = vi.spyOn(log, 'debug');
2989+
2990+
await trigger.handleContainerReport({
2991+
changed: true,
2992+
container: {
2993+
watcher: 'local',
2994+
name: 'container1',
2995+
updateAvailable: true,
2996+
triggerExclude: 'mobile',
2997+
updateKind: { kind: 'tag', semverDiff: 'major' },
2998+
},
2999+
});
3000+
3001+
expect(debugSpy).toHaveBeenCalledWith(
3002+
'Trigger conditions not met => ignore (triggerInclude=<none>, triggerExclude=mobile, included=true, excluded=true)',
3003+
);
3004+
});
3005+
29383006
test('handleContainerReport should fallback to parent logger when child logger is unavailable', async () => {
29393007
trigger.configuration = {
29403008
threshold: undefined,

‎app/triggers/providers/Trigger.ts‎

Lines changed: 75 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,21 @@ class Trigger extends Component {
544544
return notificationStore.isTriggerEnabledForRule(ruleId, this.getId(), options);
545545
}
546546

547+
private getUpdateAvailableAutoTriggerDispatchDecision() {
548+
const dispatchDecision = notificationStore.getTriggerDispatchDecisionForRule(
549+
'update-available',
550+
this.getId(),
551+
{
552+
// Keep backward compatibility: if update-available has no explicit trigger
553+
// allow-list yet, legacy auto trigger behavior remains enabled.
554+
allowAllWhenNoTriggers: true,
555+
defaultWhenRuleMissing: true,
556+
},
557+
);
558+
this.warnIfDigestRoutingIsSuppressed(dispatchDecision);
559+
return dispatchDecision;
560+
}
561+
547562
private findContainerByBusinessId(containerName: string): Container | undefined {
548563
return storeContainer
549564
.getContainersRaw()
@@ -676,8 +691,11 @@ class Trigger extends Component {
676691
return;
677692
}
678693

679-
if (!this.mustTrigger(container)) {
680-
this.log.debug(`Trigger conditions not met for ${ruleId} event => ignore`);
694+
const mustTriggerDecision = this.getMustTriggerDecision(container);
695+
if (!mustTriggerDecision.allowed) {
696+
this.log.debug(
697+
`Trigger conditions not met for ${ruleId} event => ignore (${mustTriggerDecision.reason})`,
698+
);
681699
return;
682700
}
683701

@@ -786,18 +804,7 @@ class Trigger extends Component {
786804
}
787805

788806
private isUpdateAvailableAutoTriggerEnabled() {
789-
const dispatchDecision = notificationStore.getTriggerDispatchDecisionForRule(
790-
'update-available',
791-
this.getId(),
792-
{
793-
// Keep backward compatibility: if update-available has no explicit trigger
794-
// allow-list yet, legacy auto trigger behavior remains enabled.
795-
allowAllWhenNoTriggers: true,
796-
defaultWhenRuleMissing: true,
797-
},
798-
);
799-
this.warnIfDigestRoutingIsSuppressed(dispatchDecision);
800-
return dispatchDecision.enabled;
807+
return this.getUpdateAvailableAutoTriggerDispatchDecision().enabled;
801808
}
802809

803810
private warnIfDigestRoutingIsSuppressed(
@@ -849,6 +856,42 @@ class Trigger extends Component {
849856
return (this.configuration.threshold ?? 'all').toLowerCase();
850857
}
851858

859+
private getMustTriggerDecision(containerResult: Container) {
860+
if (Trigger.isRollbackContainer(containerResult)) {
861+
return {
862+
allowed: false,
863+
reason: 'rollback-container',
864+
};
865+
}
866+
if (this.agent && this.agent !== containerResult.agent) {
867+
return {
868+
allowed: false,
869+
reason: `agent mismatch expected=${this.agent} actual=${containerResult.agent ?? '<none>'}`,
870+
};
871+
}
872+
if (this.strictAgentMatch && this.agent !== containerResult.agent) {
873+
return {
874+
allowed: false,
875+
reason: `strict agent mismatch expected=${this.agent ?? '<none>'} actual=${containerResult.agent ?? '<none>'}`,
876+
};
877+
}
878+
879+
const { triggerInclude, triggerExclude } = containerResult;
880+
const included = this.isTriggerIncluded(containerResult, triggerInclude);
881+
const excluded = this.isTriggerExcluded(containerResult, triggerExclude);
882+
883+
if (!included || excluded) {
884+
return {
885+
allowed: false,
886+
reason: `triggerInclude=${triggerInclude ?? '<none>'}, triggerExclude=${triggerExclude ?? '<none>'}, included=${included}, excluded=${excluded}`,
887+
};
888+
}
889+
890+
return {
891+
allowed: true,
892+
};
893+
}
894+
852895
private isPureBatchMode() {
853896
return Trigger.normalizeMode(this.configuration.mode) === 'batch';
854897
}
@@ -926,12 +969,15 @@ class Trigger extends Component {
926969
logContainer: Component['log'],
927970
) {
928971
if (!Trigger.isThresholdReached(container, this.getSimpleModeThreshold())) {
929-
logContainer.debug('Threshold not reached => ignore');
972+
logContainer.debug(
973+
`Threshold not reached => ignore (threshold=${this.getSimpleModeThreshold()}, updateKind=${container.updateKind?.kind ?? 'unknown'}, semverDiff=${container.updateKind?.semverDiff ?? 'unknown'})`,
974+
);
930975
return;
931976
}
932977

933-
if (!this.mustTrigger(container)) {
934-
logContainer.debug('Trigger conditions not met => ignore');
978+
const mustTriggerDecision = this.getMustTriggerDecision(container);
979+
if (!mustTriggerDecision.allowed) {
980+
logContainer.debug(`Trigger conditions not met => ignore (${mustTriggerDecision.reason})`);
935981
return;
936982
}
937983

@@ -970,15 +1016,22 @@ class Trigger extends Component {
9701016
* @returns {Promise<void>}
9711017
*/
9721018
async handleContainerReport(containerReport: ContainerReport) {
973-
if (!this.isUpdateAvailableAutoTriggerEnabled()) {
974-
return;
975-
}
976-
9771019
// Strip Docker recreate alias prefixes before any trigger processing
9781020
Trigger.canonicalizeReportName(containerReport);
9791021

1022+
const dispatchDecision = this.getUpdateAvailableAutoTriggerDispatchDecision();
1023+
if (!dispatchDecision.enabled) {
1024+
this.log.debug(
1025+
`Skipping update-available notification for ${fullName(containerReport.container)} (${dispatchDecision.reason})`,
1026+
);
1027+
return;
1028+
}
1029+
9801030
// Filter on changed containers with update available and passing trigger threshold
9811031
if (!this.shouldHandleSimpleContainerReport(containerReport)) {
1032+
this.log.debug(
1033+
`Skipping update-available notification for ${fullName(containerReport.container)} (changed=${containerReport.changed}, once=${this.configuration.once ?? false}, updateAvailable=${containerReport.container.updateAvailable})`,
1034+
);
9821035
return;
9831036
}
9841037

@@ -1235,20 +1288,7 @@ class Trigger extends Component {
12351288
}
12361289

12371290
mustTrigger(containerResult: Container) {
1238-
if (Trigger.isRollbackContainer(containerResult)) {
1239-
return false;
1240-
}
1241-
if (this.agent && this.agent !== containerResult.agent) {
1242-
return false;
1243-
}
1244-
if (this.strictAgentMatch && this.agent !== containerResult.agent) {
1245-
return false;
1246-
}
1247-
const { triggerInclude, triggerExclude } = containerResult;
1248-
return (
1249-
this.isTriggerIncluded(containerResult, triggerInclude) &&
1250-
!this.isTriggerExcluded(containerResult, triggerExclude)
1251-
);
1291+
return this.getMustTriggerDecision(containerResult).allowed;
12521292
}
12531293

12541294
/**

‎app/triggers/providers/pushover/Pushover.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ const configurationValid = {
2424
auto: 'all',
2525
order: 100,
2626
simpletitle:
27-
'${isDigestUpdate ? "New image available for container " + container.name + " (tag " + currentTag + ")" : "New " + container.updateKind.kind + " found for container " + container.name}',
27+
'${isDigestUpdate ? "New image available for container " + container.name + container.notificationWatcherSuffix + " (tag " + currentTag + ")" : "New " + container.updateKind.kind + " found for container " + container.name + container.notificationWatcherSuffix}',
2828

2929
simplebody:
30-
'${isDigestUpdate ? "Container " + container.name + " running tag " + currentTag + " has a newer image available" : "Container " + container.name + " running with " + container.updateKind.kind + " " + container.updateKind.localValue + " can be updated to " + container.updateKind.kind + " " + container.updateKind.remoteValue}${container.result && container.result.link ? "\\n" + container.result.link : ""}',
30+
'${isDigestUpdate ? "Container " + container.name + container.notificationWatcherSuffix + " running tag " + currentTag + " has a newer image available" : "Container " + container.name + container.notificationWatcherSuffix + " running with " + container.updateKind.kind + " " + container.updateKind.localValue + " can be updated to " + container.updateKind.kind + " " + container.updateKind.remoteValue}${container.result && container.result.link ? "\\n" + container.result.link : ""}',
3131

3232
batchtitle: '${containers.length} updates available',
3333
resolvenotifications: false,
@@ -107,10 +107,10 @@ test('maskConfiguration should mask sensitive data', async () => {
107107
auto: 'all',
108108
order: 100,
109109
simplebody:
110-
'${isDigestUpdate ? "Container " + container.name + " running tag " + currentTag + " has a newer image available" : "Container " + container.name + " running with " + container.updateKind.kind + " " + container.updateKind.localValue + " can be updated to " + container.updateKind.kind + " " + container.updateKind.remoteValue}${container.result && container.result.link ? "\\n" + container.result.link : ""}',
110+
'${isDigestUpdate ? "Container " + container.name + container.notificationWatcherSuffix + " running tag " + currentTag + " has a newer image available" : "Container " + container.name + container.notificationWatcherSuffix + " running with " + container.updateKind.kind + " " + container.updateKind.localValue + " can be updated to " + container.updateKind.kind + " " + container.updateKind.remoteValue}${container.result && container.result.link ? "\\n" + container.result.link : ""}',
111111

112112
simpletitle:
113-
'${isDigestUpdate ? "New image available for container " + container.name + " (tag " + currentTag + ")" : "New " + container.updateKind.kind + " found for container " + container.name}',
113+
'${isDigestUpdate ? "New image available for container " + container.name + container.notificationWatcherSuffix + " (tag " + currentTag + ")" : "New " + container.updateKind.kind + " found for container " + container.name + container.notificationWatcherSuffix}',
114114

115115
batchtitle: '${containers.length} updates available',
116116
resolvenotifications: false,

0 commit comments

Comments
 (0)