Skip to content

Commit d0dab11

Browse files
jessarcheravosalmon
jessarcher
and
avosalmon
authored
Add AGENTS.md file (#303)
Co-authored-by: Ryuta Hamasaki <ryuta@laravel.com>
1 parent 9584a2e commit d0dab11

1 file changed

Lines changed: 339 additions & 0 deletions

File tree

‎AGENTS.md‎

Lines changed: 339 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,339 @@
1+
# Laravel Nightwatch Agent Guidelines
2+
3+
## Project Overview
4+
5+
Laravel Nightwatch is a hosted application monitoring platform package for Laravel. It gathers metrics within Laravel applications and transmits them to the Nightwatch service. This is an official Laravel package that must maintain high code quality standards.
6+
7+
## Code Style & Formatting
8+
9+
- **Use Laravel Pint** with the project's configuration (`pint.json`)
10+
- **Strict comparisons**: Use `===` and `!==` instead of `==` and `!=`
11+
- **Strict parameters**: Enable strict parameter checking
12+
- **Global namespace imports**: Import classes, constants, and functions from global namespace
13+
- **Native function invocation**: Use namespaced native function calls
14+
- **Method chaining indentation**: Properly indent method chains
15+
- **Explicit string variables**: Use explicit string variable syntax when appropriate
16+
- **Combine consecutive isset/unset**: Group consecutive isset/unset calls
17+
- **Ternary to null coalescing**: Prefer null coalescing operator (`??`) over ternary
18+
19+
## Type Safety
20+
21+
- **PHPStan Level Max**: All code must pass PHPStan at maximum level
22+
- **Type hints**: Always provide type hints for:
23+
- Method parameters
24+
- Return types
25+
- Properties (with PHPDoc when needed)
26+
- **Generic types**: Use PHPDoc generics for complex types (e.g., `@template TState`)
27+
- **Array shapes**: Document array shapes with PHPDoc (e.g., `array{enabled: bool, sampling: array{...}}`)
28+
29+
## Architecture Patterns
30+
31+
### Hooks Pattern
32+
- Hooks are event listeners that capture Laravel framework events
33+
- Located in `src/Hooks/`
34+
- Should be `final` classes marked `@internal`
35+
- Always wrap hook logic in try-catch and report exceptions as handled
36+
- Example pattern:
37+
```php
38+
final class QueryExecutedListener
39+
{
40+
public function __construct(
41+
private Core $nightwatch,
42+
) {}
43+
44+
public function __invoke(QueryExecuted $event): void
45+
{
46+
try {
47+
$this->nightwatch->query($event);
48+
} catch (Throwable $e) {
49+
$this->nightwatch->report($e, handled: true);
50+
}
51+
}
52+
}
53+
```
54+
55+
### Sensors Pattern
56+
- Sensors transform events into records and payload factories
57+
- Located in `src/Sensors/`
58+
- Should be `final` classes marked `@internal`
59+
- Return tuple: `[Record, callable(): array<mixed>]` (for record-based sensors) or `?array` (for direct array sensors like `ScheduledTaskSensor`)
60+
- Use `Location` helper for file/line detection
61+
- Use `Clock` for timestamp management
62+
- **Check property visibility**: Before using reflection, verify if Laravel properties are public. Prefer direct property access over reflection when possible.
63+
- **Trust Laravel's internal constraints**: If Laravel enforces constraints on a value (e.g., requires >= 1 second for repeating tasks), trust those constraints rather than re-checking in our code
64+
65+
### State Management
66+
- Use `RequestState` for HTTP requests
67+
- Use `CommandState` for Artisan commands, scheduled tasks, and job attempts
68+
- State is managed through `Core` class
69+
- Execution stages tracked via `ExecutionStage` enum
70+
71+
### Concerns/Traits
72+
- Shared functionality goes in `src/Concerns/`
73+
- Examples: `CapturesState`, `RedactsRecords`, `RejectsRecords`
74+
- Use traits to keep `Core` class focused
75+
76+
## Class Design
77+
78+
- **Final classes**: Use `final` for internal classes that shouldn't be extended
79+
- **Visibility**: Use `private` by default, `protected` only when necessary
80+
- **Property promotion**: Prefer constructor property promotion
81+
- **Readonly**: Consider `readonly` properties when appropriate (PHP 8.2+)
82+
- **Avoid unnecessary variables**: Only create variables when they're used multiple times or significantly improve readability. Don't create variables for single-use values that can be inlined directly.
83+
84+
## Documentation
85+
86+
- **@internal**: Mark all internal classes/methods with `@internal`
87+
- **@api**: Mark public API methods with `@api`
88+
- **PHPDoc**: Provide comprehensive PHPDoc for:
89+
- Complex types
90+
- Generic types
91+
- Array shapes
92+
- Callable signatures
93+
- **Method documentation**: Document public methods, especially those marked `@api`
94+
- **Avoid unnecessary comments**: Don't add inline comments unless necessary, especially when existing code doesn't already have them for similar scenarios. Let the code be self-documenting through clear naming and structure.
95+
96+
## Error Handling
97+
98+
- **Graceful degradation**: Never let monitoring code break the application
99+
- **Exception handling**: Always catch `Throwable` in hooks
100+
- **Report as handled**: Internal exceptions should be reported as `handled: true`
101+
- **Unrecoverable exceptions**: Use `Nightwatch::unrecoverableExceptionOccurred()` for fatal errors
102+
103+
## Testing
104+
105+
- **Test coverage**: Maintain high test coverage
106+
- **Test structure**:
107+
- Feature tests in `tests/Feature/`
108+
- Unit tests in `tests/Unit/`
109+
- **Test helpers**: Use `TestCase` helper methods:
110+
- `fakeIngest()` for mocking ingest
111+
- `fakeTcpStreams()` for testing network
112+
- `setExecutionStart()` for time manipulation
113+
- **Test isolation**: Each test should be independent
114+
- **Test naming**: Use descriptive test method names
115+
116+
## Compatibility
117+
118+
- **Laravel versions**: Support Laravel 10, 11, and 12
119+
- **PHP versions**: Minimum PHP 8.2
120+
- **Backward compatibility**: Maintain backward compatibility within major versions
121+
- **Compatibility layer**: Use `Compatibility` class for version-specific code
122+
123+
### Compatibility Flags
124+
125+
Compatibility flags in the `Compatibility` class are used to handle version-specific features and APIs. Use them strategically:
126+
127+
**When to use compatibility flags:**
128+
129+
1. **Events that don't fire in older versions**
130+
- Example: `$cacheDurationCapturable` - start events (`RetrievingKey`, `WritingKey`, etc.) don't fire in older versions, so the sensor needs to know not to expect them for duration calculation
131+
- Example: `$terminatingEventExists` - the `Terminating` event doesn't exist/fire in Laravel < 11.18.0
132+
- Prevents the sensor from waiting for events that will never fire
133+
134+
2. **Classes/facades that don't exist in older versions**
135+
- Example: `$contextExists` - the `Context` facade doesn't exist in Laravel < 11
136+
- Prevents fatal errors from accessing non-existent classes/methods
137+
138+
3. **Features added in specific versions that need conditional logic**
139+
- Example: `$subMinuteScheduledTasksSupported` - sub-minute scheduling was added in Laravel 10.15.0
140+
- Documents when features were introduced and provides explicit feature detection
141+
142+
4. **When fallback behavior is needed**
143+
- Example: `$queuedJobDurationCapturable` - duration defaults to `0` if not capturable
144+
- Allows graceful degradation when features aren't available
145+
146+
**When NOT to use compatibility flags:**
147+
148+
1. **Properties that always exist but may be null**
149+
- Example: `$event->task->expression`, `$event->task->timezone` - always exist, use null coalescing (`??`) if needed
150+
- No need for a flag if the property exists in all supported versions
151+
152+
2. **When null coalescing operator is sufficient**
153+
- Example: `$event->storeName ?? ''` - property may not exist, but `??` handles it gracefully
154+
- Simpler than adding a compatibility flag for optional properties
155+
156+
**Guidelines:**
157+
158+
- **Document the feature**: Always include PR link and version tag in the compatibility flag comment. You MUST verify both the PR link and version tag are correct using the process below - never guess or reuse PR numbers from other features.
159+
- **How to find the correct PR and version**:
160+
1. Search GitHub: `curl -s "https://api.github.com/search/issues?q=repo:laravel/framework+<feature-keywords>+is:pr+is:merged" | grep -E '"title"|"number"|"html_url"'`
161+
2. Verify the PR title and number match the feature you're implementing
162+
3. Check the PR's merge date: `curl -s "https://api.github.com/repos/laravel/framework/pulls/<PR_NUMBER>" | grep "merged_at"`
163+
4. Find which release contains this PR: `curl -s "https://api.github.com/repos/laravel/framework/releases/tags/v<VERSION>" | grep "<PR_NUMBER>"`
164+
5. Verify the release notes explicitly mention the PR number
165+
- **Common mistakes to avoid**:
166+
- Using similar PR numbers without verification (e.g., #47310 vs #47279)
167+
- Guessing version numbers based on approximate dates
168+
- Not checking if the PR actually exists or matches the feature
169+
- **Be explicit**: Use flags to make intent clear (feature support vs. property existence)
170+
- **Consistency**: Follow existing patterns in the codebase
171+
- **Performance**: Flags are evaluated once at boot time, so they're efficient for runtime checks
172+
- **Test coverage**: Ensure tests cover both supported and unsupported versions when using flags
173+
174+
## Performance
175+
176+
- **Sampling**: Respect sampling configuration
177+
- **Buffering**: Use `RecordsBuffer` for efficient batching
178+
- **Memory**: Be mindful of memory usage, especially in long-running processes
179+
180+
## Security & Privacy
181+
182+
- **Redaction**: Always redact sensitive data:
183+
- Passwords
184+
- Tokens
185+
- Authorization headers
186+
- Configurable via `redact_payload_fields` and `redact_headers`
187+
- **Filtering**: Respect filtering configuration
188+
- **No PII**: Avoid capturing personally identifiable information unless explicitly configured
189+
190+
## Configuration
191+
192+
- **Environment variables**: Use `env()` helper with defaults
193+
- **Config file**: Add new config options to `config/nightwatch.php`
194+
- **Dot notation**: Use config dot notation for nested values
195+
- **Defaults**: Provide sensible defaults
196+
197+
## Records & Payloads
198+
199+
- **Record classes**: Located in `src/Records/`
200+
- **Payload structure**: Follow existing payload structure patterns
201+
- **Versioning**: Use payload version (`'v' => 1`)
202+
- **Lazy values**: Use `LazyValue` for values that cannot be determined at event capture time and need to be evaluated later when the record is serialized. This is for values that depend on state that will change or finalize between when the event is captured and when the record is sent to the ingest service
203+
- **Grouping**: Use `_group` hash for similar events. The `_group` key identifies the "type" or "identity" of an event, grouping events that are the same "thing" even if they're different executions. **Include fields that define the identity of the event, not execution details or results.** Guidelines:
204+
- **Include**: Fields that define WHAT the thing is or WHEN it's scheduled (e.g., task name, cron expression, route path, cache key, job class, query SQL)
205+
- **Exclude**: Fields that define HOW it runs or WHAT happened (e.g., withoutOverlapping, duration, status, memory usage, response code)
206+
- **Identity vs. Execution**: Identity fields define the "what" (e.g., route path for HTTP requests, cache key for cache operations, SQL for queries), while execution details define the "how" or results (e.g., middleware applied, cache hit/miss, query bindings)
207+
- **Consistency**: Changing identity fields should create a new group; changing execution details should not. For example, changing a route path creates a new group, but changing response status does not
208+
- **Backward compatibility**: When adding new fields to `_group` hashes, consider whether to include them conditionally to maintain existing group hashes. For example, only include `repeat_seconds` when it's non-zero to preserve hashes for regular scheduled tasks
209+
- **Examples**:
210+
- ✅ Scheduled task: `{name},{expression},{timezone},{repeatSeconds}` when repeatSeconds > 0, else `{name},{expression},{timezone}` - defines the schedule identity while preserving backward compatibility
211+
- ✅ HTTP request: `{methods},{domain},{route}` - defines the route identity
212+
- ✅ Cache event: `{store},{key}` - defines what's being cached
213+
- ❌ Don't include: `withoutOverlapping`, `onOneServer`, `duration`, `status`, `memoryUsage`
214+
- **String limits**: Use `Str::tinyText()`, `Str::mediumText()` for length limits
215+
- **Field naming - ALWAYS check Laravel source code**: **Before creating any new field names, check Laravel's source code for the property names.** Look at the actual class properties and methods in Laravel's framework code. Convert camelCase property names to snake_case for JSON payloads. Use Laravel's property names rather than creating new terminology.
216+
- **Complete records**: **All records must always contain all keys.** Optional fields should use appropriate sentinel values rather than being omitted from the record. This ensures consistent record structure across all records, which is important for data consistency, easier querying, backward compatibility, and type safety.
217+
- **ClickHouse storage**: Data is stored in ClickHouse, which doesn't typically use Nullable fields due to the extra storage required. **Prefer using sentinel values instead of `null`** whenever possible. Choose sentinel values based on semantic validity:
218+
- **Strings**: Use empty strings (`''`) when an empty string is not a meaningful value for the field
219+
- **Numbers**: Use `0` when zero is **semantically invalid** for that field. Examples:
220+
- ✅ `repeat_seconds => 0` for non-repeating tasks (Laravel requires ≥1 second)
221+
- ✅ `duration => 0` for skipped tasks (they don't actually run)
222+
- ❌ Don't use `0` if zero is a valid/meaningful value (e.g., `offset`, `retry_count`)
223+
- **Booleans**: Use `false` when false is the appropriate default/absence state
224+
- **Only use `null` when there is no appropriate sentinel value**: Use `null` only when `0`/`false`/`''` would be ambiguous or when the absence of a value is semantically different from any sentinel value. For example:
225+
- ✅ Use `null` if both `0` and positive numbers are valid, and you need to distinguish "not set"
226+
- ✅ Use `null` if an empty string is a valid/meaningful value distinct from "not set"
227+
- **Check the existing codebase**: Look for similar fields in other sensors to maintain consistency
228+
229+
## Agent Development
230+
231+
- **Agent code**: Located in `agent/` directory
232+
- **Build process**: Use `build.sh` for building PHAR
233+
- **Docker**: Agent has Docker support
234+
- **Scoping**: Use `scoper.inc.php` for PHAR scoping
235+
236+
## Git & Versioning
237+
238+
- **Branch**: Work on `1.x` branch
239+
- **Changelog**: Update `CHANGELOG.md` for user-facing changes
240+
- **Version**: Version managed in `version.txt`
241+
- **Commits**: Write clear, descriptive commit messages
242+
243+
## Development Workflow
244+
245+
- **Always run checks**: After creating or editing files, always run:
246+
- **Tests**: Run the test suite to ensure all tests pass
247+
- **Linting**: Run Pint to ensure code formatting is correct
248+
- **Static analysis**: Run PHPStan to ensure type safety and catch errors
249+
- **Before committing**: Ensure all checks pass before committing code
250+
- **Automated checks**: The CI pipeline will verify these, but catch issues locally first
251+
- **Verify links and PRs**: When adding PR references or version tags (especially in compatibility flags), use the GitHub API or curl commands to verify:
252+
- The PR number is correct and matches the feature
253+
- The version tag actually contains that PR in its release notes
254+
- Don't reuse PR numbers from other features or guess version numbers - always verify through GitHub's API or release notes
255+
- **Verify Laravel source**: Before accessing Laravel properties or methods, check the actual Laravel source code to confirm:
256+
- Property names and types
257+
- Whether properties are public or require reflection
258+
- What values Laravel provides (e.g., `null` vs `0`)
259+
- **Valid value ranges** - Check for validation logic or constraints that indicate which values are semantically valid (e.g., if Laravel requires a value to be ≥1, then `0` can be used as a sentinel for "not set")
260+
- When features were actually introduced (check changelogs, not just version tags)
261+
- **Match Laravel conventions**: When adding new fields to payloads, **always check Laravel's source code** for the property names. Convert camelCase property names to snake_case for JSON payloads. Use Laravel's property names rather than creating new terminology. This ensures consistency with Laravel's own APIs and makes the data more familiar to Laravel developers.
262+
- **Group related fields**: Place new fields logically alongside related fields in payloads (e.g., scheduling-related fields grouped together)
263+
264+
## Code Review Checklist
265+
266+
- [ ] Code passes PHPStan at max level
267+
- [ ] Code formatted with Pint
268+
- [ ] All tests pass
269+
- [ ] New code has tests
270+
- [ ] PHPDoc is complete
271+
- [ ] Internal classes marked `@internal`
272+
- [ ] Public API marked `@api`
273+
- [ ] Error handling is graceful
274+
- [ ] Sensitive data is redacted
275+
- [ ] Performance considerations addressed
276+
- [ ] Backward compatibility maintained
277+
- [ ] Configuration options documented
278+
279+
## Common Patterns
280+
281+
### Capturing Events
282+
```php
283+
// In Hook
284+
try {
285+
$this->nightwatch->query($event);
286+
} catch (Throwable $e) {
287+
$this->nightwatch->report($e, handled: true);
288+
}
289+
```
290+
291+
### Creating Records
292+
```php
293+
// In Sensor
294+
return [
295+
$record = new Query(...),
296+
function () use ($event, $record) {
297+
$this->executionState->queries++;
298+
return [
299+
'v' => 1,
300+
't' => 'query',
301+
'timestamp' => $this->clock->microtime(),
302+
// ... other fields
303+
];
304+
},
305+
];
306+
```
307+
308+
### State Management
309+
```php
310+
// Update execution state
311+
$this->executionState->stage = ExecutionStage::Action;
312+
$this->executionState->currentExecutionStageStartedAtMicrotime = $this->clock->microtime();
313+
```
314+
315+
## Don'ts
316+
317+
- ❌ Don't break backward compatibility
318+
- ❌ Don't let monitoring code crash the application
319+
- ❌ Don't capture sensitive data without redaction
320+
- ❌ Don't skip error handling in hooks
321+
- ❌ Don't use `public` properties unless necessary
322+
- ❌ Don't forget to mark internal code with `@internal`
323+
- ❌ Don't ignore PHPStan errors
324+
- ❌ Don't commit code that doesn't pass Pint
325+
- ❌ Don't add features without tests
326+
- ❌ Don't use `dd()` or `dump()` in production code
327+
328+
## Do's
329+
330+
- ✅ Do use strict typing
331+
- ✅ Do handle all exceptions gracefully
332+
- ✅ Do write comprehensive tests
333+
- ✅ Do document public APIs
334+
- ✅ Do respect configuration options
335+
- ✅ Do use final classes for internal code
336+
- ✅ Do follow Laravel conventions
337+
- ✅ Do maintain backward compatibility
338+
- ✅ Do optimize for performance
339+
- ✅ Do redact sensitive data

0 commit comments

Comments
 (0)