New statistics, without calling out to Plot.ly. #61

jcjones · 2016-09-17T16:58:21Z

Operations should set a cron job to call _maintenance_update_graph_data.sh
periodically throughout the day. When there's a more permament server we'll
update the URL that script downloads from.

Right now, my server produces an update once per day, and that's all this
dataset has for resolution anyway, so it should be OK.

There may need to be some git finangling to force-update around the constantly-changing js/certcounts.tsv, but feel free to move it around as seems appropriate.

The plot.ly JS is MIT-licensed, so we're good to go on redistributing.

I have more datasets in progress, particularly around continents, countries, and
TLDs, but this conquers the first goal of fixing things to not be so suck.

Operations should set a cron job to call `_maintenance_update_graph_data.sh` periodically throughout the day. When there's a more permament server we'll update the URL that script downloads from. Right now, my server produces an update once per day, and that's all this dataset has for resolution anyway, so it should be OK. I have more datasets in progress, particularly around continents, countries, and TLDs, but this conquers the first goal of fixing things to not be so suck.

jsha

This is terrific! I really like the overall idea here, both in terms of how we visually present the data, and how we get the data to the browser.

jsha · 2016-09-18T18:02:35Z

_maintenance_update_graph_data.sh

+#!/bin/bash
+outFile="$(dirname $0)/js/graphdata.tsv"
+
+curl --silent https://ct.tacticalsecret.com/graphdata.tsv >${outFile}


Could you name graphdata.tsv certcounts.tsv or similar? That way if we add additional graphs using this method, we can distinguish them.

jsha · 2016-09-18T18:03:43Z

js/graphdata.tsv

@@ -0,0 +1,356 @@
+datestamp	certsIssued	certsActive	fqdnsActive	regDomainsActive
+2015-09-12	1	NULL	NULL	NULL


Is there a bug in your export script? It looks like the fqdnsActive and regDomainsActive columns are frequently null when they shouldn't be.

There's no problem, it's just expensive to calculate. The NULL values are ones where I have not rewound time to derive the data. I picked a series of dates ~45 days apart to provide historical context, and going forward the data will be day-to-day.

jsha · 2016-09-18T18:04:44Z

js/stats.js

@@ -0,0 +1,74 @@
+function parse_tsv(s, f) {


Please add a comment header to this file describing what it's used for.

jsha · 2016-09-18T18:08:57Z

js/stats.js

+  dateFormat = /\d{4}-\d{2}-\d{2}/;
+  numFormat = /\d+/;
+
+  parse_tsv(this.responseText.trim(), function(row){


Rather than trimming here, it would probably be cleaner to make parse_tsv robust to empty lines.

jsha · 2016-09-18T18:09:41Z

js/stats.js

+  numFormat = /\d+/;
+
+  parse_tsv(this.responseText.trim(), function(row){
+    if (!dateFormat.exec(row[0])) {


dateFormat.test would be better here.

jsha · 2016-09-18T18:10:57Z

js/stats.js

+    }
+  }
+
+  Plotly.plot( timeline, traces, layout);


Stray space.

jsha · 2016-09-18T18:11:21Z

js/stats.js

+    legend: {
+      xanchor:"left",
+      yanchor:"top",
+      x:0,


To be consistent, we should have a space after all colons in this section.

jsha · 2016-09-18T18:15:24Z

js/stats.js

+  Plotly.plot( timeline, traces, layout);
+}
+
+window.onload = function () {


Non-blocking nit: This will wait for the whole document to be loaded, including images and scripts, before starting the fetch for graph data. It would be better to start the fetch immediately, and then if the event listener fires and the <div id="timeline"> doesn't yet exist, postpone until onload.

Sure. PR is welcome, I'm not a JS expert.

jsha · 2016-09-18T18:16:06Z

stats.md

-  <a href="https://plot.ly/~letsencrypt/20/" target="_blank" title="Certificates by ~TLD" style="display: block; text-align: center;"><img src="https://plot.ly/~letsencrypt/20.png" alt="Certificates by ~TLD" style="max-width: 100%;width: 600px;"  width="600" onerror="this.onerror=null;this.src='https://plot.ly/404.png';" /></a>
-  <script data-plotly="letsencrypt:20"  src="https://plot.ly/embed.js" async></script>
-</div>
+<script src="/js/plotly-min.js"></script>


I think both of these scripts can by async, if you make the fix I suggested in the non-blocking nit above.

To follow-up: This is dependent on the non-blocking nit, which I'd suggest we do in a follow-on. I've opened #62.

jsha · 2016-09-18T18:18:17Z

js/stats.js

+    margin: { t: 0 },
+    yaxis: { title: 'Active Count' },
+    yaxis2: {
+      title: 'Issued Per Day',


Is this title accurate? I think all of the numbers from the graphdata.tsv are actually cumulative rather than per-day.

This is accurate for yaxis2, it's covering tIssued

Maybe I'm misunderstanding. Does tIssued represent a count of the certificates actually issued on a given day, or a cumulative count? I think it's valuable to have graphs of each, but I don't think we should display daily counts on the same graph as cumulative ones.

Aha, just looked at your demo and it makes more sense now. I do think we should split out the two data types (with different scales) onto two graphs. I definitely see an argument for putting them together, since the incremental data helps explain the cumulative data, but I think the different scales together makes it much harder to interpret the graph.

OK, I added that in b2bb96c, with the compromise that I kept a combined graph at a different page (/stats-dashboard/) - that's for mozilla/moz-corsica#71 , for we Mozillians and whomever else wants a fullscreen single-graph dash.

jcjones

Thanks for the review, jsha!

jcjones · 2016-09-18T19:23:30Z

js/graphdata.tsv

@@ -0,0 +1,356 @@
+datestamp	certsIssued	certsActive	fqdnsActive	regDomainsActive
+2015-09-12	1	NULL	NULL	NULL


There's no problem, it's just expensive to calculate. The NULL values are ones where I have not rewound time to derive the data. I picked a series of dates ~45 days apart to provide historical context, and going forward the data will be day-to-day.

jcjones · 2016-09-18T19:23:38Z

_maintenance_update_graph_data.sh

+#!/bin/bash
+outFile="$(dirname $0)/js/graphdata.tsv"
+
+curl --silent https://ct.tacticalsecret.com/graphdata.tsv >${outFile}


jcjones · 2016-09-18T19:24:19Z

js/stats.js

+    margin: { t: 0 },
+    yaxis: { title: 'Active Count' },
+    yaxis2: {
+      title: 'Issued Per Day',


This is accurate for yaxis2, it's covering tIssued

jcjones · 2016-09-18T19:24:49Z

js/stats.js

+  Plotly.plot( timeline, traces, layout);
+}
+
+window.onload = function () {


Sure. PR is welcome, I'm not a JS expert.

jcjones · 2016-09-18T19:24:58Z

stats.md

-  <a href="https://plot.ly/~letsencrypt/20/" target="_blank" title="Certificates by ~TLD" style="display: block; text-align: center;"><img src="https://plot.ly/~letsencrypt/20.png" alt="Certificates by ~TLD" style="max-width: 100%;width: 600px;"  width="600" onerror="this.onerror=null;this.src='https://plot.ly/404.png';" /></a>
-  <script data-plotly="letsencrypt:20"  src="https://plot.ly/embed.js" async></script>
-</div>
+<script src="/js/plotly-min.js"></script>


jcjones · 2016-09-18T19:27:25Z

js/stats.js

+    legend: {
+      xanchor:"left",
+      yanchor:"top",
+      x:0,


jcjones · 2016-09-18T19:27:43Z

js/stats.js

+    }
+  }
+
+  Plotly.plot( timeline, traces, layout);


jcjones · 2016-09-18T19:27:56Z

js/stats.js

+  numFormat = /\d+/;
+
+  parse_tsv(this.responseText.trim(), function(row){
+    if (!dateFormat.exec(row[0])) {


jcjones · 2016-09-18T19:29:33Z

js/stats.js

+  dateFormat = /\d{4}-\d{2}-\d{2}/;
+  numFormat = /\d+/;
+
+  parse_tsv(this.responseText.trim(), function(row){


jcjones · 2016-09-18T19:29:40Z

js/stats.js

@@ -0,0 +1,74 @@
+function parse_tsv(s, f) {


jcjones · 2016-09-19T09:18:45Z

"demo": https://ct.tacticalsecret.com/

There was an off-by-one error in the summation math for the numActive column. Also, I've continued calculating old numActiveFQDNs and numActiveRegistereDomains for the dataset, though it takes time for each row. Updating this here is a little silly, since the maintenance task will update it anyway, but eh, why not have the latest when this merges to master? :) And also, I noticed the off-by-one error in the Github review, so it feels right to fix it.

- Also adds "stats-dashboard/" which contains the combined graph, as that's what I'd like to show internally in Mozilla. :) - Do an atomic update in _maintenance_update_graph_data.sh to avoid the case where during the curl execution the tsv would be blank. - Moves some styling into css/main.scss.

jsha

Generally looking good!

jsha · 2016-09-20T19:00:23Z

js/stats.js

@@ -0,0 +1,129 @@
+// stats.js is used by stats.md to download graph data from the webserver,
+// and then display it using plotly.js. Right now it displays a single graph.


"displays a single graph" is now out of date.

jsha · 2016-09-20T19:04:01Z

stats-dashboard.md

+---
+
+<div class="dashboard">
+


Please add an HTML comment describing why this is here and linking to moz-corsica, so future maintainers know there is a dependency.

jsha

Whoops, accidentally hit the "Approve" button too early. There are still a couple of minor changes (above), but also this is blocked on Ops deployment work.

jsha · 2016-09-20T20:47:26Z

Also, could you take the step of rewinding time to fill in the history gaps before we commit this? The linear jumps on the graph make it looks like something noteworthy happened, when it didn't.

This kicks off the TSV fetch without waiting for the whole document to finish loading. It also puts the "var" keyword on some variables.

jsha · 2016-09-20T22:33:51Z

_maintenance_update_graph_data.sh

+# Download to a temp file, then move over-top the original, so that the update is
+# atomic from the perspective of the webserver
+curl --silent https://ct.tacticalsecret.com/cert-timeline.tsv >"${outFile}.bak"
+mv "${outFile}.bak" "${outFile}"


This mv should only happen if the curl is successful, so curl ... && mv .... Alternately you can set -o errexit at the top of the script. I don't have strong opinions about which one.

Sure; I debated even having this script, though - I figured Ops should pull this out, customize it, and delete it from the repo to avoid the chances of someone convincing us to merge a change with badness in it.

Still, updated in 0984a30. (correction: bad internet connection)

jcjones · 2016-09-20T23:02:56Z

If you want to wait to merge, that's fine. I figured even with some missing data it was better than the existing graphs, and the NULLs will fill in over time with the maintenance task picking up the increased-fidelity data.

Optimistically - at the rate it's currently going - they should all be filled in sometime mid-next-week, approximately September 28th. I'm afraid I don't have the time to optimize it more before it completes. I'll check back in after a few days and see if I should commit another intermediate state.

- Set errexit in _maintenance_update_graph_data.sh - Reset it to not-executable, and rename to indicate it shouldn't be directly run - Remove inaccurate comment in stats.js - Note reference to Mozilla in stats-dashboard. The exact tooling is likely to change, but at the time of this commit it was relative to https://github.com/mozilla/moz-corsica/

jcjones · 2016-09-20T23:23:38Z

Merged your slick stats-update-update in 4903f1f -- thank you! I knew there'd be a clean JS way to do such a thing, but all I knew to do was to bring in require.js so that stats.js and plotly.js would load in the right order, and then make an event listener in stats.js.... So yeah, thanks!

Move Google Analytics to its own file and load it per guidance: https://developers.google.com/analytics/devguides/collection/analyticsjs/#alternative_async_tracking_snippet This in combination with PR #61 permits the following, much tighter CSP policy: > Content-Security-Policy "default-src 'self'; style-src 'unsafe-inline' 'self'; script-src 'unsafe-eval' 'self' https://www.google-analytics.com; img-src 'self' data: https://www.google-analytics.com;" This can land whenever, but the CSP policy shouldn't be updated until #61 lands. Additionally, the CSP policy should be updated with care and tested as well as possible before being left alone.

That makes sure the Plotly script is fully loaded.

…ats_update

jcjones added 2 commits September 17, 2016 09:56

More graph cleanups.

c81f86e

jsha requested changes Sep 18, 2016

View reviewed changes

Add the graphdata to GitIgnore

c4e541a

jcjones commented Sep 18, 2016

View reviewed changes

Review updates

02ea6e7

jcjones force-pushed the stats_update branch from c9883a7 to 02ea6e7 Compare September 18, 2016 19:35

This was referenced Sep 18, 2016

The new statistics graphs should load asynchronously #62

Closed

Tighten the Content-Security-Policy #63

Closed

Rename to cert-timeline.tsv

1b3ebb6

jcjones added 3 commits September 19, 2016 10:37

Swap the left-and-right Y axes for clarity

b550561

jsha reviewed Sep 20, 2016

View reviewed changes

jsha approved these changes Sep 20, 2016

View reviewed changes

jsha requested changes Sep 20, 2016

View reviewed changes

Load TSV asynchronously.

3cd1845

This kicks off the TSV fetch without waiting for the whole document to finish loading. It also puts the "var" keyword on some variables.

jsha requested changes Sep 20, 2016

View reviewed changes

jcjones force-pushed the stats_update branch from 7bcccd4 to 839867f Compare September 20, 2016 23:10

jcjones force-pushed the stats_update branch from 839867f to 0984a30 Compare September 20, 2016 23:11

Merge in @jsha's super-slick stats-update-update!

4903f1f

Fix ordering for stats-dashboard.md

3cdb716

jcjones mentioned this pull request Sep 23, 2016

Tighten CSP #69

Merged

jsha mentioned this pull request Oct 11, 2016

Statistics Engine - Initial Version (Feature Parity) letsencrypt/boulder#1919

Closed

jsha and others added 4 commits October 13, 2016 17:50

Wait for document ready rather than DOM.

5af6666

That makes sure the Plotly script is fully loaded.

Merge branch 'stats_update' of github.com:letsencrypt/website into st…

b3d11ba

…ats_update

Add a graph to show Firefox Telemetry of what % of pageloads are TLS.

c60fdbc

Note that it's a moving average

50c15e9

danjeffery merged commit ccf2532 into master Oct 21, 2016

jsha deleted the stats_update branch July 31, 2017 21:17

		@@ -0,0 +1,356 @@
		datestamp certsIssued certsActive fqdnsActive regDomainsActive
		2015-09-12 1 NULL NULL NULL

		@@ -0,0 +1,129 @@
		// stats.js is used by stats.md to download graph data from the webserver,
		// and then display it using plotly.js. Right now it displays a single graph.

		---

		<div class="dashboard">

New statistics, without calling out to Plot.ly. #61

New statistics, without calling out to Plot.ly. #61

Conversation

jcjones commented Sep 17, 2016 • edited

jsha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcjones left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcjones commented Sep 19, 2016

jsha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsha left a comment

Choose a reason for hiding this comment

jsha commented Sep 20, 2016

Choose a reason for hiding this comment

jcjones Sep 20, 2016 • edited

Choose a reason for hiding this comment

jcjones commented Sep 20, 2016

jcjones commented Sep 20, 2016

jcjones commented Sep 17, 2016 •

edited

jcjones Sep 20, 2016 •

edited