Skip to content

feat: add dash0 slos command group (SLO IaC support) - #207

Merged
nicolastakashi merged 30 commits into
mainfrom
slos
Sep 11, 2026
Merged

feat: add dash0 slos command group (SLO IaC support)#207
nicolastakashi merged 30 commits into
mainfrom
slos

Conversation

@nicolastakashi

@nicolastakashi nicolastakashi commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Adds a dash0 slos command group (create / get / list / update / delete) for managing service level objectives (SLOs) as code, backed by the /api/slos endpoint using OpenSLO documents (apiVersion: openslo.com/v1, kind: SLO).

  • Dataset-scoped via --dataset; ID at metadata.labels."dash0.com/id"; deep-link URL support (/goto/alerting/slos/details).
  • Wires SLO into apply (create-or-replace by ID).
  • Integration tests against the mock server, plus a roundtrip test (test/roundtrip/test_slo_roundtrip.sh, registered in run_all.sh) — validated end-to-end against dev: create → list → get → export → apply → delete.
  • Docs (docs/commands.md, docs/cli-naming-conventions.md) and a .chloggen/ entry.

Part of cross-facility SLO IaC support (API client, CLI, Terraform provider, Kubernetes operator).

Dependency

Builds against the published github.com/dash0hq/dash0-api-client-go v1.18.0 (SLO client support) — no local replace. The openslo.com/v1 apiVersion is the domain-qualified form the backend dual-accepts on write and canonicalizes on read (INS-551), so the same document is installable as the operator's Kubernetes CRD.

Status

Rebased on main. build / test-unit / test-integration / lint pass. test_slo_roundtrip and test_apply_slo_idempotency pass against dev on the static-token shard. The oauth shard reports green but its steps are skipped — DASH0_OAUTH_REFRESH_TOKEN is not provisioned, so that column carries no signal (see #236 discussion). Any red on unrelated test_log_roundtrip / test_span_roundtrip / test_apply_check_rule_idempotency is dev read-lag, not from this change.

Closes #206

nicolastakashi
nicolastakashi commented Jul 28, 2026
View reviewed changes

@nicolastakashi nicolastakashi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: dash0 slos command group

Overall this is a faithful clone of the existing views / synthetic-checks shape — flag registration, the asset.Import* helper, error contexts, table columns, roundtrip script structure, and fixture layout all match. No new patterns introduced for problems the repo already solves, and apply.go touches exactly the three kind switches it needs plus KindDisplayName (I checked; none missed).

Two places diverge from the established pattern in ways that matter, plus one open question and one test gap. Details inline; comments use conventional comment labels.

Blocking: slos delete --force is missing the client.IsAlreadyDeleted branch that all 8 other delete commands have — that regresses the invariant shipped in 1.16.2 (#217).

Verified while reviewing (no action needed): SloTimeWindow.Duration resolves to type Duration = string in api-client v1.18.0, so the sigsyaml.Unmarshal in parseDocumentHeader is safe — no time.Duration field, unlike the prometheusrule case that carries the warning comment. AssetType: "SLO" in uppercase is the right deviation from the lowercase convention: it renders as SLO "x" not found and capitalizeFirst is a no-op on it. SloSpec has no unstably-ordered list field, so nothing needs the SortViewPermissions treatment from #231.

Comment thread internal/slos/delete.go
Comment thread internal/slos/update.go
Comment thread internal/asset/slo.go Outdated
Comment thread test/roundtrip/run_all.sh
Comment thread internal/slos/integration_test.go
Comment thread internal/slos/list.go
Comment thread internal/slos/list.go Outdated
Comment thread docs/commands.md Outdated
nicolastakashi added a commit that referenced this pull request Jul 28, 2026
@nicolastakashi @claude
fix(slos): address PR #207 review feedback
8825f9a 
- delete: add IsAlreadyDeleted idempotency branch so `slos delete --force`
  on an already-deleted SLO exits 0 (matches the 8 other delete commands,
  CHANGELOG 1.16.2 / #217); cover with an integration test.
- update: StripSLOServerFields before the dry-run diff and PUT so
  `get -o yaml | update -f` no longer sends server-managed labels/timestamps
  or shows a spurious diff (mirrors views/update.go).
- asset/ImportSLO: mirror ImportTeam upsert-key selection — prefer
  dash0.com/id, fall back to dash0.com/origin (read before strip), POST only
  when neither is present, so an origin-only document upserts via PUT instead
  of duplicating on every apply (#227 shape).
- list: replace interface{} with any on the column closures (matches
  views/list.go).
- integration tests: assert the update wire body strips dash0.com/version,
  dash0.com/origin, and created-at; add create/update --dry-run tests; add a
  second element to list_success.json and exercise --limit truncation.
- roundtrip: add test_apply_slo_idempotency.sh (double-apply, no duplicate /
  stable id) and register it in run_all.sh.
- docs: add teams to the command-group enumerations in commands.md and
  cli-naming-conventions.md and adjust the count wording.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nicolastakashi
nicolastakashi marked this pull request as ready for review July 28, 2026 09:42
@nicolastakashi
nicolastakashi requested a review from a team as a code owner July 28, 2026 09:42
mmanciop
mmanciop requested changes Jul 28, 2026
View reviewed changes
Comment thread test/roundtrip/fixtures/slo.yaml Outdated
Comment thread test/roundtrip/test_apply_slo_idempotency.sh
Comment thread test/roundtrip/test_apply_slo_idempotency.sh
Comment thread test/roundtrip/test_slo_roundtrip.sh Outdated
nicolastakashi added a commit that referenced this pull request Jul 28, 2026
@nicolastakashi @claude
fix(slos): correct the documented upsert key, drop foreign id from PO…
cadc0b3 
…ST body

Addresses six pre-merge review findings on #207.

1. The docs taught `dash0.com/id` as the SLO upsert key while `ImportSLO` is
   origin-first, and SLO ids are server-assigned (`slo_<ulid>`) so the pinned
   example id could never exist. The identifier table, the SLO YAML example,
   the `apply` section, and the "notification channels and spam filters are the
   two exceptions" sentence (in both docs/commands.md and the skill's SKILL.md)
   now say origin — modeled on the existing `Dash0Team` row, which describes
   the same routing.
2. `apply --dry-run` printed no identifier for an origin-only SLO document (the
   recommended form). Added the origin fallback both origin-keyed baselines
   already have.
3. `slos get <origin>` built the deep link and the id label from the raw
   argument, producing a URL that does not resolve. Both are now read off the
   response, mirroring `notificationchannels/get.go`.
4. `StripSLOServerFields` does not clear `dash0.com/id`, so the cross-org POST
   fallback (id branch, preflight 404) sent the source org's id. `ImportSLO`
   now calls the previously-unused `dash0api.ClearSLOID` on that path, and the
   regression test asserts the POST body carries no id. Also deleted the
   `ImportSLO` doc comment sentence claiming the strip clears the id.
5. There was no `slos` topic in the Agent Skill bundle: added the
   `ManifestEntry`, the generator `topicSpec`, the SKILL.md topic-index row,
   and regenerated, so `dash0 skill show slos` resolves.
6. `slos list -o wide`/`-o csv` were missing the `ORIGIN` column the shared
   format contract promises, and `slos get` printed no `Origin:` line. Since
   origin is the SLO upsert key, it is the field users script against.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mmanciop
mmanciop requested changes Jul 28, 2026
View reviewed changes
Comment thread .agents/skills/dash0-cli/references/slos.md Outdated
Comment thread .agents/skills/dash0-cli/SKILL.md Outdated
Comment thread .agents/skills/dash0-cli/SKILL.md Outdated
Comment thread .claude/skills/dash0-cli/references/slos.md Outdated
Comment thread .claude/skills/dash0-cli/references/slos.md Outdated
@nicolastakashi nicolastakashi mentioned this pull request Jul 28, 2026
Closed
nicolastakashi added a commit that referenced this pull request Jul 28, 2026
@nicolastakashi @claude
docs(slos): document the Dash0/OpenSLO v1 differences in the embedded…
4a5b580 
… skill

Second review round on #207, all on the embedded Agent Skill content.

- Add an "SLO documents and OpenSLO v1 compatibility" section to
  docs/commands.md and wire it into the `slos` skill topic. It replaces the
  flatly wrong "SLO documents use the OpenSLO v1 format" claim: the
  `apiVersion` is domain-qualified (`openslo.com/v1`, dual-accepted with the
  upstream bare `openslo/v1` on write, canonicalized on read), only a subset
  is supported (one objective, inline `ratioMetric`, `Occurrences`, rolling
  `4w`/`28d`), a table lists what is rejected with a 400, `alertPolicies` is
  called out as accepted-but-silently-ignored, and queries must be bare
  PromQL vector selectors. Verified field-by-field against `SloDefinition`
  and `SloSpec` in the API OpenAPI spec.
- Note that the same document body is what the operator's `openslo.com/v1`
  `SLO` CRD accepts (group/kind/version and every spec field the example
  uses match; `alertPolicies` and `indicatorRef` are deliberately absent),
  phrased conservatively because that CRD is still unmerged
  (dash0hq/dash0-operator#1247).
- Expand "SLOs" to "service level objectives (SLOs)" on first use in the
  SKILL.md frontmatter description.
- Reframe the origin-vs-ID prose in SKILL.md and docs/commands.md. Origin
  being the upsert key is no longer an exception list: it is the rule for
  the API-managed kinds (notification channels, spam filters, SLOs, teams),
  while views, check rules, recording rules, and synthetic checks are the
  kinds where the server treats origin as provenance and the CLI strips it.
  Dashboards are called out separately, since their `dash0Extensions.origin`
  is deprecated in favor of `dash0Extensions.id`.
- Align the skill's SLO example with test/roundtrip/fixtures/slo.yaml so
  both use the Dash0 synthetic-check metrics (`dash0.synthetic_check.runs`
  split on `dash0_synthetic_check_outcome="Healthy"`) instead of
  `http_server_request_duration_seconds_count`.

Deduplicating the two published skill copies is tracked separately in #236.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mmanciop
mmanciop reviewed Jul 28, 2026
View reviewed changes
Comment thread .agents/skills/dash0-cli/references/slos.md Outdated
@nicolastakashi
nicolastakashi marked this pull request as draft July 31, 2026 10:31
nicolastakashi added a commit that referenced this pull request Aug 4, 2026
@nicolastakashi @claude
fix(slos): address PR #207 review feedback
c66ee8c 
- delete: add IsAlreadyDeleted idempotency branch so `slos delete --force`
  on an already-deleted SLO exits 0 (matches the 8 other delete commands,
  CHANGELOG 1.16.2 / #217); cover with an integration test.
- update: StripSLOServerFields before the dry-run diff and PUT so
  `get -o yaml | update -f` no longer sends server-managed labels/timestamps
  or shows a spurious diff (mirrors views/update.go).
- asset/ImportSLO: mirror ImportTeam upsert-key selection — prefer
  dash0.com/id, fall back to dash0.com/origin (read before strip), POST only
  when neither is present, so an origin-only document upserts via PUT instead
  of duplicating on every apply (#227 shape).
- list: replace interface{} with any on the column closures (matches
  views/list.go).
- integration tests: assert the update wire body strips dash0.com/version,
  dash0.com/origin, and created-at; add create/update --dry-run tests; add a
  second element to list_success.json and exercise --limit truncation.
- roundtrip: add test_apply_slo_idempotency.sh (double-apply, no duplicate /
  stable id) and register it in run_all.sh.
- docs: add teams to the command-group enumerations in commands.md and
  cli-naming-conventions.md and adjust the count wording.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nicolastakashi added a commit that referenced this pull request Aug 4, 2026
@nicolastakashi @claude
fix(slos): correct the documented upsert key, drop foreign id from PO…
d7e7329 
…ST body

Addresses six pre-merge review findings on #207.

1. The docs taught `dash0.com/id` as the SLO upsert key while `ImportSLO` is
   origin-first, and SLO ids are server-assigned (`slo_<ulid>`) so the pinned
   example id could never exist. The identifier table, the SLO YAML example,
   the `apply` section, and the "notification channels and spam filters are the
   two exceptions" sentence (in both docs/commands.md and the skill's SKILL.md)
   now say origin — modeled on the existing `Dash0Team` row, which describes
   the same routing.
2. `apply --dry-run` printed no identifier for an origin-only SLO document (the
   recommended form). Added the origin fallback both origin-keyed baselines
   already have.
3. `slos get <origin>` built the deep link and the id label from the raw
   argument, producing a URL that does not resolve. Both are now read off the
   response, mirroring `notificationchannels/get.go`.
4. `StripSLOServerFields` does not clear `dash0.com/id`, so the cross-org POST
   fallback (id branch, preflight 404) sent the source org's id. `ImportSLO`
   now calls the previously-unused `dash0api.ClearSLOID` on that path, and the
   regression test asserts the POST body carries no id. Also deleted the
   `ImportSLO` doc comment sentence claiming the strip clears the id.
5. There was no `slos` topic in the Agent Skill bundle: added the
   `ManifestEntry`, the generator `topicSpec`, the SKILL.md topic-index row,
   and regenerated, so `dash0 skill show slos` resolves.
6. `slos list -o wide`/`-o csv` were missing the `ORIGIN` column the shared
   format contract promises, and `slos get` printed no `Origin:` line. Since
   origin is the SLO upsert key, it is the field users script against.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nicolastakashi added a commit that referenced this pull request Aug 4, 2026
@nicolastakashi @claude
docs(slos): document the Dash0/OpenSLO v1 differences in the embedded…
9b65b3a 
… skill

Second review round on #207, all on the embedded Agent Skill content.

- Add an "SLO documents and OpenSLO v1 compatibility" section to
  docs/commands.md and wire it into the `slos` skill topic. It replaces the
  flatly wrong "SLO documents use the OpenSLO v1 format" claim: the
  `apiVersion` is domain-qualified (`openslo.com/v1`, dual-accepted with the
  upstream bare `openslo/v1` on write, canonicalized on read), only a subset
  is supported (one objective, inline `ratioMetric`, `Occurrences`, rolling
  `4w`/`28d`), a table lists what is rejected with a 400, `alertPolicies` is
  called out as accepted-but-silently-ignored, and queries must be bare
  PromQL vector selectors. Verified field-by-field against `SloDefinition`
  and `SloSpec` in the API OpenAPI spec.
- Note that the same document body is what the operator's `openslo.com/v1`
  `SLO` CRD accepts (group/kind/version and every spec field the example
  uses match; `alertPolicies` and `indicatorRef` are deliberately absent),
  phrased conservatively because that CRD is still unmerged
  (dash0hq/dash0-operator#1247).
- Expand "SLOs" to "service level objectives (SLOs)" on first use in the
  SKILL.md frontmatter description.
- Reframe the origin-vs-ID prose in SKILL.md and docs/commands.md. Origin
  being the upsert key is no longer an exception list: it is the rule for
  the API-managed kinds (notification channels, spam filters, SLOs, teams),
  while views, check rules, recording rules, and synthetic checks are the
  kinds where the server treats origin as provenance and the CLI strips it.
  Dashboards are called out separately, since their `dash0Extensions.origin`
  is deprecated in favor of `dash0Extensions.id`.
- Align the skill's SLO example with test/roundtrip/fixtures/slo.yaml so
  both use the Dash0 synthetic-check metrics (`dash0.synthetic_check.runs`
  split on `dash0_synthetic_check_outcome="Healthy"`) instead of
  `http_server_request_duration_seconds_count`.

Deduplicating the two published skill copies is tracked separately in #236.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nicolastakashi added a commit that referenced this pull request Aug 4, 2026
@nicolastakashi
fix(slos): address PR #207 review feedback
f42afb3 
- delete: add IsAlreadyDeleted idempotency branch so `slos delete --force`
  on an already-deleted SLO exits 0 (matches the 8 other delete commands,
  CHANGELOG 1.16.2 / #217); cover with an integration test.
- update: StripSLOServerFields before the dry-run diff and PUT so
  `get -o yaml | update -f` no longer sends server-managed labels/timestamps
  or shows a spurious diff (mirrors views/update.go).
- asset/ImportSLO: mirror ImportTeam upsert-key selection — prefer
  dash0.com/id, fall back to dash0.com/origin (read before strip), POST only
  when neither is present, so an origin-only document upserts via PUT instead
  of duplicating on every apply (#227 shape).
- list: replace interface{} with any on the column closures (matches
  views/list.go).
- integration tests: assert the update wire body strips dash0.com/version,
  dash0.com/origin, and created-at; add create/update --dry-run tests; add a
  second element to list_success.json and exercise --limit truncation.
- roundtrip: add test_apply_slo_idempotency.sh (double-apply, no duplicate /
  stable id) and register it in run_all.sh.
- docs: add teams to the command-group enumerations in commands.md and
  cli-naming-conventions.md and adjust the count wording.
nicolastakashi added a commit that referenced this pull request Aug 4, 2026
@nicolastakashi
fix(slos): correct the documented upsert key, drop foreign id from PO…
d7cc3ea 
…ST body

Addresses six pre-merge review findings on #207.

1. The docs taught `dash0.com/id` as the SLO upsert key while `ImportSLO` is
   origin-first, and SLO ids are server-assigned (`slo_<ulid>`) so the pinned
   example id could never exist. The identifier table, the SLO YAML example,
   the `apply` section, and the "notification channels and spam filters are the
   two exceptions" sentence (in both docs/commands.md and the skill's SKILL.md)
   now say origin — modeled on the existing `Dash0Team` row, which describes
   the same routing.
2. `apply --dry-run` printed no identifier for an origin-only SLO document (the
   recommended form). Added the origin fallback both origin-keyed baselines
   already have.
3. `slos get <origin>` built the deep link and the id label from the raw
   argument, producing a URL that does not resolve. Both are now read off the
   response, mirroring `notificationchannels/get.go`.
4. `StripSLOServerFields` does not clear `dash0.com/id`, so the cross-org POST
   fallback (id branch, preflight 404) sent the source org's id. `ImportSLO`
   now calls the previously-unused `dash0api.ClearSLOID` on that path, and the
   regression test asserts the POST body carries no id. Also deleted the
   `ImportSLO` doc comment sentence claiming the strip clears the id.
5. There was no `slos` topic in the Agent Skill bundle: added the
   `ManifestEntry`, the generator `topicSpec`, the SKILL.md topic-index row,
   and regenerated, so `dash0 skill show slos` resolves.
6. `slos list -o wide`/`-o csv` were missing the `ORIGIN` column the shared
   format contract promises, and `slos get` printed no `Origin:` line. Since
   origin is the SLO upsert key, it is the field users script against.
nicolastakashi added a commit that referenced this pull request Aug 4, 2026
@nicolastakashi
docs(slos): document the Dash0/OpenSLO v1 differences in the embedded…
ef437df 
… skill

Second review round on #207, all on the embedded Agent Skill content.

- Add an "SLO documents and OpenSLO v1 compatibility" section to
  docs/commands.md and wire it into the `slos` skill topic. It replaces the
  flatly wrong "SLO documents use the OpenSLO v1 format" claim: the
  `apiVersion` is domain-qualified (`openslo.com/v1`, dual-accepted with the
  upstream bare `openslo/v1` on write, canonicalized on read), only a subset
  is supported (one objective, inline `ratioMetric`, `Occurrences`, rolling
  `4w`/`28d`), a table lists what is rejected with a 400, `alertPolicies` is
  called out as accepted-but-silently-ignored, and queries must be bare
  PromQL vector selectors. Verified field-by-field against `SloDefinition`
  and `SloSpec` in the API OpenAPI spec.
- Note that the same document body is what the operator's `openslo.com/v1`
  `SLO` CRD accepts (group/kind/version and every spec field the example
  uses match; `alertPolicies` and `indicatorRef` are deliberately absent),
  phrased conservatively because that CRD is still unmerged
  (dash0hq/dash0-operator#1247).
- Expand "SLOs" to "service level objectives (SLOs)" on first use in the
  SKILL.md frontmatter description.
- Reframe the origin-vs-ID prose in SKILL.md and docs/commands.md. Origin
  being the upsert key is no longer an exception list: it is the rule for
  the API-managed kinds (notification channels, spam filters, SLOs, teams),
  while views, check rules, recording rules, and synthetic checks are the
  kinds where the server treats origin as provenance and the CLI strips it.
  Dashboards are called out separately, since their `dash0Extensions.origin`
  is deprecated in favor of `dash0Extensions.id`.
- Align the skill's SLO example with test/roundtrip/fixtures/slo.yaml so
  both use the Dash0 synthetic-check metrics (`dash0.synthetic_check.runs`
  split on `dash0_synthetic_check_outcome="Healthy"`) instead of
  `http_server_request_duration_seconds_count`.

Deduplicating the two published skill copies is tracked separately in #236.
@nicolastakashi
nicolastakashi marked this pull request as ready for review August 4, 2026 19:50
mmanciop
mmanciop reviewed Aug 5, 2026
View reviewed changes
Comment thread internal/apply/apply.go Outdated
mmanciop
mmanciop reviewed Aug 5, 2026
View reviewed changes
Comment thread internal/asset/diff_test.go Outdated
mmanciop
mmanciop reviewed Aug 5, 2026
View reviewed changes
Comment thread internal/skill/content/references/apply.md Outdated
mmanciop
mmanciop reviewed Aug 5, 2026
View reviewed changes
Comment thread internal/testutil/fixtures/slos/create_success.json
@nicolastakashi nicolastakashi mentioned this pull request Aug 5, 2026
Merged
nicolastakashi added a commit that referenced this pull request Aug 6, 2026
@nicolastakashi
fix(slos): address PR #207 review feedback
dc7cafc 
- delete: add IsAlreadyDeleted idempotency branch so `slos delete --force`
  on an already-deleted SLO exits 0 (matches the 8 other delete commands,
  CHANGELOG 1.16.2 / #217); cover with an integration test.
- update: StripSLOServerFields before the dry-run diff and PUT so
  `get -o yaml | update -f` no longer sends server-managed labels/timestamps
  or shows a spurious diff (mirrors views/update.go).
- asset/ImportSLO: mirror ImportTeam upsert-key selection — prefer
  dash0.com/id, fall back to dash0.com/origin (read before strip), POST only
  when neither is present, so an origin-only document upserts via PUT instead
  of duplicating on every apply (#227 shape).
- list: replace interface{} with any on the column closures (matches
  views/list.go).
- integration tests: assert the update wire body strips dash0.com/version,
  dash0.com/origin, and created-at; add create/update --dry-run tests; add a
  second element to list_success.json and exercise --limit truncation.
- roundtrip: add test_apply_slo_idempotency.sh (double-apply, no duplicate /
  stable id) and register it in run_all.sh.
- docs: add teams to the command-group enumerations in commands.md and
  cli-naming-conventions.md and adjust the count wording.
@nicolastakashi
test(roundtrip): anchor SLO idempotency action-word matches
dd53720 
The second apply correctly updates in place (same origin, only updated-at
changes), but the check `grep -q "created"` matched the
`dash0.com/created-at:` line in the update diff and reported a false
duplicate. Anchor the created/deleted matches to end-of-line so they hit
the action word only, not the *-at annotations. The exactly-one-per-origin
count assertion remains the authoritative duplicate check.
@nicolastakashi
fix(slos): mirror Teams upsert routing in ImportSLO + add regression …
0939e5e 
…tests

ImportSLO now faithfully mirrors ImportTeam's canonical upsert logic:

- Origin-first precedence. When dash0.com/origin is present it wins over
  dash0.com/id and is used as the upsert key (PUT is create-or-replace).
  Previously ImportSLO was id-first, which is the opposite of Teams.
- Preflight-driven id path with a genuine-404 POST fallback. On the id path
  the preflight GET routes the write: a 200 upserts via PUT, a 404 falls
  through to POST (cross-environment apply, where the id belongs to another
  org), and any other error is surfaced instead of silently POSTing — so a
  transient 5xx/auth/network blip never spawns a duplicate. Previously
  ImportSLO always PUT whenever any key was present.

Added the upsert-routing integration tests the SLO suite was missing,
driven through the real `dash0 slos create -f <file>` path and asserting
the HTTP method + path the mock received via server.Requests() (not
LastRequest, since the preflight GET precedes the write):

- TestCreateSLOFromFile_UpsertByOrigin
- TestCreateSLOFromFile_UpsertByID
- TestCreateSLOFromFile_UpsertByID_FallsBackToPOSTWhenNotFound
- TestCreateSLOFromFile_OriginWinsOverID
@nicolastakashi
test(slos): assert real idempotency on re-apply, not just no duplicate
5583525 
Michele is right that the previous idempotency test proved only that a second
apply created no duplicate — nothing in it tested idempotency. A re-apply of an
unchanged document should be a no-op, and `apply` expresses that by rendering an
empty before/after diff and printing "no changes". That is the contract every
sibling asset already meets (see TestApply_Dashboard_* and friends), so SLO now
meets it too. No new ImportAction state was introduced: ImportSLO still reports
`updated` for an upsert, and the "no changes" text comes from the diff being
empty.

The reason it was not empty: asset.marshalForDiff normalizes each asset through
its Strip*ServerFields helper before diffing, and *dash0api.SloDefinition was the
only asset type with no case in that switch. It therefore fell through to the
default branch and neither side was stripped. Because the server bumps
dash0.com/updated-at and dash0.com/version on every PUT — even for a
byte-identical body — every re-apply rendered a spurious two-line
version/updated-at diff. CI showed exactly that diff and nothing else. Adding the
missing case fixes `apply` and `slos update` together and changes nothing for any
other asset.

Assertions added:

- test_apply_slo_idempotency.sh: the second apply must state "no changes", and
  must report neither `created` nor `updated`. Both action-word greps stay
  anchored to end-of-line, because "dash0.com/updated-at" appears in diff text
  and would false-match an unanchored grep.
- test_slo_roundtrip.sh: re-applying the just-exported YAML must state
  "no changes", which is what closes the round-trip.
- TestApply_SLO_SecondApplyReportsNoChanges (internal/apply) reproduces the
  server behavior from the CI log against the mock server: the PUT response is
  the GET response with only version and updated-at bumped. It also pins that the
  second apply still upserts by origin and never POSTs.
- TestUpdateSLO_UnchangedDocumentReportsNoChanges (internal/slos) covers the same
  no-op through `slos update`.
- TestPrintDiff_SLO_ServerBumpedMetadataIsNotAChange and
  TestPrintDiff_SLO_RealSpecChangeStillDiffs unit-pin the normalization and guard
  that it does not swallow a genuine spec change.

These run on the mock server, so the contract is pinned without the live API. That
matters because only the `static` roundtrip shard actually executes: the oauth
matrix jobs are skipped-as-pass while DASH0_OAUTH_REFRESH_TOKEN is unset.

Also fixes a read-lag flake that failed the static roundtrip at Step 6 with
"found 0": `slos get "$ORIGIN"` succeeded while `slos list --all` still returned 0
for the same origin under a second after the restore PUT, and two unrelated
static jobs failed in the same minute with ingestion-lag symptoms. The
list-count assertions in both scripts now poll with backoff, as the log and span
roundtrips already do, instead of asserting once.

Finally, the roundtrip fixture's SLI now uses the Dash0 synthetic-check counter
dash0.synthetic_check.runs split on dash0.synthetic_check.outcome, which is a more
instructive availability example than a generic HTTP-duration counter. Metric name
and the "Healthy" outcome value were confirmed against the live metric catalog,
not guessed. Both queries remain bare vector selectors, as the API requires, and
counter: true still holds because the metric is a Sum.
@nicolastakashi
fix(slos): correct the documented upsert key, drop foreign id from PO…
f9adca7 
…ST body

Addresses six pre-merge review findings on #207.

1. The docs taught `dash0.com/id` as the SLO upsert key while `ImportSLO` is
   origin-first, and SLO ids are server-assigned (`slo_<ulid>`) so the pinned
   example id could never exist. The identifier table, the SLO YAML example,
   the `apply` section, and the "notification channels and spam filters are the
   two exceptions" sentence (in both docs/commands.md and the skill's SKILL.md)
   now say origin — modeled on the existing `Dash0Team` row, which describes
   the same routing.
2. `apply --dry-run` printed no identifier for an origin-only SLO document (the
   recommended form). Added the origin fallback both origin-keyed baselines
   already have.
3. `slos get <origin>` built the deep link and the id label from the raw
   argument, producing a URL that does not resolve. Both are now read off the
   response, mirroring `notificationchannels/get.go`.
4. `StripSLOServerFields` does not clear `dash0.com/id`, so the cross-org POST
   fallback (id branch, preflight 404) sent the source org's id. `ImportSLO`
   now calls the previously-unused `dash0api.ClearSLOID` on that path, and the
   regression test asserts the POST body carries no id. Also deleted the
   `ImportSLO` doc comment sentence claiming the strip clears the id.
5. There was no `slos` topic in the Agent Skill bundle: added the
   `ManifestEntry`, the generator `topicSpec`, the SKILL.md topic-index row,
   and regenerated, so `dash0 skill show slos` resolves.
6. `slos list -o wide`/`-o csv` were missing the `ORIGIN` column the shared
   format contract promises, and `slos get` printed no `Origin:` line. Since
   origin is the SLO upsert key, it is the field users script against.
@nicolastakashi
docs(slos): document the Dash0/OpenSLO v1 differences in the embedded…
9baaef4 
… skill

Second review round on #207, all on the embedded Agent Skill content.

- Add an "SLO documents and OpenSLO v1 compatibility" section to
  docs/commands.md and wire it into the `slos` skill topic. It replaces the
  flatly wrong "SLO documents use the OpenSLO v1 format" claim: the
  `apiVersion` is domain-qualified (`openslo.com/v1`, dual-accepted with the
  upstream bare `openslo/v1` on write, canonicalized on read), only a subset
  is supported (one objective, inline `ratioMetric`, `Occurrences`, rolling
  `4w`/`28d`), a table lists what is rejected with a 400, `alertPolicies` is
  called out as accepted-but-silently-ignored, and queries must be bare
  PromQL vector selectors. Verified field-by-field against `SloDefinition`
  and `SloSpec` in the API OpenAPI spec.
- Note that the same document body is what the operator's `openslo.com/v1`
  `SLO` CRD accepts (group/kind/version and every spec field the example
  uses match; `alertPolicies` and `indicatorRef` are deliberately absent),
  phrased conservatively because that CRD is still unmerged
  (dash0hq/dash0-operator#1247).
- Expand "SLOs" to "service level objectives (SLOs)" on first use in the
  SKILL.md frontmatter description.
- Reframe the origin-vs-ID prose in SKILL.md and docs/commands.md. Origin
  being the upsert key is no longer an exception list: it is the rule for
  the API-managed kinds (notification channels, spam filters, SLOs, teams),
  while views, check rules, recording rules, and synthetic checks are the
  kinds where the server treats origin as provenance and the CLI strips it.
  Dashboards are called out separately, since their `dash0Extensions.origin`
  is deprecated in favor of `dash0Extensions.id`.
- Align the skill's SLO example with test/roundtrip/fixtures/slo.yaml so
  both use the Dash0 synthetic-check metrics (`dash0.synthetic_check.runs`
  split on `dash0_synthetic_check_outcome="Healthy"`) instead of
  `http_server_request_duration_seconds_count`.

Deduplicating the two published skill copies is tracked separately in #236.
@nicolastakashi
fix(slos): rely on StripSLOServerFields to clear dash0.com/id
32b68fb 
Upgrade to dash0-api-client-go v1.18.2, which clears dash0.com/id in
StripSLOServerFields (dash0hq/dash0-api-client-go#24). This removes the
local ClearSLOID workarounds that stood in for the missing behaviour.

Also adds the SLO case to marshalForDiff. SLO was the only asset type
absent from the type switch, so neither side of a diff was normalized and
re-applying an unchanged document rendered a spurious hunk from the
server-bumped dash0.com/updated-at and dash0.com/version instead of
reporting "no changes".

slos update -f now falls back to dash0.com/origin when no id is present,
matching notification channels and teams. SLO ids are server-assigned, so
a hand-authored document can only pin an origin — the flow the command's
own help advertises was previously impossible.
@nicolastakashi
test(slos): use a realistic Terraform origin in fixtures
467e77b 
Three fixtures claimed to represent a Terraform-managed SLO but carried
`dash0.com/origin: terraform`, which is not a shape any tool produces. The
provider mints `"tf_" + uuid.New().String()` (slo_resource.go), so the
fixtures now use a `tf_<uuid>` value.

The all-zeros `dash0.com/id` is left as-is: it is the repo-wide placeholder
convention shared with the spam-filter tests, and reads as a deliberate
placeholder rather than a wrong format.
@nicolastakashi
docs(slos): drop the forward reference to Dash0Team
4b9cac9 
The SLO paragraph in `apply` defined its upsert-key selection as "the same as
Dash0Team", but Dash0Team is documented after SLO, so a reader going top-down
met the rule before its definition. The four lines that follow already spell
the selection out in full, so the cross-reference only added the dependency.
It now opens with the constraint that explains why origin is the key.

Also signposts the direction of the OpenSLO-compatibility pointer, which
refers to a section roughly 200 lines further down.

Edited docs/commands.md (the source) and regenerated the skill bundle; the
reference files are generated and carry a do-not-edit-by-hand header.
@nicolastakashi
test(slos): cover the packed spec.service identity
6f61e63 
spec.service is not a plain service name: the API splits on the first `/`, so
`payments/eu` resolves to service.namespace=payments and service.name=eu, and a
leading `/` escapes a name that itself contains a slash (`/payments/eu` is the
literal name `payments/eu`). Every fixture used a bare name, so none of that
was exercised.

The CLI is a passthrough here, and that is the property worth pinning: a
well-meaning "normalize the path" change would silently retarget an SLO at a
different service. The new table test asserts all three forms reach the wire
byte-for-byte. Verified it fails when the value is mangled, not just that it
passes today.

Also documents the format, which was absent from the CLI docs entirely even
though the field is impossible to use correctly without it.
@nicolastakashi
refactor(slos): drop the explanatory comment on the origin fallback
26fdfb5 
The comment claimed the fallback was about the upsert key, but parseDocumentHeader
feeds display output only: doc.id reaches nothing but formatNameAndId. Upsert
routing happens in ImportSLO, which keys on originOrId. The fallback now reads
like its notification-channel and team siblings.
@nicolastakashi
docs(slos): document delete-by-origin and the null timeWindow on read
f46b7d6 
`DELETE /api/slos/{originOrId}` has always accepted an origin, and the PR's own
roundtrip test deletes by one, but the cobra help claimed ID only. The help now
matches `slos get`, whose long description already documented both.

Also records that the omitted-timeWindow default is not observable on read: the
API returns `timeWindow: null` rather than an explicit 28d window, so a stored
document cannot distinguish "defaulted" from "unset". The existing sentence
described the behaviour correctly but implied a populated value.

Amended the existing changelog entry instead of adding one, since the feature is
unreleased and its subtext already claimed origin support for `get` alone.
@nicolastakashi
refactor(slos): use GetSLOOrigin instead of reading the label by hand
e1dab4b 
Upgrades to dash0-api-client-go v1.19.0, which adds GetSLOOrigin
(dash0hq/dash0-api-client-go#28), and replaces the four open-coded reads of
the dash0.com/origin label with it:

- internal/asset/slo.go (ImportSLO upsert routing)
- internal/slos/update.go (`update -f` origin fallback)
- internal/apply/apply.go (parseDocumentHeader)
- internal/slos/slos_cmd.go (local sloOrigin helper, now deleted; list and get
  call the API-client helper directly)

Each carried its own nil-check for Labels and the pointer, and two carried
comments explaining that no such helper existed. Both comments are gone with
the code they described.

No behaviour change: GetSLOOrigin has the same nil-handling as the code it
replaces, matching GetTeamOrigin and GetNotificationChannelOrigin.
@nicolastakashi
fix(apply): delete SLOs removed since the --since ref
8b2520b 
asset.IsValidKind accepts "slo", so a removed SLO document was recorded
as a deletion candidate, but deleteAssetByKindAndIdentifier had no case
for it. The run hit the default branch and failed with "unsupported kind
for deletion: slo" only after the surviving documents had already been
created or updated, leaving the sync half-applied.
@nicolastakashi
fix(apply): identify SLOs by origin, not just id
4a88742 
ExtractIdentifier read only dash0.com/id for SLOs, while ImportSLO
upserts by dash0.com/origin first. SLO ids are server-assigned, so the
recommended hand-authored form carries origin alone -- such a document
extracted no identifier at all, so any --since run in which it was
deleted hard-failed up front with the NoIdentifier error, before
creating, updating, or deleting anything.

The --since deletion test moves to that origin-only form, which also
corrects its now-stale assertion: a document carrying both labels
deletes by origin, not by id.
@nicolastakashi
fix(slos): accept the origin as update's positional argument
026eeb5 
The consistency check compared the argument against only the preferred
identifier -- the id whenever the document carries one. An exported SLO
(`dash0 slos get -o yaml`) carries both labels, so `slos update <origin>
-f <file>` was rejected before any API call, even though origin is the
key the docs recommend and both `get <origin>` and `delete <origin>`
accept it.

The two labels are alternative handles on the same SLO, so the argument
is now checked against each independently, mirroring `teams update`.
@nicolastakashi
fix(slos): surface a non-404 preflight error on the origin route
4110f2f 
ImportSLO's origin branch discarded every GetSLO error, including 5xx,
auth failures, and network blips. The route is PUT either way, so the
preflight only decides "created" vs "updated" -- but on a transient
failure the PUT still replaced an existing SLO while the run reported it
as created with no diff, claiming a write it had not made.

The error is now classified the way the sibling id branch already
classifies it: a genuine 404 falls through to the PUT, anything else is
returned.
@nicolastakashi
fix(slos): name the SLO the same way in update --dry-run
5fcd67b 
The dry-run path titled its output with metadata.name while the real run
used dash0api.GetSLOName, which prefers the dash0.com/display-name
annotation. The same operation on the same document therefore printed
two different SLO names depending on --dry-run.
@nicolastakashi
docs(commands): correct which asset CRUD groups are dataset-scoped
05a3986 
Of the nine command groups sharing the CRUD subcommands, seven are
dataset-scoped: notification-channels is organization-level, as the
asset-type table and the Organizational commands section both already
state. The same sentence also claimed `teams` has no `apply`, but
Dash0Team is a supported kind and this document describes its upsert
behavior in the apply section.
@nicolastakashi
fix(slos): let update's positional argument be the id an origin-pin…
75d0404 
…ned file omits

The guard rejected any argument not matching a label in the file, so with
a document pinning only dash0.com/origin, passing the SLO's real id --
the form the command's own Example shows -- hard-errored before any API
call. SLO ids are server-assigned, so an origin-pinned document says
nothing about the id, and there is no offline way to tell a valid
counterpart from a typo.

The check now applies only when the document pins both handles, where the
argument does have to be one of them. A wrong value against a one-label
file surfaces as a clean "SLO not found" from the preflight GET.
@nicolastakashi
docs(slos): correct the id-only POST fallback's idempotency claim
e045c30 
The asset-identifier table said the fallback keeps cross-org apply
idempotent. It does not: the server assigns an ID the document never
carries, so every further apply preflights the same unknown ID, 404s, and
POSTs another SLO. Names the duplication and points at dash0.com/origin
for any SLO applied more than once.

internal/skill/content/references/apply.md is the regenerated bundle
(make skill-bundle), per docs/agent-skill-maintenance.md.
@nicolastakashi
fix(apply): warn when --since deletes an SLO identified by id alone
5ad5972 
The analogue of the existing spam-filter warning. SLO ids are
server-assigned, so an id-only document first applied to an organization
without that id took ImportSLO's POST fallback, and the live SLO sits at
an id the document never learned. Deleting by the id recorded in git
history then 404s (hard-failing without --force) or counts as
already-deleted (with --force), orphaning the real SLO either way.
@nicolastakashi
docs(slos): correct ImportSLO's id-only fallback comment
f3ca4f3 
The doc comment said falling back to POST "keeps `apply` idempotent",
contradicting docs/commands.md, which is right: the server assigns an id
the document never captures, so the next apply preflights the same
unknown id, 404s, and POSTs another SLO. e045c30 corrected the prose and
should have taken this comment with it -- and this is the one a future
maintainer reads first.
@nicolastakashi
test(slos): assert auth and user-agent headers on the error paths
68984e2 
TestListSLOs_AuthError and TestGetSLO_NotFound omitted the
testutil.RequireHeaders validator docs/testing.md calls for, so neither
checked that the CLI sent its credentials on the request under test.

Adding the validator alone would not have been enough: the mock serves a
failed validation as a plain 400, so a bare require.Error passes even
with the header missing. Each test now also asserts the error text, which
is what makes the validator load-bearing.
@nicolastakashi
docs(slos): add SLOs to the asset-kind enumerations
b90c914 
CLAUDE.md's origin-vs-ID list covered nine kinds but not SLOs, and
README.md's asset-type list stopped at spam-filters -- the latter
required by docs/adding-commands.md step 10 for a new command group.
docs/about.md carried the same omission in its asset list.

The CLAUDE.md entry mirrors the Teams wording and adds what Teams does
not need: the id-only POST fallback is a one-way create, so a repeated
apply duplicates.
@nicolastakashi
test(slos): assert the preflight GET path and the id-path 5xx branch
44759ad 
sloIDPattern is `^/api/slos/[^/]+$`, so the mock answers a preflight
against any single segment with the same fixture. The suite had
assertPUTPath/assertPOSTPath but no GET equivalent, so a regression that
preflighted the wrong identifier left every assertion green -- confirmed
by pointing ImportSLO's origin preflight at a hardcoded wrong value.
assertGETPath now pins it in all four routing tests.

The id-keyed branch's non-404 handling also had no test, while the
identical origin-keyed branch did. Its mirror asserts neither POST nor
PUT runs on an inconclusive preflight, which is the duplicate that branch
exists to prevent.
@nicolastakashi
test(slos): stop the roundtrip delete check reading failure as success
edd3d64 
The check was `! jq -e '.[] | select(...)'`, and the negation caught
every nonzero jq exit, not just "no match" -- malformed output took the
"SLO is gone" branch exactly like a genuine absence, so the gate could
report PASS without ever establishing the SLO was deleted. An object
payload slipped through the same way, since `.[]` iterates an object's
values rather than erroring.

Now validates the payload is an array, then counts matches separately.
@nicolastakashi
nicolastakashi merged commit 4bd63e6 into main Sep 11, 2026
90 checks passed
@nicolastakashi
nicolastakashi deleted the slos branch September 11, 2026 14:34
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mmanciop mmanciop mmanciop approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add dash0 slos command group (SLO IaC support)

2 participants

@nicolastakashi @mmanciop