1484189 - [l10nstats] progress background image doesn't update in docker env

Status: RESOLVED FIXED

(Webtools Graveyard :: Elmo, defect)

Description

[Axel Hecht [:Pike]]

There's a local image that is used as background for the spark graphs on a couple of pages on elmo.

Example is https://l10n.allizom.org/static/l10nstats/progress.png?1039823

Currently, that only updates on container startup, but other than that, it grows stale.

In scl3, we had a cron job updating that, creating one again. Once an hour will be plenty enough (it's 50 days of data).

Comment 1

[Axel Hecht [:Pike]]

Attachment: PR

I've requested review on the PR at https://github.com/mozilla/elmo/pull/33 from Miles, but I'd just as well take Brians.

Mostly the question is about logging, and if we would get /var/log/cron.log out to bastion, or if I should log somewhere else.

I did look into markus/datadog, but that doesn't seem to be a good candidate to store data about "done this".
Also, I don't find metrics from the webheads at all anymore, though I think we should submit some.

Comment 2

[Brian Pitts]

Hi Axel, could we consider an approach that doesn't involve running a cronjob inside the webapp container? There's a number of reasons related to logging, monitoring, etc that we don't want to have cron and webapp running alongisde each other.

What if we made the progress image generation dynamic and exposed it via a django view? Since the image is a little heavy to render ( ~4 seconds in my testing) and I suspsect changes infrequently, we could consider caching it via django or nginx.

Comment 3

[[github robot]]

Commit pushed to develop at https://github.com/mozilla/elmo

https://github.com/mozilla/elmo/commit/5e674215765b9e42621562a924c5f2a9b2a7083c
bug 1484189, make progress graphs independent of the db

We used to put the background positions into the db, but for
multiple web heads, that's a bad idea.
Instead generate a css file along with the png image, and use
that.
If we move from file-based assets to generated views, this
will be a good head start.

Comment 4

[[github robot]]

Commit pushed to develop at https://github.com/mozilla/elmo

https://github.com/mozilla/elmo/commit/1f96f3e7afb8d3ded31a1e987bc4af4d622674e0
bug 1484189, make progress graphs independent of the db

We used to put the background positions into the db, but for
multiple web heads, that's a bad idea.
Instead generate a css file along with the png image, and use
that.
If we move from file-based assets to generated views, this
will be a good head start.

progress.css isn't available on test time, whitelist that when
we test our includes.

Comment 5

[[github robot]]

Commit pushed to develop at https://github.com/mozilla/elmo

https://github.com/mozilla/elmo/commit/e230e7c7fe2aae3350ca6a598bfcebb26337cd6a
bug 1484189, make progress graphs independent of the db

We used to put the background positions into the db, but for
multiple web heads, that's a bad idea.
Instead generate a css file along with the png image, and use
that.
If we move from file-based assets to generated views, this
will be a good head start.

progress.css isn't available on test time, whitelist that when
we test our includes.

Comment 6

[Axel Hecht [:Pike]]

I should have run tests locally, I guess?

Anywho, I've been doing a prep landing to make this easier either way.

Brian, right now something's setting http headers to always reload everything. We'll need to get that in check before we can look at an alternative way for this bug.

I'm also a bit concerned that we're opening up a really easy path to DOS the web heads. At least with the easy ways of using django cache framework, which only populates the cache on successful loads, and is allowing reloads, probably.

Comment 7

[Axel Hecht [:Pike]]

Attachment: Convert progress.css to dynamic view

Brian, this is an alternative to using statics and cron (or, as now, startup).

I created a view that renders the CSS.

One challenge is that once we start caching this, the css cache might get out of sync with the image cache, so I converted the image into a data url. The problem is that much of the CSS overlaps with the image generation.

That'll require to add data: to the CSP for img-src, mind adding that to stage and prod?

This doesn't do any actual caching yet, which is more OK as we're only using a single resource, and lazy-load it. I'd like to still add caching, though.

Comment 8

[Brian Pitts]

I like this approach!

I see that we are hard-coding the csp policy in nginx for elmo, which is not something we normally do. Are you amenable to moving that into django instead? That would make changes like this easier in the future.

Comment 9

[Brian Pitts]

Also, I noticed we already have http caching set up in nginx, so as soon as you start returning a cache-control header this will be cached.

Comment 10

[Axel Hecht [:Pike]]

I'm up for doing the CSP directly in elmo, but I'd like to wait on https://github.com/mozilla/django-csp/issues/109 for that.

Thanks for the pointer to cache-control, I'll add that.

Comment 11

[Axel Hecht [:Pike]]

Meh, so close yet so far: The session middleware sets a Vary: Cookie header, because session_csrf touches the session. We can try, but I bet that each user will get their own copy. But at least it should be locally cached while you're surfing along.

Comment 12

[Axel Hecht [:Pike]]

PPS: django-csp issue is fixed and released, putting that into another PR.

Comment 13

[Axel Hecht [:Pike]]

Comment on attachment 9036895 [details] [review]
Convert progress.css to dynamic view

Got an r+ on gh for this.

Comment 14

[Axel Hecht [:Pike]]

This works now in prod, marking FIXED.