Skip to content

Commit d4bd044

Browse files
Address PR #68 review: make the history feature flag a true on/off switch
MONITOR_HISTORY_ENABLED was read-path only, so "default off" still ran per-check ingestion and the hourly aggregate / daily prune jobs. Plus follow-up correctness and clarity fixes from the code review. Feature flag (blocker): - Add MonitorCheckLogService::logCheckIfEnabled(); all automatic per-check ingestion (uptime hooks, certificate listeners, domain verification) now no-ops when the flag is off. logCheck() stays unconditional for operator paths (backfill, seeder) so history can still be pre-staged. - Gate monitor:aggregate-check-metrics and monitor:prune-check-history behind ->when(config('monitor-history.enabled')) and add withoutOverlapping(). Core uptime/certificate/domain checks remain ungated. Correctness / clarity: - Extract DomainService::resolveExpirationNotifications() and cover the expiry-warning thresholds with tests (locks the whole-day int behaviour). - Reword monitor:backfill-check-history so it no longer implies multi-day history: it writes one current-state snapshot per enabled check type and --days only sizes the aggregation window. Locked with a test. - Heatmap legend now reflects every colour getCellClasses() emits (green/red gradients + yellow), not a single swatch per status. Docs / hygiene: - Document the flag semantics, a revised rollout checklist, and the dedicated monitor_test MySQL database the suite requires. - Untrack the accidentally committed .phpunit.cache artifact and gitignore it. Tests: 48 passed (was 34). Pint clean. Frontend build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 408d4f2 commit d4bd044

16 files changed

Lines changed: 459 additions & 56 deletions

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
.env
88
.env.backup
99
.phpunit.result.cache
10+
/.phpunit.cache
1011
Homestead.json
1112
Homestead.yaml
1213
npm-debug.log

.phpunit.cache/test-results

Lines changed: 0 additions & 1 deletion
This file was deleted.

app/Console/Commands/BackfillMonitorCheckHistory.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ class BackfillMonitorCheckHistory extends Command
1414
{
1515
protected $signature = 'monitor:backfill-check-history
1616
{--monitor-id= : Backfill only one monitor by id}
17-
{--days=30 : Days window used for aggregation after backfill}
17+
{--days=30 : Size of the aggregation window (in days) rolled up after the snapshot; does NOT fabricate per-day history}
1818
{--timezone= : Timezone used for daily aggregation}
1919
{--dry-run : Preview rows without writing}';
2020

21-
protected $description = 'Create synthetic monitor check logs from current monitor state';
21+
protected $description = 'Seed one synthetic check log per enabled check type from current monitor state (approximate, not real history)';
2222

2323
public function handle(
2424
MonitorCheckLogService $logService,

app/Listeners/LogCertificateCheckFailed.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public function handle(CertificateCheckFailed $event): void
1717
return;
1818
}
1919

20-
$this->monitorCheckLogService->logCheck(
20+
$this->monitorCheckLogService->logCheckIfEnabled(
2121
monitor: $monitor,
2222
checkType: MonitorCheckLogService::CHECK_TYPE_CERTIFICATE,
2323
status: MonitorCheckLogService::STATUS_FAILED,

app/Listeners/LogCertificateCheckSucceeded.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public function handle(CertificateCheckSucceeded $event): void
1717
return;
1818
}
1919

20-
$this->monitorCheckLogService->logCheck(
20+
$this->monitorCheckLogService->logCheckIfEnabled(
2121
monitor: $monitor,
2222
checkType: MonitorCheckLogService::CHECK_TYPE_CERTIFICATE,
2323
status: MonitorCheckLogService::STATUS_SUCCESS,

app/Models/Monitor.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function uptimeRequestSucceeded(ResponseInterface $response): void
4545
? $updatedMonitor->uptime_check_failure_reason
4646
: null;
4747

48-
app(MonitorCheckLogService::class)->logCheck(
48+
app(MonitorCheckLogService::class)->logCheckIfEnabled(
4949
monitor: $updatedMonitor,
5050
checkType: MonitorCheckLogService::CHECK_TYPE_UPTIME,
5151
status: $status,
@@ -68,7 +68,7 @@ public function uptimeRequestFailed(string $reason): void
6868
return;
6969
}
7070

71-
app(MonitorCheckLogService::class)->logCheck(
71+
app(MonitorCheckLogService::class)->logCheckIfEnabled(
7272
monitor: $updatedMonitor,
7373
checkType: MonitorCheckLogService::CHECK_TYPE_UPTIME,
7474
status: MonitorCheckLogService::STATUS_FAILED,

app/Services/DomainService.php

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function verifyDomainExpiration(Monitor $monitor): array
5050
$domainInfo = $this->getDomainExpirationDate($monitor->url);
5151

5252
if (empty($domainInfo)) {
53-
app(MonitorCheckLogService::class)->logCheck(
53+
app(MonitorCheckLogService::class)->logCheckIfEnabled(
5454
monitor: $monitor,
5555
checkType: MonitorCheckLogService::CHECK_TYPE_DOMAIN,
5656
status: MonitorCheckLogService::STATUS_FAILED,
@@ -82,7 +82,7 @@ public function verifyDomainExpiration(Monitor $monitor): array
8282
$status = $outcome['status'];
8383
$message = $outcome['message'];
8484

85-
app(MonitorCheckLogService::class)->logCheck(
85+
app(MonitorCheckLogService::class)->logCheckIfEnabled(
8686
monitor: $monitor,
8787
checkType: MonitorCheckLogService::CHECK_TYPE_DOMAIN,
8888
status: $status,
@@ -124,30 +124,42 @@ public function resolveDomainExpirationOutcome(int $daysUntilExpiration): array
124124
return ['status' => $status, 'message' => $message];
125125
}
126126

127-
protected function checkAndNotifyExpiration(Monitor $monitor, int $daysUntilExpiration): array
127+
/**
128+
* Resolve which expiration-warning notifications are due for a whole-day count
129+
* until expiration. Returns at most one notification (the first matching
130+
* threshold). An already-expired domain (negative days) never produces an
131+
* "expires in N days" warning.
132+
*/
133+
public function resolveExpirationNotifications(int $daysUntilExpiration): array
128134
{
129-
$expirationDate = $monitor->domain_expires_at;
130-
131-
if (! $expirationDate) {
135+
if ($daysUntilExpiration < 0) {
132136
return [];
133137
}
134138

135-
$domainCheckTimePeriods = config('domain-expiration.domain_check_time_period');
139+
$domainCheckTimePeriods = config('domain-expiration.domain_check_time_period', []);
136140

137-
$notifications = [];
138-
139-
foreach ($domainCheckTimePeriods as $warningType => $details) {
141+
foreach ($domainCheckTimePeriods as $details) {
140142
$daysThreshold = $details['days'];
141143

142-
if ($daysUntilExpiration >= 0 && $daysUntilExpiration === $daysThreshold) {
143-
$notifications[] = [
144+
if ($daysUntilExpiration === $daysThreshold) {
145+
return [[
144146
'days' => $daysThreshold,
145147
'message' => "Domain expires in $daysThreshold ".($daysThreshold === 1 ? 'day' : 'days').'!',
146-
];
147-
break;
148+
]];
148149
}
149150
}
150151

152+
return [];
153+
}
154+
155+
protected function checkAndNotifyExpiration(Monitor $monitor, int $daysUntilExpiration): array
156+
{
157+
if (! $monitor->domain_expires_at) {
158+
return [];
159+
}
160+
161+
$notifications = $this->resolveExpirationNotifications($daysUntilExpiration);
162+
151163
if (empty($notifications)) {
152164
return [];
153165
}

app/Services/MonitorCheckLogService.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,40 @@ class MonitorCheckLogService
2323

2424
public const STATUS_UNKNOWN = 'unknown';
2525

26+
/**
27+
* Log a check only when monitor history is enabled.
28+
*
29+
* This is the entry point for automatic, per-check ingestion (uptime/domain/
30+
* certificate hooks). When the feature flag is off it is a no-op, so a "default
31+
* off" deployment incurs no per-check write load. Operator-driven paths
32+
* (backfill, seeder) call logCheck() directly to bypass the flag.
33+
*/
34+
public function logCheckIfEnabled(
35+
Monitor $monitor,
36+
string $checkType,
37+
string $status,
38+
CarbonInterface|string|null $checkedAt = null,
39+
?string $message = null,
40+
?string $failureReason = null,
41+
?int $responseTimeMs = null,
42+
array $metadata = []
43+
): ?MonitorCheckLog {
44+
if (! config('monitor-history.enabled')) {
45+
return null;
46+
}
47+
48+
return $this->logCheck(
49+
$monitor,
50+
$checkType,
51+
$status,
52+
$checkedAt,
53+
$message,
54+
$failureReason,
55+
$responseTimeMs,
56+
$metadata
57+
);
58+
}
59+
2660
public function logCheck(
2761
Monitor $monitor,
2862
string $checkType,

docs/installation.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,3 +107,32 @@ Before you start following the guidelines, make sure to go through the [prerequi
107107
php artisan serve
108108
```
109109
Note- php artisan serve command will start a development server on your local machine that listens to port 8000 by default. This command will provide a link in the terminal which we can copy and paste it in our browser and see our laraver application.
110+
111+
## Running tests
112+
113+
The test suite runs against MySQL (not SQLite) so `RefreshDatabase` exercises the
114+
same engine as production. It uses a **dedicated** database named `monitor_test`
115+
(configured in `phpunit.xml`) so tests never touch your development data — a guard
116+
test fails fast if the suite is pointed anywhere else.
117+
118+
1. Create the test database once:
119+
120+
```sh
121+
mysql -u root -e "CREATE DATABASE IF NOT EXISTS monitor_test;"
122+
```
123+
124+
(Adjust the user/host to match your `.env`. No data needs to be seeded —
125+
migrations run automatically.)
126+
127+
2. If `php artisan config:cache` was ever run, clear it first, otherwise the cached
128+
`DB_DATABASE` overrides `phpunit.xml` and the suite would run against your dev DB:
129+
130+
```sh
131+
php artisan config:clear
132+
```
133+
134+
3. Run the suite:
135+
136+
```sh
137+
php artisan test
138+
```

docs/monitor-history.md

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,27 @@ This document covers operational workflows for monitor history logging, aggregat
44

55
## Feature Flag
66

7-
Monitor history UI and payload are controlled by:
7+
The entire monitor-history feature is controlled by a single flag (default **off**):
88

99
```bash
1010
MONITOR_HISTORY_ENABLED=true
1111
```
1212

13+
When the flag is **off**, the feature is a true no-op:
14+
15+
- Per-check ingestion (uptime/domain/certificate hooks) writes **no** rows to
16+
`monitor_check_logs`, so there is no added write load.
17+
- The scheduled `monitor:aggregate-check-metrics` and `monitor:prune-check-history`
18+
jobs are skipped entirely (nothing is aggregated or pruned).
19+
- The monitor detail page returns no history payload.
20+
21+
Core uptime/certificate/domain checking and domain-expiry notifications are
22+
**not** affected by this flag — they always run.
23+
24+
Operator commands (`monitor:backfill-check-history`, `monitor:aggregate-check-metrics`,
25+
`monitor:prune-check-history`) can still be run manually regardless of the flag, so
26+
history can be pre-staged before the feature is switched on.
27+
1328
## Timezone
1429

1530
Daily metrics are bucketed by a single server-side timezone, and the monitor
@@ -44,7 +59,8 @@ Useful options:
4459

4560
### Backfill synthetic logs (one-time bootstrap)
4661

47-
Creates synthetic check logs from current monitor state and aggregates days window.
62+
Seeds **one** synthetic check log per enabled check type from each monitor's
63+
*current* state, then aggregates the window into daily metrics.
4864

4965
```bash
5066
php artisan monitor:backfill-check-history --days=30
@@ -59,7 +75,11 @@ Useful options:
5975
Notes:
6076

6177
- Backfill records include `metadata.source=backfill`.
62-
- Backfill is best-effort and should be treated as approximate history.
78+
- This is a **single current-state snapshot per check type**, not a reconstruction
79+
of real per-day history (that data does not exist). It exists only so a monitor
80+
is not completely empty before real checks start accumulating. `--days` sizes the
81+
aggregation window that rolls the snapshot up; it does not fabricate per-day rows.
82+
- Real history begins accumulating from the moment the feature flag is switched on.
6383

6484
### Prune old raw logs
6585

@@ -76,12 +96,13 @@ Useful options:
7696

7797
## Scheduler
7898

79-
Configured schedules:
99+
History maintenance schedules (only run while `MONITOR_HISTORY_ENABLED=true`, and
100+
guarded with `withoutOverlapping()` so a slow run cannot stack on the next tick):
80101

81102
- `monitor:aggregate-check-metrics` hourly
82103
- `monitor:prune-check-history` daily at `01:00`
83104

84-
Existing schedules remain unchanged:
105+
Existing core schedules remain unchanged and always run:
85106

86107
- `monitor:check-uptime` every minute
87108
- `monitor:check-certificate` daily
@@ -101,22 +122,27 @@ Default:
101122
## Rollout Checklist
102123

103124
1. Deploy migrations for history tables and idempotency key.
104-
2. Deploy ingestion listeners/model hooks/services.
105-
3. Enable scheduler entries for aggregate/prune commands.
106-
4. Run backfill in `--dry-run` mode and review expected writes.
107-
5. Run live backfill once approved.
108-
6. Validate monitor detail page:
125+
2. Deploy the code (ingestion hooks, scheduler entries, services). All of it stays
126+
dormant while `MONITOR_HISTORY_ENABLED` is off.
127+
3. (Optional) Pre-stage approximate history while the flag is still off:
128+
- Run `monitor:backfill-check-history --dry-run` and review expected writes.
129+
- Run the live backfill once approved.
130+
4. Enable `MONITOR_HISTORY_ENABLED=true` in the target environment. This activates,
131+
together: per-check ingestion, the hourly aggregate + daily prune schedules, and
132+
the detail-page payload. Real history accumulates from this point forward.
133+
5. Validate the monitor detail page:
109134
- range filters work
110135
- heatmaps render by check type
111136
- recent checks and totals are sensible
112-
7. Enable `MONITOR_HISTORY_ENABLED=true` in target environment.
113-
8. Monitor logs for command failures and verify aggregate row growth.
137+
6. Monitor logs for command failures and verify aggregate row growth.
114138

115139
## Troubleshooting
116140

117141
### No heatmap data visible
118142

119-
- Confirm feature flag is enabled.
143+
- Confirm `MONITOR_HISTORY_ENABLED=true`. Ingestion only writes logs while the flag
144+
is on, so a freshly enabled feature has no history until checks run (or a backfill
145+
is performed).
120146
- Confirm `monitor_check_logs` has data.
121147
- Run `monitor:aggregate-check-metrics --lookback=30`.
122148
- Check selected timezone/range in monitor detail.

0 commit comments

Comments
 (0)