Add DirectHttpClient - #328
Conversation
DirectHttpClient is a simple, single-threaded interface for sending pre-aggregated metrics, an equivalent of gaugeWithTimestamp and countWithTimestamp from regular dogstatsd, but also supports distributions. To avoid data corruption when moving from one client to another, we still need to apply count to rate translation. Main role for the client is to own separation between series and sketches, build payloads and route them to their respective endpoints in the core agent. Users of this class are expected to have unique timeseries and submit one point per time series per payload (an iteration of user's reporting loop). For this reason we do not do any de-duplication or grouping of metrics, keeping the code simpler and faster. Duplicates and multiple points are not a correctness issue, and simply result in less efficient payloads.
![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: a1eb2b5f45
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } catch (InterruptedException ex) { | ||
| Thread.currentThread().interrupt(); | ||
| } |
There was a problem hiding this comment.
When the supplied forwarder blocks and is interrupted, this catch makes PayloadBuilder.flushPayload() believe the send succeeded; it then clears the buffered payload, so flush() or an auto-flush can return with the thread interrupted but with metrics permanently dropped. This is reachable with the provided HTTP forwarder configured with WhenFull.BLOCK, where send() waits for queue space and throws InterruptedException; let the failure propagate or preserve the payload for retry instead of swallowing it here.
Useful? React with 👍 / 👎.
| try { | ||
| forwarder.send(seriesUri, payload); | ||
| } catch (InterruptedException ex) { | ||
| Thread.currentThread().interrupt(); |
There was a problem hiding this comment.
The codex comment seems valid. Is it intentional to continue with clearing the payload in the case that send() is interrupted and the payload is never enqueued?
There was a problem hiding this comment.
Current behavior is definitely sub-optimal, although I'm not sure if propagating the exception to the caller would be any more helpful. Fortunately, thread interruption is not something that happens by itself, it has to be coded in, and isn't that common. I'll leave a note about this behavior and will revisit blocking mode later.
2 participants
DirectHttpClient is a simple, single-threaded interface for sending pre-aggregated metrics, an equivalent of gaugeWithTimestamp and countWithTimestamp from regular dogstatsd, but also supports distributions.
To avoid data corruption when moving from one client to another, we still need to apply count to rate translation.
Main role for the client is to own separation between series and sketches, build payloads and route them to their respective endpoints in the core agent.
Users of this class are expected to have unique timeseries and submit one point per time series per payload (an iteration of user's reporting loop). For this reason we do not do any de-duplication or grouping of metrics, keeping the code simpler and faster. Duplicates and multiple points are not a correctness issue, and simply result in less efficient payloads.