Skip to content

feat: add create-kafka-topic skill#157

Closed
evanh wants to merge 9 commits into
mainfrom
evanh/feat/create-kafka-topic-skill
Closed

feat: add create-kafka-topic skill#157
evanh wants to merge 9 commits into
mainfrom
evanh/feat/create-kafka-topic-skill

Conversation

@evanh

@evanh evanh commented Jun 22, 2026

Copy link
Copy Markdown
Member

Adds a create-kafka-topic skill that provisions a new Kafka topic across the four repos that must all agree for a Sentry topic to exist and deploy:

  • sentry-kafka-schemas — schema definition + CODEOWNERS entry
  • ops — default partitions, per-region overrides, all_topics.yaml, schemas requirement bump
  • sentryTopic enum + KAFKA_TOPIC_TO_CLUSTER, schemas dependency bump
  • getsentry — cell-silo topic→cluster mapping

It opens one PR per repo and reports the four PR links annotated with their merge-order dependencies (the schemas PR must merge + release before the ops/sentry version bumps).

The steps and file formats were reconciled against the real taskworker-ingest-push topic PRs (sentry-kafka-schemas#484, ops#20917, sentry#117135, getsentry#20495). Per how ops works, CI regenerates the materialized/k8s config (make materialize), so the skill only edits source-of-truth files.

Includes the required SPEC.md and registration in README.md, .claude/settings.json, and the claude-settings-audit allowlist.

🤖 Generated with Claude Code

Adds a skill that provisions a new Kafka topic across the four repos that
must agree for a Sentry topic to exist: sentry-kafka-schemas (schema +
CODEOWNERS), ops (partitions, per-region overrides, all_topics), sentry
(Topic enum + cluster mapping), and getsentry (cellsilo cluster mapping).
Opens one PR per repo and reports the four PR links with their merge-order
dependencies.

Reconciled against the taskworker-ingest-push topic PRs (sentry-kafka-schemas#484,
ops#20917, sentry#117135, getsentry#20495). CI handles ops materialized config,
so the skill only edits source-of-truth files.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Comment thread skills/create-kafka-topic/SKILL.md
Comment thread skills/create-kafka-topic/SKILL.md Outdated
Testing the skill surfaced two issues:

- getsentry no longer needs a per-topic change. It loads KAFKA_TOPIC_TO_CLUSTER
  at runtime from the topicctl-generated YAML mounted by ops (getsentry#20512,
  #20661), so the cellsilo.py edit is obsolete. The skill is now 3 repos, not 4.
- Make the "which regions to enable" prompt an explicit blocking gate so it is
  never inferred from a reference topic.

Updates SKILL.md, SPEC.md, and the README description accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Comment thread skills/create-kafka-topic/SKILL.md
Comment thread skills/create-kafka-topic/SKILL.md Outdated
The team used for the sentry-kafka-schemas CODEOWNERS entry is now also added
as a reviewer (--reviewer) on all three PRs.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Comment thread skills/create-kafka-topic/SKILL.md Outdated
evanh and others added 2 commits June 22, 2026 15:40
New ops regional override files must have a counterpart in the
cookiecutter-region template (disabled: true) so newly-created regions get the
topic too; Warden's cookiecutter-region-backport check flags the omission.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Instead of deferring the sentry-kafka-schemas pin bump, the ops and sentry PRs
now bump to the anticipated next version (latest released patch + 1) so the
required change is visible in the diff. The PR body flags the version as
anticipated, and for sentry notes that uv.lock's resolved entry must be
regenerated with `uv lock` once the release publishes.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@linear-code

linear-code Bot commented Jun 23, 2026

Copy link
Copy Markdown

STREAM-1224

- Enabled regions outside the sibling-derived candidate set are now always
  given an override file (file set = candidate ∪ user-enabled regions).
- Use the full shared_config/kafka/topics/regional_overrides/... path for
  per-region override files so they're staged by git add.
- Clarify the Topic enum member is SCREAMING_SNAKE_CASE while the value is the
  kebab-case topic name, with a concrete example.
- Note the KAFKA_TOPIC_TO_CLUSTER value is "default" today, but to match a
  sibling family if that ever differs.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@evanh

evanh commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

Addressed the review comments in 6daf901:

  • Enabled regions omit override files (Bugbot) — fixed. The "file set" is now explicitly the sibling-derived candidate set ∪ every region the user enabled, so an enabled region always gets an override file even if the sibling topic didn't define it.
  • Ambiguous / missing regional-override path (Bugbot + Sentry) — fixed. Step 3.2 now uses the full shared_config/kafka/topics/regional_overrides/<region>/<topic_name>.yaml path, matching git add shared_config/kafka/topics/.
  • Enum template conflates names (Bugbot) — fixed. Clarified the Topic member is SCREAMING_SNAKE_CASE (hyphens→underscores, uppercased) while the value is the kebab-case name, with a concrete TASKWORKER_LAUNCHPAD_PUSH = "taskworker-launchpad-push" example.
  • Hardcoded default cluster mapping (Bugbot) — not a current bug: all 97 KAFKA_TOPIC_TO_CLUSTER entries map to "default". Softened the wording anyway to say match a sibling's value if a family ever uses a non-default cluster.

@untitaker untitaker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think the skill should ask a few clarifying questions about whether this really needs a schema change. if this is just another taskbroker or outcomes topic then a few steps can be skipped

@evanh

evanh commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

i think the skill should ask a few clarifying questions about whether this really needs a schema change. if this is just another taskbroker or outcomes topic then a few steps can be skipped

Which steps can be skipped?

@untitaker

untitaker commented Jun 23, 2026

Copy link
Copy Markdown
Member

see getsentry/sentry-kafka-schemas#490 (review) -- this is not how we create taskworker or outcomes topics. we already have a schema definition for those, and sentry-kafka-schemas should not contain infra-specific topics that are not present in self-hosted or elsewhere

EDIT: after discussing in slack we did create taskworker topics like that. I do think we should still add a caveat about outcomes

Add a public/private decision up front:

- Public (available to self-hosted): unchanged three-PR flow
  (sentry-kafka-schemas + ops + sentry).
- Private (internal regions only): a single ops PR. The per-region overrides
  carry `cluster:` + `override_topic: <public_topic>` to inherit the public
  topic's topic_creation_config. No sentry-kafka-schemas change, no sentry
  change, no all_topics.yaml entry, and no version bump.

Steps 1 (schemas) and 4 (sentry) are marked public-only; Step 0 asks the
public/private question and, for private, the override topic; Step 3 and the
report handle both paths. SPEC and README updated.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Comment thread skills/create-kafka-topic/SKILL.md Outdated
Comment thread skills/create-kafka-topic/SKILL.md Outdated
Comment thread skills/create-kafka-topic/SKILL.md
Comment thread skills/create-kafka-topic/SKILL.md Outdated
- Collect a reference/sibling topic in Step 0 (it was used by Steps 2-4 but
  never gathered).
- Broaden the collision check to all locations (sentry-kafka-schemas, ops
  default + regional overrides, and the sentry Topic enum / cluster map),
  regardless of public/private (was schemas-only for public).
- Add a clarifying question + caveat that not every public topic needs a new
  schema entry; outcomes topics in particular reuse an existing schema and
  must not be added to sentry-kafka-schemas (per reviewer feedback).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@evanh

evanh commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Addressed the latest review feedback in aefd099:

  • @untitaker's note ("ask whether this really needs a schema change… caveat about outcomes") — Step 0 now asks, for public topics, whether a new schema definition is actually needed, and Step 1 carries a caveat: outcomes topics reuse an existing schema and must not be added to sentry-kafka-schemas (infra-specific variants don't belong in the registry self-hosted consumes). taskworker topics keep the new-topics/<name>.yaml flow, as confirmed in the thread.
  • Reference sibling topic never collected (Bugbot) — Step 0 now explicitly collects a reference/sibling topic; it's what drives region discovery and all_topics.yaml / Topic enum placement.
  • Public collision check too narrow (Bugbot + Sentry, HIGH) — the collision check now covers all locations regardless of public/private: sentry-kafka-schemas/topics/, the ops default + regional-override paths, and the sentry Topic enum / KAFKA_TOPIC_TO_CLUSTER.

The remaining Bugbot comments (enum member naming, regional-override path prefix, hardcoded cluster, enabled-region file set) were already fixed in earlier commits (2739a04, 6daf901) and re-anchored to the latest commit as lines shifted.

Comment thread skills/create-kafka-topic/SKILL.md
Comment thread skills/create-kafka-topic/SKILL.md Outdated
Comment thread skills/create-kafka-topic/SKILL.md Outdated
Two inconsistencies introduced by the public/private split:

- Outcomes-style public topics skip Step 1 (no new schema), but the version
  bumps (Steps 3/4) and the report (Step 5) still assumed a schemas release.
  Gate the bumps and add a two-PR report variant (ops + sentry, no bump) for
  the reuse-existing-schema case.
- The collision check referenced $SCHEMAS/$SENTRY for private topics, which
  only locate $OPS. Scope the collision check to the repos each path locates
  (ops always; schemas + sentry for public).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@evanh

evanh commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Addressed the two new findings in 26adc81 and resolved the threads:

  • Outcomes path bumps schemas — the version bumps (Steps 3.5 / 4.3) are now gated on Step 1 actually adding a new schema, and Step 5 has a two-PR report variant (ops + sentry, no bump, no merge-first ordering) for public topics that reuse an existing schema (outcomes). Private remains one ops PR.
  • Private path skips collision repos / omits $SENTRY — the collision check is now scoped to the repos each path actually locates: $OPS always (which also covers deployed public topics' ops files), plus $SCHEMAS and $SENTRY only on the public path where they're checked out.

The earlier Bugbot comments (enum naming, override path, hardcoded cluster, enabled-region file set) were already resolved.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 26adc81. Configure here.

- Private topics: ops PR only — no `sentry-kafka-schemas` change, no `sentry` change, no `all_topics.yaml` entry, no version bump. The `override_topic` must be an existing public topic in `all_topics.yaml`.
- Insert enum / `all_topics.yaml` entries next to sibling topics (grouped by family), not alphabetically.
- Confirm the generated schema file with the user before opening the first PR (public).
- Sequencing (only when Step 1 adds a new schema): the schemas PR gates the dependency bumps in `sentry`/`ops`. When the topic reuses an existing schema (outcomes) or is private, there is no schemas release, no version bump, and no cross-PR ordering. The ops/sentry PRs still bump the pin to the anticipated next version (latest released patch + 1) so the required change is visible; the PR body must flag it as anticipated and, for sentry, note that `uv.lock` must be regenerated with `uv lock` once the release publishes. Private topics have no such dependency.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SPEC contradicts no version bump

High Severity

The Runtime Contract sequencing bullet first states there is no version bump when Step 1 is skipped (outcomes or private), then immediately instructs ops/sentry PRs to bump sentry-kafka-schemas to the anticipated next version. That reverses Step 3.5/4.3 and can produce spurious dependency bumps.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 26adc81. Configure here.

- Per-region override contents: `disabled: true` when not enabled. When enabled, `cluster:` (+ `partitions:` only if it differs from the default); **private topics also include `override_topic: <override_topic>`**.
- Private topics: ops PR only — no `sentry-kafka-schemas` change, no `sentry` change, no `all_topics.yaml` entry, no version bump. The `override_topic` must be an existing public topic in `all_topics.yaml`.
- Insert enum / `all_topics.yaml` entries next to sibling topics (grouped by family), not alphabetically.
- Confirm the generated schema file with the user before opening the first PR (public).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Schema confirm applies wrongly

Medium Severity

The non-negotiable constraint requires confirming the generated schema file before the first PR for any public topic. When Step 1 is skipped (outcomes), there is no schema file, yet the first PR is ops—this constraint does not fit that path.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 26adc81. Configure here.

Commit and open the PR (drop `python/requirements.txt` from the `git add` for private topics):

```bash
git add shared_config/kafka/topics/ cookiecutters/cookiecutter-region/ python/requirements.txt

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Commit omits schema-skip case

Medium Severity

Step 3’s commit note only says to drop python/requirements.txt from git add for private topics, not when Step 1 was skipped for public topics that reuse a schema. The sample commands always stage requirements.txt and, in Step 4, pyproject.toml/uv.lock, which fights the explicit “skip the bump” rules.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 26adc81. Configure here.

@evanh

evanh commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

This has become a mess of a PR. I'm going to close this and try again later.

@evanh evanh closed this Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants