Skip to content

docs(pivot): RFC-0013 §migration rescope — PR-I in-repo submodule#181

Merged
trilamsr merged 2 commits into
mainfrom
chore/pivot-pr-i-rescope
May 31, 2026
Merged

docs(pivot): RFC-0013 §migration rescope — PR-I in-repo submodule#181
trilamsr merged 2 commits into
mainfrom
chore/pivot-pr-i-rescope

Conversation

@trilamsr

@trilamsr trilamsr commented May 31, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Doc-only rescope of RFC-0013 §migration. Headline: PR-I is now an in-repo Go submodule at module/ (path github.com/tracecoreai/tracecore/module), not an external tracecoreai/tracecore-components repo. Captures three sequencing findings from adversarial scoping of PR-B / PR-F / PR-I before any of those PRs are attempted.

Root cause

Two separate root causes converged into one rescope:

  1. External-repo framing was unjustified. RFC originally specified tracecoreai/tracecore-components as a separate GitHub repo, but listed no operational driver (different maintainer set, different release cadence submodule tags can't solve, license incompatibility). Open-source project. One fork wins: one CI, one issue tracker, one DCO, one PR for cross-cutting changes. Go submodule tags give an independent version line; OCB gomod: + replaces: ./module for dev-loop resolves identical to an external repo. The split was aesthetic, not operational.

  2. RFC sequencing was under-scoped. Adversarial scoping of PR-B / PR-F / PR-I against the actual codebase surfaced three blockers the RFC didn't name:

    • PR-A2 (switch cmd/tracecore to OCB-generated main; delete legacy boot wiring) is the sequencing gate. PR-B2, PR-F, and PR-I all depend on the OCB-generated boot owning the receiver-registration path before they can delete or rewire internal/pipeline + internal/consumer.
    • PR-B is not one PR. It splits into PR-B1 (port nccl_fr off internal/selftelemetry + internal/runtime/lifecycle; doable now) + PR-B2 (port off internal/pipeline + internal/consumer; needs PR-A2).
    • PR-I is not one PR. It splits into PR-I.1 (move surviving nccl_fr into module/receiver/ncclfrreceiver/; needs PR-B2) + PR-I.2 (build rankjoinprocessor + patterndetectorprocessor as net-new processors; needs PR-K to sever the k8sevents import in internal/synthesis/patterns/).

Adversarial-review fixes

Round 1 (commit 4c793df) — 4 findings:

  • 🔴 v0.1.0 sequence renumbering bug (was 1,2,3,4,3,4,5,8,9,10; now 1,2,3,4,5,6,7,8,9,10).
  • 🟡 PR-B1 metric-namespace collision risk unaddressed → spelled out receiver-scoped meter strategy.
  • 🟡 Migration guide moat-row described post-PR-I state in present tense → switched to explicit "Currently in ... Will live in ...".
  • 🟡 CHANGELOG entry density (253-word paragraph) → split into headline + 3 bullets + file-list.

Round 2 (commit 182c641) — 2 findings:

  • 🔴 CHANGELOG.md:7 stale tracecoreai/tracecore-components ref in the Unreleased preamble (pre-existing text, inconsistent with the rescope headline). Rewritten to in-repo module/ submodule wording.
  • 🟡 PR-B1 metric naming used otelcol_receiver_nccl_fr_* (underscore). OCB upstream convention is otelcol_receiver_<name>_* where <name> is the receiver package name without underscores. Per project memory, the rename is nccl_fr → ncclfr. RFC metric naming aligned to otelcol_receiver_ncclfr_* so helpers emit the right shape from day one and no rename is needed at PR-I.

Release notes

NONE

Test plan

  • make verify green (golangci-lint, vet, mod-verify, fmt, tidy, doc-check, alert-check, chart-appversion-check, no-autoupdate-check) — both commits
  • Two rounds of independent adversarial review on the diff (not the plan), all 6 findings addressed
  • Cross-file consistency: RFC §1, §6, §migration §4 v0.2.0 row, §migration §OQ Bump the gh-actions group across 1 directory with 4 updates #2, §migration v0.1.0 sequence (1-10), §migration v0.2.0 PR-I body, docs/migration/v0.1-to-v0.2.md moat row, docs/rfcs/README.md new-receiver pointers, docs/STRATEGY.md moat-location framing, CHANGELOG.md Unreleased preamble — all aligned on in-repo submodule path
  • No code touched; no behavior change; OCB build path unchanged (no builder-config.yaml / Makefile / source-tree edits in diff)

Tri Lam added 2 commits May 30, 2026 20:05
Headline: PR-I is now an in-repo Go submodule at module/ (not external
tracecoreai/tracecore-components repo). Open-source project — one fork,
one CI, one issue tracker, one DCO. Go submodule tags give independent
version line; OCB gomod: + replaces: ./module resolves identical to
external repo.

Three sequencing findings from adversarial scoping of PR-B / PR-F / PR-I:

- New PR-A2 introduced as sequencing gate (switch cmd/tracecore to
  OCB-generated main; precondition for PR-B2 / PR-F / PR-I).
- PR-B splits into PR-B1 (port nccl_fr off internal/selftelemetry +
  internal/runtime/lifecycle; helpers travel as siblings using
  receiver-scoped MeterProvider — instrument names
  otelcol_receiver_nccl_fr_* cannot collide with pipeline-runtime's
  own otelcol_* namespace) + PR-B2 (port off internal/pipeline +
  internal/consumer after PR-A2 lands).
- PR-I subdivides into PR-I.1 (move nccl_fr into module/receiver/
  ncclfrreceiver/ after PR-B2) + PR-I.2 (build rankjoinprocessor +
  patterndetectorprocessor as net-new processors wrapping
  internal/synthesis/patterns/ after PR-K severs the k8sevents dep).

Adversarial-review fixes (independent reviewer on diff):
- RFC v0.1.0 sequence renumbered 1-10 (was 1,2,3,4,3,4,5,8,9,10).
- PR-B1 metric-namespace handling spelled out explicitly to prevent
  collision with upstream service/telemetry otelcol_* namespace.
- Migration guide moat-components row reworded with explicit
  current-state vs future-state ("currently in components/... will
  live in module/...").
- CHANGELOG entry split from 253-word paragraph into headline + 3
  bullets + file-list.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Signed-off-by: Tri Lam <tri@maydow.com>
Second adversarial review on PR #181 diff found:

- CHANGELOG.md:7 stale `tracecoreai/tracecore-components` ref in the
  Unreleased preamble (was pre-existing, not in commit 4c793df's
  change scope but inconsistent with the rescope headline below).
  Rewrite to in-repo `module/` submodule wording.
- RFC §migration PR-B1 metric naming used `otelcol_receiver_nccl_fr_*`
  with the underscore. OCB upstream convention is
  `otelcol_receiver_<name>_*` where <name> is the receiver package
  name without underscores. Per project memory the rename is
  `nccl_fr → ncclfr`; align the RFC metric naming to
  `otelcol_receiver_ncclfr_*` so the helpers emit the right shape
  from day one and no rename is needed at PR-I.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Signed-off-by: Tri Lam <tri@maydow.com>
@trilamsr trilamsr enabled auto-merge (squash) May 31, 2026 03:10
@trilamsr trilamsr merged commit 82eee3e into main May 31, 2026
12 checks passed
@trilamsr trilamsr deleted the chore/pivot-pr-i-rescope branch May 31, 2026 03:14
trilamsr added a commit that referenced this pull request May 31, 2026
## Summary

PR-H sliver per [RFC-0013
§migration](docs/rfcs/0013-distro-first-pivot.md#migration--rollout).
Sweeps `PRINCIPLES.md` + `CONTRIBUTING.md` for distribution-first pivot
drift. Net -128 lines.

## Root cause

Two unrelated drift accumulations, both fixed at source:

1. **PRINCIPLES.md §2 example was factually wrong.** The text "we
deferred GoReleaser, SBOM signing, eBPF integration, the Helm chart, the
OTel `pdata` import — none has cost us" predates the work that landed
since: PR-C (#174) shipped the goreleaser + SBOM + cosign stack; M5b
shipped the Helm chart; OCB adoption (#171 PR-A) pulls `pdata` in via
upstream. The illustrative example for "default to *not* adding" was
listing items we *did* add. Deleted the example; the principle's first
paragraph + bullet list above it carry the message without the
contradiction.

2. **CONTRIBUTING.md carried two layers of pivot drift.**
- **External-repo references stale post-#181.** Three mentions of
`tracecoreai/tracecore-components` (separate-repo framing) survived the
PR #181 rescope to in-repo `module/` Go submodule. Updated to
`github.com/tracecoreai/tracecore/module` with layout
`module/receiver/<name>/`, `module/processor/<name>/`.
- **Adding-a-component tutorial contradicted RFC-0013 policy.** ~130
lines of tutorial taught contributors to add receivers under
`components/` referencing `clockreceiver` / `dcgm` / `kernelevents` as
canonical shapes — but RFC-0013 §6 forbids new in-tree components
("Nothing else is built in-house"), and all three canonical references
are queued for v0.1.0 or v0.2.0 deletion per §7. The routing block 100
lines above already declared the forbidding policy, so the tutorial was
a live-policy contradiction with deleted-receiver examples. Replaced
with a ~10-line routing block covering the three actual branches
(upstream first / moat → `module/` / RFC for fifth scope) plus a
one-liner on the surviving factory shape. This is also why the four
explicit `clockreceiver` references (lines 139/142/149/152 pre-edit)
were resolved by deletion-of-containing-section rather than name-swap:
no candidate survivor exists (`dcgm` deletes v0.1.0,
`clockreceiver`/`kernelevents`/`k8sevents`/`containerstdout` delete
v0.2.0, `nccl_fr` is logs-only and moves out to `module/` in PR-I.1).

## Changes

- `PRINCIPLES.md` — delete one stale concrete-example paragraph (3 lines
net).
- `CONTRIBUTING.md` —
  - L21, L32: `tracecoreai/tracecore-components` → in-repo `module/`.
- L103-L235 (old): collapse "Adding a component" tutorial → 7-line
routing block.

## Scope-discipline notes

Per prompt scope-fence:
- `MILESTONES.md` / `CHANGELOG.md` deletion-table drift untouched
(historic / intentional per prompt).
- `docs/STRATEGY.md` untouched (out of scope per prompt).
- `PRINCIPLES.md` §16 already cites RFC-0013 — no additional edit
needed.
- `STYLE.md` lines 106/115 reference `clockreceiver` as a Go-import
example; left in place — code example for `package clockreceiver`
import-form is shape-illustrative (will rotate when source actually
deletes in PR-K). Flagged for the next sweep, not PR-H.
- `cmd/tracecore/`, `bench/install/`, `install/kubernetes/`,
`AGENTS.md`, `tools/components-gen/` all carry `clockreceiver`
references and are explicitly *out of PR-H scope* — those migrations
land in PR-K (test-fixture coordination) per CHANGELOG line 16.

## Test plan

- [x] `make check` — clean (golangci-lint 0 issues, vet clean,
mod-verify ok).
- [x] `make doc-check` — clean (505 markdown links resolve including new
`#upstream-contribution-policy` + `#rfc-process` cross-links;
banned-phrase lint clean across 109 markdown files).
- [x] `make ci` (via pre-push hook) — clean (actionlint + zizmor +
alert-check + chart-appversion-check + no-autoupdate-check + clean-tree
all pass).
- [x] `grep -n "clockreceiver\|tracecore-components" CONTRIBUTING.md
PRINCIPLES.md` — zero hits post-edit.
- [x] Anchor verification: `#upstream-contribution-policy` resolves to
`## Upstream contribution policy` (L15); `#rfc-process` resolves to `###
RFC process` (L39).

Doc-only diff; no Go test corpus changes.

Signed-off-by: Tri Lam <tri@maydow.com>
Co-authored-by: Tri Lam <tri@maydow.com>
trilamsr added a commit that referenced this pull request May 31, 2026
)

## Summary

Four amendments to `docs/rfcs/0013-distro-first-pivot.md` (plus a
one-line sweep across 6 companion docs) per the scope-review findings
staged before PR-I.1 / PR-K / PR-M code work begins. Pre-stages each
decision in the RFC so the autonomous code PRs don't escalate
mid-flight.

## Root cause

#181 (RFC-0013 PR-I in-repo submodule rescope) was incomplete:

1. Sweep missed 6 companion docs still pointing at the original-design
external `tracecoreai/tracecore-components` repo.
2. §7 listed 3 GitHub workflows for deletion that were already removed
pre-RFC and 1 issue template (`component-bug-dcgm.yml`) that was already
removed pre-RFC.
3. PR-K was a single 4-receiver-delete-plus-chart-migration mega-PR with
no decoupling of the `internal/synthesis/patterns/` k8sevents dep break,
which is on PR-I.2's critical path.
4. PR-I.1 conflated the `module/go.mod` scaffolding with the `git mv` +
package rename, blocking PR-I.1a from landing without PR-B2 even though
the scaffolding step has no nccl_fr dep.

Mid-flight discovery during merge cycle: merged commit #188
(`feat(pivot): PR-B2 — port dcgm off internal selftel + lifecycle`)
reused the `PR-B2` slug for a PR-B1-shape dcgm port (which is moot since
dcgm is deleted entirely in PR-F), creating a naming collision against
the canonical PR-B2 defined in the RFC — the nccl_fr
`internal/{pipeline,consumer,runtime/lifecycle}` → upstream port that
hard-gates the PR-I.1b `git mv`.

## Amendments

1. **§6/§7 sweep miss (Amendment 1)**: Remove surviving
`tracecoreai/tracecore-components` external-repo references across
`docs/getting-started.md`, `docs/followups/M11.md`,
`docs/followups/M19.md`, `docs/FOLLOWUPS.md`,
`docs/rfcs/0003-pipeline-runtime-and-component-contract.md`,
`AGENTS.md`. All re-pointed at `github.com/tracecoreai/tracecore/module`
per RFC-0013 §6. Verified zero surviving stale refs.
2. **§7 nonexistent workflow entries (Amendment 2)**: Collapse
`pyspy-integration.yml`, `python-publish.yml`,
`kernelevents-integration.yml` deletion rows into one row marked
"already removed pre-RFC". `component-bug-dcgm.yml` also already
removed. Only `component-bug-kernelevents.yml` survives for PR-K. §4
v0.3.0 row + PR-M slug cleaned for consistency.
3. **§migration PR-K sub-slice (Amendment 3)**:
- **PR-K.1** — sever `internal/synthesis/patterns/` from
`components/receivers/k8sevents` via local model types in
`internal/synthesis/patterns/model.go`. No deletions. **Unblocks
PR-I.2.**
- **PR-K.2** — delete
`components/receivers/{clockreceiver,kernelevents,k8sevents,containerstdout}`
+ migrate ~86 test fixtures + delete `tools/failure-inject/xidgen/` +
keep `tools/failure-inject/ncclhang/`.
- **PR-K.3** — chart cleanup: flip `containerstdout-on-values.yaml` to
filelog+container-stanza, delete `containerstdout-rbac.yaml`, delete
`.github/ISSUE_TEMPLATE/component-bug-kernelevents.yml`, ship
`NOTES.txt` deprecation + values-key removal.
4. **§migration PR-I sub-slice + PR-B2 promotion (Amendment 4)**:
- **PR-B2** reframed as hard gate for PR-I.1b: port
`components/receivers/nccl_fr` off
`internal/{pipeline,consumer,runtime/lifecycle}` to upstream
`go.opentelemetry.io/collector/{component,receiver,consumer,pipeline}`.
Slug-collision note added re: merged #188.
- **PR-I.1a** — `module/go.mod` + root `go.work` + `builder-config.yaml`
`replaces:` skeleton. No file movement. Tag `module/v0.0.1` (genesis
tag, validates the tagging contract).
- **PR-I.1b** — `git mv components/receivers/nccl_fr →
module/receiver/ncclfrreceiver` + `git mv pkg/nccl/fr_parser →
module/pkg/nccl/fr_parser` + rename Go package `nccl_fr` →
`ncclfrreceiver` + update all importers. Hard-gated on PR-B2. No new
tag; next bump is `module/v0.1.0` at PR-I.2.
- **PR-I.2** — `rankjoinprocessor` + `patterndetectorprocessor` net-new.
Hard-gated on PR-K.1. Tag `module/v0.1.0` (first version pinned in
`builder-config.yaml` for v0.2.0).

Also: PR-J marked `(landed, #195)` with note that recipe docs landed but
chart-values compat map follows in PR-K.3.

## Adversarial review (5 lenses, inline)

- **(a) PR slug internal consistency**: PR-I.1b ↔ PR-B2 ↔ PR-I.2 ↔
PR-K.1 bidirectional gates all match. PR-J landed marker consistent with
#195. §4 v0.2.0 row has pre-existing drift (mentions dcgm+kueue in
v0.2.0 when PR-F+#168 already deleted them in v0.1.0) — out of these 4
amendments' scope; flag for follow-up.
- **(b) PR-B2 hard-gate naming**: tightened from "Hard gate for PR-I.1"
to "Hard gate for PR-I.1b" — accurate because PR-I.1a is
scaffolding-only with no file movement.
- **(c) Sub-PR numbering collision**: #188 explicitly addressed in
slug-collision note. #185/#186/#187/#193/#194/#196 are PR-B1-shape ports
for non-nccl receivers (no PR-slug label in their commits), no
collision.
- **(d) Stale external-repo refs**: `grep -rn
"tracecoreai/tracecore-components" docs/ AGENTS.md README.md` returns
zero hits post-amendment.
- **(e) Cross-reference link integrity**:
`docs/migration/v0.1-to-v0.2.md` references `#migration--rollout` and
`#3-customer-stable-telemetry-contracts`; both anchors preserved (§
headers unchanged). `make doc-check` confirms 526 markdown links
resolve.

## Test plan

- [x] `make doc-check` — 526 markdown links resolve, 0 stale refs,
banned-phrase lint clean, alert-check + chart-appversion gates green.
- [x] Pre-push hooks: golangci-lint clean, go vet clean, go mod verify
clean.
- [ ] CI doc-check + actionlint + zizmor gates pass on PR.

```release-notes
NONE
```

Signed-off-by: Tri Lam <tri@maydow.com>
Co-authored-by: Tri Lam <tri@maydow.com>
trilamsr pushed a commit that referenced this pull request May 31, 2026
Signed-off-by: Tri Lam <tri@maydow.com>
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.

1 participant