Skip to content

stats: name PUBLISH counters by relay role - #528

Merged
afrind merged 1 commit into
mainfrom
fix/151-publish-counter-naming
Jul 29, 2026
Merged

stats: name PUBLISH counters by relay role#528
afrind merged 1 commit into
mainfrom
fix/151-publish-counter-naming

Conversation

@afrind

@afrind afrind commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

PUBLISH counters now follow the same convention as SUBSCRIBE: the prefix is the relay's own role and Success/Error is the request outcome.

moqPublishSuccess -> pubPublishSuccess (relay sent PUBLISH)
moqPublishError -> pubPublishError
moqPublishOkSent -> subPublishSuccess (relay received PUBLISH)
moqPublishReceived -> dropped

subPublishReceived is dropped rather than renamed: it counted request arrival, which no other handshake exposes, and its value is implied by subPublishSuccess + subPublishError. onPublish() stays as a no-op override because moxygen declares it pure virtual.

Renames the exported Prometheus series, so external dashboards and alerts referencing the moqx_moqPublish* names need updating.

Fixes: #151


This change is Reviewable

PUBLISH counters now follow the same convention as SUBSCRIBE: the prefix
is the relay's own role and Success/Error is the request outcome.

  moqPublishSuccess  -> pubPublishSuccess    (relay sent PUBLISH)
  moqPublishError    -> pubPublishError
  moqPublishOkSent   -> subPublishSuccess    (relay received PUBLISH)
  moqPublishReceived -> dropped

subPublishReceived is dropped rather than renamed: it counted request
arrival, which no other handshake exposes, and its value is implied by
subPublishSuccess + subPublishError.  onPublish() stays as a no-op
override because moxygen declares it pure virtual.

Renames the exported Prometheus series, so external dashboards and alerts
referencing the moqx_moqPublish* names need updating.

Fixes: #151

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

@gmarzot gmarzot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@gmarzot reviewed 4 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on afrind).

@gmarzot gmarzot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is the rationale around moqx_ ... like a catgegory? the "moqx_moq..." vs "moqx_pub..."

@gmarzot made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on afrind).

@afrind

afrind commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

what is the rationale around moqx_ ... like a catgegory? the "moqx_moq..." vs "moqx_pub..."

I think I removed all the moqx_moq now. moqx prefixes all our counters -- it's always been like that, maybe just so you can tell who you are talking to if you didn't know?

@afrind
afrind merged commit 09591a8 into main Jul 29, 2026
16 checks passed
@afrind
afrind deleted the fix/151-publish-counter-naming branch July 29, 2026 21:57
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.

moqx_moqPublishSuccess_total should be moqx_pubPublishSuccess_total?

2 participants