Skip to content

FEATURE: Support synchronous metric sends before process exit - #384

Draft
tgxworld wants to merge 2 commits into
mainfrom
tgxworld/synchronous-metric-send
Draft

tgxworld wants to merge 2 commits into
mainfrom
tgxworld/synchronous-metric-send

Conversation

@tgxworld

@tgxworld tgxworld commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

This commit adds PrometheusExporter::Client#send_sync and PrometheusExporter::Client#send_json_sync so callers can finish writing a metric to the socket before their process exits. This lets the Prometheus plugin report Pitchfork worker timeouts through its existing client.

Synchronous sends can proceed while asynchronous metrics remain queued and propagate connection or write errors. Successful return confirms the socket write completed, without waiting for collector acknowledgement.

Older Rails test appraisals use json below version 3 because their encoders pass an option removed in that version.

This commit adds send_sync and send_json_sync to the shared exporter client so callers can wait for a metric socket write before exiting. Synchronous sends reuse serialization and coordinate socket access with the background sender. Connection and write errors propagate to the caller; successful writes do not imply collector acknowledgement.
tgxworld added a commit to discourse/discourse-prometheus that referenced this pull request Sep 25, 2026
Use the synchronous client API proposed in discourse/prometheus_exporter#384. Remove explicit queue draining and Ruby Timeout. The draft remains dependent on an exporter release and a matching gem version update.
A continuously replenished async queue could keep a synchronous sender waiting indefinitely for the socket lock.

This commit releases the lock and yields between messages so synchronous writes can proceed before the queue empties. Add a regression test for sustained async traffic. Constrain JSON below version 3 in older Rails test appraisals, whose JSON encoder still passes the removed quirks_mode option, so CI can verify the change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant