Add metrics exporter#184
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dbd077325
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| auto handler = CreateTrackHandler(type); | ||
| session_.PublishTrack(connection_handle, handler); |
There was a problem hiding this comment.
When any metrics sample is processed, this path unconditionally opens the three metrics publish tracks on that connection, even for clients that never sent SUBSCRIBE_TRACKS for the metrics namespace. In deployments with normal media-only clients, their first metrics callback now consumes request IDs and sends unsolicited PUBLISH control messages for metrics tracks; wire this into the existing metrics namespace subscription flow instead of creating tracks for every connection.
Useful? React with 👍 / 👎.
| std::lock_guard<std::mutex> lock(tracks_mutex_); | ||
| tracks_by_connection_.try_emplace(connection_handle, std::move(tracks)); |
There was a problem hiding this comment.
If a connection closes after the initial tracks_by_connection_ check but before this insertion, RemoveConnection() can erase it and then this worker thread stores handlers for a closed connection. Later PublishSample() iterates every stored handler, so subsequent metrics keep trying to publish to this stale target until process exit; revalidate the connection under the mutex before try_emplace or keep an active-connection marker.
Useful? React with 👍 / 👎.
…be tracks and subscribe matching
| return false; | ||
| } | ||
|
|
||
| it->second->ObjectReceived(object_headers, data); |
There was a problem hiding this comment.
I wonder if this should be dispatched across from the metrics thread (which I think it's being called from in the metrics case) to the notifier thread either here or by the caller. Otherwise it could race with internal state that assumes ObjectReceived is only ever called from the notifier thread.
2 participants
Export metrics via publish track using namespace
<metrics namespace>/<relay id>and name<type such as connection, subscribe, publish>Metrics will be emitted every time the callback is called with metrics.
Adapters/consumers can consume metrics by using SUBSCRIBE_TRACKS on the metrics namespace.
Instead of using a JSON lib with another deps, this has a small implementation to format the metrics in json format.