Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dimo414 opened this issue Sep 4, 2022 · 2 comments

Comments

@dimo414
Copy link
Owner

dimo414 commented Sep 4, 2022

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.

@bdd
Copy link

bdd commented Dec 14, 2022

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 bdd/runitor#32 (reply in thread).
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.

@dimo414
Copy link
Owner Author

dimo414 commented Dec 14, 2022

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.

This was referenced Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants