MoQ stats gauge correction

[akash-a-n]

removed unnecessary stats
correctly tracks the number of active publishers

---

This change is 

[akash-a-n]

I have removed a bunch of namespace gauges that were inaccurately tracking the number of active publishNamespace and subscribeNamespace messages.
I have fixed a double decrement bug for pubActiveSubscribers -- (number of subscribers when session is a publisher)
I have fixed the non-decrementing issue with subActivePublishers -- (number of publishers when session is a subscriber)

@akash-a-n made 1 comment.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on afrind, gmarzot, michalhosna, mondain, Oxyd, peterchave, suhasHere, and TimEvens).

[gmarzot]

how does this PR relate to: openmoq/moxygen#148

independent?

[akash-a-n]

The third fix - for subActivePublishers needed changes in in both the layers moqx relay and in the underlying moxygen. #148 corresponds to the underlying change.

@akash-a-n made 1 comment.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on afrind, gmarzot, michalhosna, mondain, Oxyd, peterchave, suhasHere, and TimEvens).

[peterchave]

Has a fix for moqx_quicActiveStreams been looked at?
That guage appears to either not be incrementing or double decrementing?

[peterchave]

@peterchave reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on afrind, gmarzot, michalhosna, mondain, Oxyd, suhasHere, and TimEvens).

[akash-a-n]

I'll take a look at moqx_quicActiveStreams

@akash-a-n made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on afrind, gmarzot, michalhosna, mondain, Oxyd, suhasHere, and TimEvens).

[akash-a-n]

Fixed it in #256

@akash-a-n made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on afrind, gmarzot, michalhosna, mondain, Oxyd, suhasHere, and TimEvens).

[afrind]

@akash-a-n I rebased/force-pushed this for you.

Claude noticed:

Potential bug — onSubscribeError decrements without a prior increment:

  void MoQStatsCollector::SubscriberCallback::onSubscribeSuccess() {
    ++parent_.subActivePublishers_;   // incremented only on success
  }

  void MoQStatsCollector::SubscriberCallback::onSubscribeError(...) {
    --parent_.subActivePublishers_;   // decremented on error — but was never incremented!
  }

onSubscribeSuccess and onSubscribeError are mutually exclusive terminal outcomes of a SUBSCRIBE. If a subscribe
fails (error, not success), subActivePublishers_ will go negative. The decrement on onSubscribeError should be
removed.

--

Potential double-decrement: Both onUnsubscribe and onPublishDone in SubscriberCallback decrement
subActivePublishers_. If the upstream always sends PUBLISH_DONE after an unsubscribe (as the
PublisherCallback::onUnsubscribe comment suggests is the case on the pub side), this would double-decrement for
the same subscription end. Worth verifying which events actually fire together on the subscriber side.

This sounds strange - pretty sure moxygen doesn't fire publshDone after the app unsubscribes

[akash-a-n]

I have update the code to use the new callbacks added to moxygen for tracking subscription begin and subscription end.
Tested it out with conformance script. I see all gauges fall back to 0 -- no leaks.

@akash-a-n made 1 comment.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on afrind, gmarzot, michalhosna, mondain, Oxyd, peterchave, suhasHere, and TimEvens).

[afrind]

@afrind reviewed 3 files and all commit messages, and made 2 comments.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on gmarzot, michalhosna, mondain, Oxyd, suhasHere, and TimEvens).

---

src/stats/MoQStatsCollector.cpp line 108 at r3 (raw file):

void MoQStatsCollector::PublisherCallback::onPublishNamespaceSuccess() {
  ++parent_.pubPublishNamespaceSuccess_;
  ++parent_.pubActivePublishNamespaces_;

Why did we decide to drop these gauges? Is the intent to add them back later, or have downstream systems compute them?

---

src/stats/MoQStatsCollector.cpp line 145 at r3 (raw file):

void MoQStatsCollector::PublisherCallback::onUnsubscribe() {
  // Downstream subscriber unsubscribed.

We may want to count the number of unsubscribe's received regardless

[akash-a-n]

@akash-a-n made 2 comments.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on gmarzot, michalhosna, mondain, Oxyd, suhasHere, and TimEvens).

---

src/stats/MoQStatsCollector.cpp line 108 at r3 (raw file):

Previously, afrind wrote…

Why did we decide to drop these gauges? Is the intent to add them back later, or have downstream systems compute them?

These gauges seemed redundant. They have a direct correlation with their counters.

---

src/stats/MoQStatsCollector.cpp line 145 at r3 (raw file):

Previously, afrind wrote…

We may want to count the number of unsubscribe's received regardless

Sure, added two new metrics to count unsubscribe messages.

[afrind]

@afrind reviewed 2 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on gmarzot, michalhosna, mondain, Oxyd, suhasHere, and TimEvens).