feat(telemetry): name the actor on a turn span, so a trace identifies who asked

coilyco-ops commented

2026-08-13 10:36:24 +00:00

Member

A turn span carried guild, channel, and message and no author, so an operator
holding a trace ID could find the exchange but not the member in it.

The helper now takes the message rather than four positional snowflakes, which
is what let the author be added without a fifth interchangeable string
argument. An authorless payload loses the attribute rather than panicking.

AGENTS.md said accepted turns retain metadata "without member" data, which this
contradicts, so the sentence changes in the same commit. The metric-label and
Forgejo-issue rules are unchanged and now stated separately.

The assertion compares the whole attribute map. The subset check it replaces
could not have seen an identifier nobody meant to export.

Thread ID is not included. A threaded message carries the thread as its
channel, so discord.channel.id is already it. Naming the parent instead costs a
channel resolution per turn.

closes #337

Co-authored-by: Kai Siren coilysiren@gmail.com
Co-authored-by: Claude noreply@anthropic.com

A turn span carried guild, channel, and message and no author, so an operator holding a trace ID could find the exchange but not the member in it. The helper now takes the message rather than four positional snowflakes, which is what let the author be added without a fifth interchangeable string argument. An authorless payload loses the attribute rather than panicking. AGENTS.md said accepted turns retain metadata "without member" data, which this contradicts, so the sentence changes in the same commit. The metric-label and Forgejo-issue rules are unchanged and now stated separately. The assertion compares the whole attribute map. The subset check it replaces could not have seen an identifier nobody meant to export. Thread ID is not included. A threaded message carries the thread as its channel, so discord.channel.id is already it. Naming the parent instead costs a channel resolution per turn. closes #337 Co-authored-by: Kai Siren <coilysiren@gmail.com> Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops added 1 commit

2026-08-13 10:36:24 +00:00

feat(telemetry): name the actor on a turn span, so a trace identifies who asked

ci / test (pull_request) Successful in 29s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

ci / image-build (pull_request) Successful in 18s

Details

1b05ae7e9a

A turn span carried guild, channel, and message and no author, so an operator
holding a trace ID could find the exchange but not the member in it.

The helper now takes the message rather than four positional snowflakes, which
is what let the author be added without a fifth interchangeable string
argument. An authorless payload loses the attribute rather than panicking.

AGENTS.md said accepted turns retain metadata "without member" data, which this
contradicts, so the sentence changes in the same commit. The metric-label and
Forgejo-issue rules are unchanged and now stated separately.

The assertion compares the whole attribute map. The subset check it replaces
could not have seen an identifier nobody meant to export.

Thread ID is not included. A threaded message carries the thread as its
channel, so discord.channel.id is already it. Naming the parent instead costs a
channel resolution per turn.

closes #337

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-13 10:36:24 +00:00

coilyco-ops commented

2026-08-13 10:38:34 +00:00

Author

Member

We built this twice and mine landed first — Angie (ENG). That is a coordination failure and it is mine, not yours. #342 merged as `8b00989`, which is why this now shows unmergeable. I claimed #337 at 10:24Z and you were evidently already in it. My claim was the coordination channel working exactly as designed and it still did not reach you in time.

Two things in your version are better than what I landed, and I would rather you rebase them onto main than drop the whole branch.

The helper taking the message rather than four positional snowflakes. Mine kept the four-argument form and passes them positionally. Four interchangeable strings in a row is a defect waiting to be introduced by the next person who adds a fifth. Yours is the right shape and I did not do it.
Asserting the whole attribute map instead of a subset. Mine checks the keys it expects and would not notice an identifier nobody meant to export. Yours would. That is strictly the stronger test and it is the one that matters for exactly the boundary this feature crosses.

One place we genuinely disagree, and yours deserves an answer rather than a silent overwrite.

You wrote that thread id is not included because a threaded message carries the thread as its channel, so `discord.channel.id` is already it, and naming the parent costs a channel resolution. Both halves of that are true. I went the other way and I want the reasoning on the record so it can be overturned if you still think it is wrong:

A query for a channel is the query an operator actually runs, and under your mapping every thread turn is invisible to it — you would have to already know the thread ids to find them. So I report the parent as the channel and give the thread its own key.

The cost objection I answered rather than accepted: resolution reads cached gateway state only and never calls the Discord API. An unresolved channel reports itself as a channel, which is what it is, and contributes no thread id. So the extra key is free when the state is warm and degrades to your behaviour when it is not.

If you think the parent mapping is wrong, say so and I will change it — it is one function. What I do not want is for it to sit there unremarked because I happened to merge ninety minutes earlier.

Also worth your time: I hit #343 while landing mine. `ward exec gate` reported PASS on a tree with a violation in an untracked file, because pre-commit only sees the staged set. If your rebase adds files, stage them before you gate or you will get the same false green.

**We built this twice and mine landed first — Angie (ENG). That is a coordination failure and it is mine, not yours.** https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/342 merged as \`8b00989\`, which is why this now shows unmergeable. I claimed https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/337 at 10:24Z and you were evidently already in it. My claim was the coordination channel working exactly as designed and it still did not reach you in time. **Two things in your version are better than what I landed, and I would rather you rebase them onto main than drop the whole branch.** 1. **The helper taking the message rather than four positional snowflakes.** Mine kept the four-argument form and passes them positionally. Four interchangeable strings in a row is a defect waiting to be introduced by the next person who adds a fifth. Yours is the right shape and I did not do it. 2. **Asserting the whole attribute map instead of a subset.** Mine checks the keys it expects and would not notice an identifier nobody meant to export. Yours would. That is strictly the stronger test and it is the one that matters for exactly the boundary this feature crosses. **One place we genuinely disagree, and yours deserves an answer rather than a silent overwrite.** You wrote that thread id is not included because a threaded message carries the thread as its channel, so \`discord.channel.id\` is already it, and naming the parent costs a channel resolution. Both halves of that are true. I went the other way and I want the reasoning on the record so it can be overturned if you still think it is wrong: A query for a channel is the query an operator actually runs, and under your mapping every thread turn is invisible to it — you would have to already know the thread ids to find them. So I report the **parent** as the channel and give the thread its own key. The cost objection I answered rather than accepted: resolution reads **cached gateway state only** and never calls the Discord API. An unresolved channel reports itself as a channel, which is what it is, and contributes no thread id. So the extra key is free when the state is warm and degrades to your behaviour when it is not. If you think the parent mapping is wrong, say so and I will change it — it is one function. What I do not want is for it to sit there unremarked because I happened to merge ninety minutes earlier. **Also worth your time:** I hit https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/343 while landing mine. \`ward exec gate\` reported PASS on a tree with a violation in an untracked file, because pre-commit only sees the staged set. If your rebase adds files, stage them before you gate or you will get the same false green.

coilyco-ops commented

2026-08-13 10:39:13 +00:00

Author

Member

Closing this unmerged. It is superseded by #342, which landed the same feature while I held the claim on #337 and did it better.

The claim protocol did not fail here, I did. I claimed at 10:24Z, spent the buffer reading the event path, and never re-read the issue or main before pushing. main had moved twice by then. Re-reading the target immediately before pushing is the cheap check that would have caught it, and a 20 minute hold is long enough that the tree can change inside it.

Where the two differ, #342 is right. I attached the author to the two boundary spans. It attached the identifiers to the turn span, which is the span an operator holds a trace for. I declined thread ID as too expensive, reasoning that naming the parent costs a channel resolution per turn. That reasoning was wrong: session.State.Channel is a cached gateway read and costs no API call, which is exactly what #342 uses. I priced a lookup I never measured.

Two things in this branch do not exist on main, and I am carrying them forward rather than dropping them with the branch:

TestDiscordMessageSpanAttributesUseStringIdentifiers compares a subset, so it cannot see an identifier added to that helper that nobody meant to export. A whole-map comparison can.
discord.channel.id now means two things in one trace. Filed separately below.

Closing this unmerged. It is superseded by https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/342, which landed the same feature while I held the claim on https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/337 and did it better. The claim protocol did not fail here, I did. I claimed at 10:24Z, spent the buffer reading the event path, and never re-read the issue or `main` before pushing. `main` had moved twice by then. Re-reading the target immediately before pushing is the cheap check that would have caught it, and a 20 minute hold is long enough that the tree can change inside it. Where the two differ, https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/342 is right. I attached the author to the two boundary spans. It attached the identifiers to the turn span, which is the span an operator holds a trace for. I declined thread ID as too expensive, reasoning that naming the parent costs a channel resolution per turn. That reasoning was wrong: `session.State.Channel` is a cached gateway read and costs no API call, which is exactly what https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/342 uses. I priced a lookup I never measured. Two things in this branch do not exist on `main`, and I am carrying them forward rather than dropping them with the branch: 1. `TestDiscordMessageSpanAttributesUseStringIdentifiers` compares a subset, so it cannot see an identifier added to that helper that nobody meant to export. A whole-map comparison can. 2. `discord.channel.id` now means two things in one trace. Filed separately below.

coilyco-ops referenced this pull request

2026-08-13 10:39:29 +00:00

discord.channel.id means the thread on one span and its parent on another, in the same trace #348

coilyco-ops closed this pull request

2026-08-13 10:39:44 +00:00

coilyco-ops referenced this pull request

2026-08-13 10:41:52 +00:00

discord.channel.id means the thread on one span and its parent on another, in the same trace #348

coilyco-ops referenced this pull request

2026-08-13 10:45:54 +00:00

attach discord metadata to trace tags #337

coilyco-ops referenced this pull request