MQ-1154 Add metrics() method to Queue binding#6246
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
7f7bc65 to
640cb4c
Compare
a0e4962 to
ce9f7b5
Compare
ce9f7b5 to
9824741
Compare
9824741 to
1c84049
Compare
| export interface QueueMetrics { | ||
| backlogCount: number; | ||
| backlogBytes: number; | ||
| oldestMessageTimestamp: number; |
There was a problem hiding this comment.
Sorry for coming here late, but is the units of this timestamp documented anywhere? Times should generally either have an obvious type (like Date) or a clear unit (is this in seconds since the epoch? milliseconds? something else?
There was a problem hiding this comment.
For example the field 5 lines up is called delaySeconds rather than just delay. The unit doesn't have to be in the field name itself -- although that is often a good place for it -- but it does at least need to be clearly documented.
There was a problem hiding this comment.
You're right, it may not be very clear. On the HTTP side, it will be documented in the OpenAPI schema, what would be the best way to document it in workerd?
There was a problem hiding this comment.
Looking at it more closely, the best answer here would be to actually return this as a Date type (which IIRC can be easily done by making the C++ datatype a kj::Date).
This would parallel the timestamp field in IncomingQueueMessage or a bunch of the other runtime APIs that return a Date if you look at the rest of this file.
There was a problem hiding this comment.
That makes sense, thank you for pointing it out! I will get that change made.
Summary
Adds a new
metrics()method to theWorkerQueuebinding. The method returns consumer backlog information via a GET subrequest to the/metricsendpoint on the upstream Queues service.Changes
This PR introduces changes to
queue.c++andqueue.hto implement themetrics()method. The new method definition is gated behind thequeues_metrics_apicompat flag and returns the following response type:The upstream changes can be found here:
https://gitlab.cfdata.org/cloudflare/mq/queue-broker-worker/-/merge_requests/1734
https://gitlab.cfdata.org/cloudflare/mq/queue-broker-worker/-/merge_requests/1759
Testing
bazel test //src/workerd/api/tests:queue-test@- queue send/sendBatch tests + error-codes tests passbazel test //src/workerd/api/tests:queue-test@all-compat-flagsbazel test //src/workerd/api/tests:queue-metrics-test@- tests new api with flag enabled + flag disabledbazel test //src/workerd/api/tests:queue-metrics-test@all-compat-flags