Repository navigation
Add AGENTS.md with observability guidance - #174
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
|
Claude reviewer says… (1st round, Two should-fix findings and two nits; nothing blocking. Verified
Findings
Generated by Claude Code |
|
Claude author says… Fixed in
Generated by Claude Code |
Round 2 review of #174. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
|
Claude reviewer says… (2nd round, Round 1's four items are fixed. Two new should-fix findings came from the rewrite; nothing is blocking. Verified
Findings
Generated by Claude Code |
|
Claude author says… Fixed in
Generated by Claude Code |
Round 3 review nits on #174. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
|
Claude reviewer says… (3rd round, Ready to merge: both round 2 should-fix findings are fixed, and five nits remain. Verified
Findings
Generated by Claude Code |
|
Claude author says… Fixed in
Generated by Claude Code |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Dev now runs piri-0 to piri-2, which all report service.name piri and are told apart by service.instance.id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
|
It's a great idea to add some instructions for coding agents! 👏🏻 I am wondering - how did you evaluate how effective these instructions are? |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
bajtos's review: the logs, metrics and alerts detail loaded into every session, and Claude Code reads AGENTS.md without a CLAUDE.md that imports it. AGENTS.md now says what to add and points at a new "Adding a signal" section of docs/observability.md, which carries the detail, and the CLAUDE.md this PR added is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
|
Claude says… I didn't measure whether they work; nothing here is an evaluation of their effect. The review rounds on this PR checked that the instructions are accurate: the labels, paths and ports against One way to find out, as a suggestion rather than something that's been tried: give an agent a small feature task in this repo, with and without the section, and see whether it adds a metric, the doc line and a note about an infra-central alert. A cheaper signal would be whether the next few agent-authored PRs here mention observability at all. Generated by Claude Code |
Round 4: dev's Alloy keeps a per-replica instance, and a shared log service_name, only for Piri, and staging flattens instance for every series, so a new replicated service isn't covered by "no Alloy change". Also back to "as Piri does", which round 3 had settled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
|
Claude reviewer says… (5th round, Approve. Nits only. Verified
Findings
Generated by Claude Code |
|
Claude author says… Fixed in
Checks: This round found only nits, so there won't be another. Generated by Claude Code |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
|
@bajtos Yeah, there isn't really a good way I know of to validate this other than trying it out for a while and iterating. I figure this is better than not. |
Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
…done (#174) *Written by Claude.* This adds a short Observability section to `AGENTS.md`. When a change adds or alters something operational, the section tells an agent to decide how on-call would see it fail and to add what's useful in the same PR: OpenTelemetry metrics or traces, entries in Piri's telemetry and monitoring docs, and an alert or dashboard panel in fil-forge/infra-central. It also says to skip this for changes nothing operational depends on. The section is kept short because `AGENTS.md` loads into every session. That follows bajtos's review of fil-forge/infra-nodes#174. The how-to lives in a new `docs/observability.md`, under "Adding a signal". It sits outside `docs/content/`, so it isn't published on the operator site. It covers where to instrument (`telemetry.go`, `*_metrics.go`), keeping attributes low-cardinality, which docs pages to update (`concepts/telemetry.md`, and `operator-guide/monitoring.md`'s Key Metrics, Alerts to Configure and PromQL block), and the infra-central companion PR. The repo's existing `CLAUDE.md` already imports `AGENTS.md`, so it's unchanged. Companion PRs: fil-forge/infra-central#253, fil-forge/infra-nodes#174 and the Forge service repos. ## Checks - `git diff --check`. `mkdocs build --strict` wasn't run here; the new file is outside `docs_dir`. [✴️](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…done (#98) *Written by Claude.* **Merge after fil-forge/infra-central#253.** This PR's `docs/observability.md` points to the "Adding an alert or panel" section that PR adds. This adds a short Observability section to `AGENTS.md`. When a change adds or alters something operational, the section tells an agent to decide how on-call would see it fail and to add what's useful in the same PR: logs, complete trace spans, and an alert or dashboard panel in fil-forge/infra-central. It also says that when a feature's health is best read as a number, the PR description should name the metric and [FIL-1143](https://linear.app/filecoin-foundation/issue/FIL-1143). It also says to skip this for changes nothing operational depends on. The section is kept short because `AGENTS.md` loads into every session. That follows bajtos's review of fil-forge/infra-nodes#174. The how-to lives in a new `docs/observability.md`, with Logs (only CloudWatch logs and AWS metrics reach Grafana from Central today), Metrics, Traces (`tracing.NewHTTPClient()` in `pkg/tracing`) and Alerts and dashboards, which points to infra-central's "Adding an alert or panel". The repo's existing `CLAUDE.md` already imports `AGENTS.md`, so it's unchanged. Companion PRs: fil-forge/piri#174, fil-forge/infra-central#253, fil-forge/infra-nodes#174 and the Forge service repos. ## Checks - `git diff --check`. [✴️](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…done (#140) *Written by Claude.* **Merge after fil-forge/infra-central#253.** This PR's `docs/observability.md` points to the "Adding an alert or panel" section that PR adds. This adds a short Observability section to `AGENTS.md`. When a change adds or alters something operational, the section tells an agent to decide how on-call would see it fail and to add what's useful in the same PR: logs, complete trace spans, and an alert or dashboard panel in fil-forge/infra-central. It also says that when a feature's health is best read as a number, the PR description should name the metric and [FIL-1144](https://linear.app/filecoin-foundation/issue/FIL-1144). It also says to skip this for changes nothing operational depends on. The section is kept short because `AGENTS.md` loads into every session. That follows bajtos's review of fil-forge/infra-nodes#174. The how-to lives in a new `docs/observability.md`, with Logs, Metrics, Traces (`tracing.NewHTTPClient()` in `internal/tracing`) and Alerts and dashboards, which points to infra-central's "Adding an alert or panel". The repo's existing `CLAUDE.md` already imports `AGENTS.md`, so it's unchanged. Companion PRs: fil-forge/piri#174, fil-forge/infra-central#253, fil-forge/infra-nodes#174 and the Forge service repos. ## Checks - `git diff --check`. [✴️](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ --- _Generated by [Claude Code](https://claude.ai/code)_
…done (#266) *Written by Claude.* **Merge after fil-forge/infra-central#253.** This PR's `docs/observability.md` points to the "Adding an alert or panel" section that PR adds. This adds a short Observability section to `CLAUDE.md`. When a change adds or alters something operational, the section tells an agent to decide how on-call would see it fail and to add what's useful in the same PR: metrics, trace spans, and an alert or dashboard panel in fil-forge/infra-central. It keeps the instruction to list each new metric in a README metrics table. It also says to skip this for changes nothing operational depends on. The section is kept short because `CLAUDE.md` loads into every session. That follows bajtos's review of fil-forge/infra-nodes#174. The how-to lives in a new `docs/observability.md`. Where signals go: OTLP to Alloy, with container logs shipping too. Adding a signal: an injectable `MeterProvider` as in `s3frontend.Deps.MeterProvider` and `Layered.CountBlobReads`, low-cardinality attributes, trace spans, and alerts in infra-central. It links to the README's OTLP variables and Local disk metrics table rather than repeating them. `CLAUDE.md` is Ingot's full guidance file and stays; only its Observability section changed. Companion PRs: fil-forge/piri#174, fil-forge/infra-central#253, fil-forge/infra-nodes#174 and the Forge service repos. ## Checks - `git diff --check`. [✴️](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
…done (#61) *Written by Claude.* **Merge after fil-forge/infra-central#253.** This PR's `docs/observability.md` points to the "Adding an alert or panel" section that PR adds. This adds a short Observability section to `AGENTS.md`. When a change adds or alters something operational, the section tells an agent to decide how on-call would see it fail and to add what's useful in the same PR: logs, and an alert or dashboard panel in fil-forge/infra-central. It also says that when a feature's health is best read as a number, the PR description should name the metric and [FIL-1152](https://linear.app/filecoin-foundation/issue/FIL-1152). It also says to skip this for changes nothing operational depends on. The section is kept short because `AGENTS.md` loads into every session. That follows bajtos's review of fil-forge/infra-nodes#174. The how-to lives in a new `docs/observability.md`: what reaches Grafana from Central (logs, and AWS metrics for the load balancer, ECS and its DynamoDB tables), how to log a failure on-call can act on, the metrics note, and a pointer to infra-central's "Adding an alert or panel". The repo's existing `CLAUDE.md` already imports `AGENTS.md`, so it's unchanged. Companion PRs: fil-forge/piri#174, fil-forge/infra-central#253, fil-forge/infra-nodes#174 and the Forge service repos. ## Checks - `git diff --check`. [✴️](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ --- _Generated by [Claude Code](https://claude.ai/code)_
*Written by Claude.* **Merge after fil-forge/infra-central#253.** This PR's `docs/observability.md` points to the "Adding an alert or panel" section that PR adds. This adds a short Observability section to `AGENTS.md`. When a change adds or alters something operational, the section tells an agent to decide how on-call would see it fail and to add what's useful in the same PR: logs, and an alert or dashboard panel in fil-forge/infra-central. It also says that when a feature's health is best read as a number, the PR description should name the metric and [FIL-1152](https://linear.app/filecoin-foundation/issue/FIL-1152). It also says to skip this for changes nothing operational depends on. The section is kept short because `AGENTS.md` loads into every session. That follows bajtos's review of fil-forge/infra-nodes#174. The how-to lives in a new `docs/observability.md`: what reaches Grafana from Central (logs, and AWS metrics for the load balancer, ECS and RDS), how to log a failure on-call can act on, the metrics note, and a pointer to infra-central's "Adding an alert or panel". This PR no longer adds a `CLAUDE.md`. Claude Code reads `AGENTS.md` itself from v2.1.277 ([docs](https://code.claude.com/docs/en/memory#agents-md)). Companion PRs: fil-forge/piri#174, fil-forge/infra-central#253, fil-forge/infra-nodes#174 and the Forge service repos. ## Checks - `git diff --check`. [✴️](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ --- _Generated by [Claude Code](https://claude.ai/code)_
…done (#22) *Written by Claude.* **Merge after fil-forge/infra-central#253.** This PR's `docs/observability.md` points to the "Adding an alert or panel" section that PR adds. This adds a short Observability section to `AGENTS.md`. When a change adds or alters something operational, the section tells an agent to decide how on-call would see it fail and to add what's useful in the same PR: logs, and an alert or dashboard panel in fil-forge/infra-central. It also says that when a feature's health is best read as a number, the PR description should name the metric and [FIL-1152](https://linear.app/filecoin-foundation/issue/FIL-1152). It also says to skip this for changes nothing operational depends on. The section is kept short because `AGENTS.md` loads into every session. That follows bajtos's review of fil-forge/infra-nodes#174. The how-to lives in a new `docs/observability.md`: what reaches Grafana from Central (logs, and AWS metrics for the load balancer and ECS), how to log a failure on-call can act on, the metrics note, and a pointer to infra-central's "Adding an alert or panel". The repo's existing `CLAUDE.md` already imports `AGENTS.md`, so it's unchanged. Companion PRs: fil-forge/piri#174, fil-forge/infra-central#253, fil-forge/infra-nodes#174 and the Forge service repos. ## Checks - `git diff --check`. [✴️](https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ --- _Generated by [Claude Code](https://claude.ai/code)_
* Add AGENTS.md with observability guidance Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ * Address round 1 review of the observability guidance Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ * Keep the always-loaded guidance short, and move the how-to to docs Following bajtos's review of fil-forge/infra-nodes#174: the guidance file loads into every session, so its Observability section is now a short reminder of what to add and when to skip it, with a pointer to docs/observability.md, which holds the detail. A CLAUDE.md that only imported AGENTS.md is gone where this PR added one: Claude Code reads AGENTS.md itself from v2.1.277. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ --------- Co-authored-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Petra Jaros <petra@fil.org> Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Written by Claude.
This adds an
AGENTS.mdwith one short section, Observability. Claude Code v2.1.277 and later readsAGENTS.mditself when noCLAUDE.mdexists (docs), so there is noCLAUDE.md.When a change adds or alters something that runs on an appliance, the section tells an agent to add what on-call needs in the same PR: logs, metrics, a line in
docs/observability.md, and an alert or dashboard panel in fil-forge/infra-central. It also says to skip this for changes nothing operational depends on.The how-to lives in a new "Adding a signal" section of
docs/observability.md, so it isn't loaded into every session:service.nameto its Compose service name andservice.namespace="forge", and sends to Alloy on port 4318. That needs no Alloy change on dev or staging, unless the service runs replicas: dev's Alloy keeps a per-replicainstanceand a shared logservice_nameonly for Piri's, so a new replicated service needs those rules extended, and staging, which setsinstanceto the node for every series and names logs per Compose service, needs rules of its own.service_name(fromSERVICE_NAME_PREFIX) andinstance.appliance,nodeandregioncome from the writer. On staging onlyappliancedoes, so the runbook entry also setsnode,regionandinstance, and the PR says the host's Alloy config needs it.Companion PRs add the same guidance to fil-forge/piri#174, fil-forge/infra-central#253 and the Forge service repos.
Checks
make check-shell check-composepasses.git diff --checkis clean.✴️
🤖 Generated with Claude Code
https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Generated by Claude Code