task-mon icon indicating copy to clipboard operation
task-mon copied to clipboard

Detect and cache the Ping-Body-Limit response header and support non-default limit

Open dimo414 opened this issue 3 years ago • 2 comments

As of v2.1 responses include a Ping-Body-Limit header indicating how large the log payload can be. We could cache this value somewhere like /tmp/.healthchecks-<base_url_slug>.Ping-Body-Limit to be used in subsequent invocations.

Also, the existing hard-coded value doesn't support other installs with different limits, so a --body_limit flag should be added so callers can proactively specify the limit they'd like to use.

dimo414 avatar Sep 04 '22 07:09 dimo414

Another option would be adding start ping support to task-mon and grabbing the instance's ping body limit from that response, so you don't need to maintain any local state.

It was designed impromptu in https://github.com/bdd/runitor/discussions/32#discussioncomment-2694931. Relevant lines in runitor: https://github.com/bdd/runitor/blob/main/cmd/runitor/main.go#L253-L270.

FWIW, even without instance config sniffing capability, sending start pings provide a ton of value. e.g.

  • measuring the execution time
  • catching task runner bugs ;)
  • coupled with run ids, an ability to have multiple runners reporting to same uuid or pingKey+slug
  • catching possibly stuck or unhealthily slow jobs by giving them if they don't a report status grace_time after the start ping.

bdd avatar Dec 14 '22 01:12 bdd

adding start ping support to task-mon

Start pings are already supported via the --time flag; I'd intentionally made it an opt-in feature, but I do take your point about the benefits of turning it on by default. I'll split that out into a separate FR.

coupled with run ids

I was not aware of that new feature! Very cool!

if they don't a report status grace_time after the start ping.

IIRC that was part of my motivation for not enabling --time by default. I didn't want users to need to configure the grace time properly if it didn't align with their use case.

dimo414 avatar Dec 14 '22 07:12 dimo414