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

Alert on too old Last-Modified #3299

Closed
1 task done
TheLastProject opened this issue Jun 21, 2023 · 10 comments · Fixed by #3253
Closed
1 task done

Alert on too old Last-Modified #3299

TheLastProject opened this issue Jun 21, 2023 · 10 comments · Fixed by #3253
Labels
feature-request Request for new features to be added

Comments

@TheLastProject
Copy link

⚠️ Please verify that this feature request has NOT been suggested before.

  • I checked and didn't find similar feature request

🏷️ Feature Request Type

New Monitor

🔖 Feature description

Being able to receive an alert when a webpage hasn't changed for a while

✔️ Solution

The ability to alert based on how old the last-modified header is

❓ Alternatives

Comparing the whole webpage data, but this would be a lot more network traffic (you should be able to get the last-modified header with just a HEAD request IIRC)

📝 Additional Context

In F-Droid, the build server only updates its state when something changed. We currently view it through https://monitor.f-droid.org/ but it would be nice if we could notice if any step got stuck (the last-modified staying the same longer than expected)

See https://f-droid.org/repo/status/running.json for the current state, the content can differ a lot depending on the state, but there is always a last-modified header

@TheLastProject TheLastProject added the feature-request Request for new features to be added label Jun 21, 2023
@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Jun 21, 2023

Comparing the whole webpage data, but this would be a lot more network traffic (you should be able to get the last-modified header with just a HEAD request IIRC)

Downloading just headers vs webpage is something that should have a negligible impact on performance.
We are not doing 1k requests/sec. We at most should do 20 (400 monitors is a lot of monitors) ⇒ should be neglable

Please go into detail further why you think this is not a valid alternative

@TheLastProject
Copy link
Author

TheLastProject commented Jun 21, 2023

The last-modified header is standardized and it would be really easy to compare dates and alert if it's older than X period. One HEAD request, one header to read, one comparison to make.

Looking at the whole data requires downloading the whole thing (which for big JSON files could be hundreds or thousands of KB), remembering a (hash of) the value and storing that in a database and comparing it with (a hash of) the new value.

It also means that you can only compare to when you first downloaded the data and thus will be late to alert (think: warning after 7 days for data already stale for 6 when the alert is created) and if the value of the data goes from A to B and back to A in between checks it will not be noticed, possibly causing false positives (think a process switching between 2 states and the second state finished very quickly).

I personally see no reason to not use the last-modified header as it is both the simplest code and the most reliable and standardized method. Full downloading would probably work reliably enough for F-Droid's use case as there are more than 2 phases all taking fairly long, but it doesn't add any benefit over reading the last-modified header and complicates debugging (one could compare the last-modified header locally very easily, recreating the internal state of Uptime Kuma would be way more difficult and error prone). It therefore seems like a technologically worse solution to compare the full data in every way.

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Jun 21, 2023

The problem I am thinking about was mostly about how this should be integrated:
Checking headers is something we probably should support, but some headers require different handling compared to others.

Just adding a LastMod monitor would not be a good idea imo, as other headers might be important to check to.

Should this be added as a separate monitor (given the complexity) or as another configuration option for the http-monitor?

Would #3253 be a better solution?

@IzzySoft
Copy link

Just adding a LastMod monitor would not be a good idea imo, as other headers might be important to check to.

Then maybe a more generic approach where the header "field name" (here: "Last-Modified") could be specified? That would then be flexible enough, and could e.g. be used for ETag as well. Other useful headers coming to mind would be Content-Length, Content-MD5 – and there are probably more.

For all the ones mentioned, "value changed" would already be useful. Specific modes could apply to types like date (newer/older) and content length (smaller/bigger), while for others (hash, etag) "changed" would probably be the only relevant one – and a good "starting point" as well.

@TheLastProject
Copy link
Author

CommanderStorm: Would #3253 be a better solution?

Given all the JSON of the F-Droid buildserver has a startTimestamp field containing an unix timestamp, it seems like this feature could work for us if an option is added to interpret a JSON path as an UNIX timestamp and base the monitor.expectedValue in that MR on "More than X time ago".

So, the MR in question doesn't solve our needs yet, but the JSON parsing path could be a path with a solution for us after more features are added.

The JSON parsing path still seems slightly more difficult in Uptime Kuma's side because the date format in Last-Modified headers is standardized and there would be only one way possible to parse it, but a timestamp in JSON could be written in any number of ways, so for the JSON parsing path Uptime Kuma will need to support at least UNIX timestamps for our use case (but possibly more date formats for other people's use cases).

But yeah, with small added features #3253 could definitely work.

IzzySoft: For all the ones mentioned, "value changed" would already be useful. Specific modes could apply to types like date (newer/older) and content length (smaller/bigger), while for others (hash, etag) "changed" would probably be the only relevant one – and a good "starting point" as well.

I do think that if we have a setting to alert if a chosen header hasn't changed in a set amount of time that would work too, but it feels kinda messy and does have the downside of not being able to notice if the situation is already broken at time of monitor creation. Although I imagine it will be generally useful for people to detect things being cached for too long.

@CommanderStorm
Copy link
Collaborator

Given that jsonata supports these two features, I don't think anything needs to be changed in #3253 to allow monitoring this.

@TheLastProject
Copy link
Author

Ah, then I misunderstood the code. If using such comparisons is indeed possible then yes that will fit our use case.

@TheLastProject
Copy link
Author

Could I ask for the original request (watching the Last-Modified header on a HEAD request) to be reconsidered?

I've added the mirrors we want to monitor (which are about 40 * 6 monitors = 240 monitors) downloading a multi-megabyte JSON file and, well, Uptime Kuma gets killed on our host for being out-of-memory (even with just regular GET requests, which is why we're using HEAD requests now).

Being able to monitor Last-Modified with a HEAD request would prevent us from having to scale up our hardware :)

(Should I make a new issue perhaps, given I can't re-open this?)

@CommanderStorm
Copy link
Collaborator

Should I make a new issue perhaps, given I can't re-open this

Yes, please open a new feature-request for http(s) - header.
Btw, here is our contribution guide, in case you/somebody else needing this would like to contribute in this area ^^

From a design perspective, it might also be interesting

  • to know what you are monitoring and
  • what your current system is

⇒ if the memory consumption could/needs to be improved.
If you can, please attach a Minimal, Reproducible Example.

@obfusk
Copy link
Contributor

obfusk commented Aug 12, 2023

I thought about this a bit more and opened #3568.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features to be added
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants