From 612831a2d8bc7ee70bb4f45b21fc195b8840cd54 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Fri, 8 Mar 2019 00:05:37 -0500 Subject: [PATCH] Remove legacy analytics (#3179) We're getting good results from #3093, so there's no reason to keep maintaining this code. Ref #1848 #2068 --- core/base-service/base-static.js | 3 - core/base-service/legacy-request-handler.js | 3 - .../legacy-request-handler.spec.js | 3 - core/server/analytics.js | 148 ------------------ core/server/server.js | 7 - core/server/server.spec.js | 30 ---- doc/code-walkthrough.md | 4 +- doc/production-hosting.md | 19 +-- now.json | 3 +- 9 files changed, 8 insertions(+), 212 deletions(-) delete mode 100644 core/server/analytics.js diff --git a/core/base-service/base-static.js b/core/base-service/base-static.js index d784054765fb3..3d3b48d720f77 100644 --- a/core/base-service/base-static.js +++ b/core/base-service/base-static.js @@ -1,7 +1,6 @@ 'use strict' const makeBadge = require('../../gh-badges/lib/make-badge') -const analytics = require('../server/analytics') const BaseService = require('./base') const { serverHasBeenUpSinceResourceCached, @@ -23,8 +22,6 @@ module.exports = class BaseStaticService extends BaseService { }) camp.route(regex, async (queryParams, match, end, ask) => { - analytics.noteRequest(queryParams, match) - if (serverHasBeenUpSinceResourceCached(ask.req)) { // Send Not Modified. ask.res.statusCode = 304 diff --git a/core/base-service/legacy-request-handler.js b/core/base-service/legacy-request-handler.js index ecfeb5218fb38..3deb6e56854eb 100644 --- a/core/base-service/legacy-request-handler.js +++ b/core/base-service/legacy-request-handler.js @@ -7,7 +7,6 @@ const queryString = require('query-string') const LruCache = require('../../gh-badges/lib/lru-cache') const makeBadge = require('../../gh-badges/lib/make-badge') const { makeBadgeData: getBadgeData } = require('../../lib/badge-data') -const analytics = require('../server/analytics') const log = require('../server/log') const { setCacheHeaders } = require('./cache-headers') const { @@ -121,8 +120,6 @@ function handleRequest(cacheHeaderConfig, handlerOptions) { }) } - analytics.noteRequest(queryParams, match) - const filteredQueryParams = {} allowedKeys.forEach(key => { filteredQueryParams[key] = queryParams[key] diff --git a/core/base-service/legacy-request-handler.spec.js b/core/base-service/legacy-request-handler.spec.js index 5c465499c8833..40e4d74e6adaa 100644 --- a/core/base-service/legacy-request-handler.spec.js +++ b/core/base-service/legacy-request-handler.spec.js @@ -5,7 +5,6 @@ const fetch = require('node-fetch') const nock = require('nock') const portfinder = require('portfinder') const Camp = require('camp') -const analytics = require('../server/analytics') const { makeBadgeData: getBadgeData } = require('../../lib/badge-data') const { handleRequest, @@ -50,8 +49,6 @@ function fakeHandlerWithNetworkIo(queryParams, match, sendBadge, request) { } describe('The request handler', function() { - before(analytics.load) - let port, baseUrl beforeEach(async function() { port = await portfinder.getPortPromise() diff --git a/core/server/analytics.js b/core/server/analytics.js deleted file mode 100644 index 6419621a9a40e..0000000000000 --- a/core/server/analytics.js +++ /dev/null @@ -1,148 +0,0 @@ -'use strict' - -const fs = require('fs') -const { URL } = require('url') - -// We can either use a process-wide object regularly saved to a JSON file, -// or a Redis equivalent (for multi-process / when the filesystem is unreliable. -let redis -let useRedis = false -if (process.env.REDISTOGO_URL) { - const { port, hostname, password } = new URL(process.env.REDISTOGO_URL) - redis = require('redis').createClient(port, hostname) - redis.auth(password) - useRedis = true -} - -let analytics = {} -let autosaveIntervalId - -const analyticsPath = process.env.SHIELDS_ANALYTICS_FILE || './analytics.json' - -function performAutosave() { - const contents = JSON.stringify(analytics) - if (useRedis) { - redis.set(analyticsPath, contents) - } else { - fs.writeFileSync(analyticsPath, contents) - } -} - -function scheduleAutosaving() { - const analyticsAutoSavePeriod = 10000 - autosaveIntervalId = setInterval(performAutosave, analyticsAutoSavePeriod) -} - -// For a clean shutdown. -function cancelAutosaving() { - if (autosaveIntervalId) { - clearInterval(autosaveIntervalId) - autosaveIntervalId = null - } - performAutosave() -} - -function defaultAnalytics() { - const analytics = Object.create(null) - // In case something happens on the 36th. - analytics.vendorMonthly = new Array(36) - resetMonthlyAnalytics(analytics.vendorMonthly) - analytics.rawMonthly = new Array(36) - resetMonthlyAnalytics(analytics.rawMonthly) - analytics.vendorFlatMonthly = new Array(36) - resetMonthlyAnalytics(analytics.vendorFlatMonthly) - analytics.rawFlatMonthly = new Array(36) - resetMonthlyAnalytics(analytics.rawFlatMonthly) - analytics.vendorFlatSquareMonthly = new Array(36) - resetMonthlyAnalytics(analytics.vendorFlatSquareMonthly) - analytics.rawFlatSquareMonthly = new Array(36) - resetMonthlyAnalytics(analytics.rawFlatSquareMonthly) - return analytics -} - -function load() { - const defaultAnalyticsObject = defaultAnalytics() - if (useRedis) { - redis.get(analyticsPath, (err, value) => { - if (err == null && value != null) { - // if/try/return trick: - // if error, then the rest of the function is run. - try { - analytics = JSON.parse(value) - // Extend analytics with a new value. - for (const key in defaultAnalyticsObject) { - if (!(key in analytics)) { - analytics[key] = defaultAnalyticsObject[key] - } - } - return - } catch (e) { - console.error('Invalid Redis analytics, resetting.') - console.error(e) - } - } - analytics = defaultAnalyticsObject - }) - } else { - // Not using Redis. - try { - analytics = JSON.parse(fs.readFileSync(analyticsPath)) - // Extend analytics with a new value. - for (const key in defaultAnalyticsObject) { - if (!(key in analytics)) { - analytics[key] = defaultAnalyticsObject[key] - } - } - } catch (e) { - if (e.code !== 'ENOENT') { - console.error('Invalid JSON file for analytics, resetting.') - console.error(e) - } - analytics = defaultAnalyticsObject - } - } -} - -let lastDay = new Date().getDate() -function resetMonthlyAnalytics(monthlyAnalytics) { - for (let i = 0; i < monthlyAnalytics.length; i++) { - monthlyAnalytics[i] = 0 - } -} -function incrMonthlyAnalytics(monthlyAnalytics) { - try { - const currentDay = new Date().getDate() - // If we changed month, reset empty days. - while (lastDay !== currentDay) { - // Assumption: at least a hit a month. - lastDay = (lastDay + 1) % monthlyAnalytics.length - monthlyAnalytics[lastDay] = 0 - } - monthlyAnalytics[currentDay]++ - } catch (e) { - console.error(e.stack) - } -} - -function noteRequest(queryParams, match) { - incrMonthlyAnalytics(analytics.vendorMonthly) - if (queryParams.style === 'flat') { - incrMonthlyAnalytics(analytics.vendorFlatMonthly) - } else if (queryParams.style === 'flat-square') { - incrMonthlyAnalytics(analytics.vendorFlatSquareMonthly) - } -} - -function setRoutes(server) { - server.ajax.on('analytics/v1', (json, end) => { - end(analytics) - }) -} - -module.exports = { - load, - scheduleAutosaving, - cancelAutosaving, - noteRequest, - setRoutes, -} diff --git a/core/server/server.js b/core/server/server.js index 8c79e958b86cc..f175e112f2b90 100644 --- a/core/server/server.js +++ b/core/server/server.js @@ -18,7 +18,6 @@ const { } = require('../base-service/legacy-request-handler') const { clearRegularUpdateCache } = require('../legacy/regular-update') const { staticBadgeUrl } = require('../badge-urls/make-badge-url') -const analytics = require('./analytics') const log = require('./log') const sysMonitor = require('./monitor') const PrometheusMetrics = require('./prometheus-metrics') @@ -252,10 +251,6 @@ module.exports = class Server { key, })) - analytics.load() - analytics.scheduleAutosaving() - analytics.setRoutes(camp) - this.cleanupMonitor = sysMonitor.setRoutes({ rateLimit }, camp) const { githubConstellation, metrics } = this @@ -304,7 +299,5 @@ module.exports = class Server { if (this.metrics) { this.metrics.stop() } - - analytics.cancelAutosaving() } } diff --git a/core/server/server.spec.js b/core/server/server.spec.js index 88ecaaf87c8a5..8f29f2e593a7a 100644 --- a/core/server/server.spec.js +++ b/core/server/server.spec.js @@ -9,7 +9,6 @@ const isPng = require('is-png') const isSvg = require('is-svg') const sinon = require('sinon') const portfinder = require('portfinder') -const Joi = require('joi') const svg2img = require('../../gh-badges/lib/svg-to-img') const { createTestServer } = require('./in-process-server-test-helpers') @@ -126,33 +125,4 @@ describe('The server', function() { expect(await res.text()).to.include(expectedError) }) }) - - describe('analytics endpoint', function() { - it('should return analytics in the expected format', async function() { - const countSchema = Joi.array() - .items( - Joi.number() - .integer() - .min(0) - .required() - ) - .length(36) - .required() - const analyticsSchema = Joi.object({ - vendorMonthly: countSchema, - rawMonthly: countSchema, - vendorFlatMonthly: countSchema, - rawFlatMonthly: countSchema, - vendorFlatSquareMonthly: countSchema, - rawFlatSquareMonthly: countSchema, - }).required() - - const res = await fetch(`${baseUrl}$analytics/v1`) - expect(res.ok).to.be.true - - const json = await res.json() - - Joi.assert(json, analyticsSchema) - }) - }) }) diff --git a/doc/code-walkthrough.md b/doc/code-walkthrough.md index 3a5ff649e1ee5..f9ea519d7997d 100644 --- a/doc/code-walkthrough.md +++ b/doc/code-walkthrough.md @@ -82,8 +82,8 @@ test this kind of logic through unit tests (e.g. of `render()` and 2. The Server, which is defined in [`core/server/server.js`][core/server/server], is based on the web framework [Scoutcamp][]. It creates an http server, sets up helpers for - analytics, token persistence, and monitoring. Then it loads all the - services, injecting dependencies as it asks each one to register its route + token persistence and monitoring. Then it loads all the services, + injecting dependencies as it asks each one to register its route with Scoutcamp. 3. The service registration continues in `BaseService.register`. From its diff --git a/doc/production-hosting.md b/doc/production-hosting.md index 65f3555f95bb1..ef4ff53e2eb54 100644 --- a/doc/production-hosting.md +++ b/doc/production-hosting.md @@ -65,8 +65,7 @@ Shields has mercifully little persistent state: 1. The GitHub tokens we collect are saved on each server in JSON files on disk. They can be fetched from the [GitHub auth admin endpoint][] for debugging. -2. The analytics data is also saved on each server in JSON files on disk. -3. The server keeps a few caches in memory. These are neither persisted nor +2. The server keeps a few caches in memory. These are neither persisted nor inspectable. - The [request cache][] - The [regular-update cache][] @@ -214,6 +213,9 @@ the server. It's generously donated by [Sentry][sentry home]. We bundle ## Monitoring +Overall server performance and requests by service are monitored using +[Prometheus and Grafana][metrics]. + Request performance is monitored in two places: - [Status][] (using [UptimeRobot][]) @@ -221,24 +223,13 @@ Request performance is monitored in two places: - [@RedSparr0w's monitor][monitor] which posts [notifications][] to a private [#monitor chat room][monitor discord] -Overall server performance is monitored using Prometheus and Grafana. -Coming soon! ([#2068][issue 2068]) - +[metrics]: https://metrics.shields.io/ [status]: https://status.shields.io/ [server metrics]: https://metrics.shields.io/ [uptimerobot]: https://uptimerobot.com/ [monitor]: https://shields.redsparr0w.com/1568/ [notifications]: http://shields.redsparr0w.com/discord_notification [monitor discord]: https://discordapp.com/channels/308323056592486420/470700909182320646 -[issue 2068]: https://github.com/badges/shields/issues/2068 - -## Analytics - -The server analytics data is public and can be fetched from the -[analytics endpoint][] or using the [analytics script][]. - -[analytics endpoint]: https://github.com/badges/shields/blob/master/lib/analytics.js -[analytics script]: https://github.com/badges/ServerScript/blob/master/stats.js ## Known limitations diff --git a/now.json b/now.json index 40cdc71a81947..517a49a9a6027 100644 --- a/now.json +++ b/now.json @@ -2,8 +2,7 @@ "version": 1, "name": "shields", "env": { - "PERSISTENCE_DIR": "/tmp/persistence", - "SHIELDS_ANALYTICS_FILE": "/tmp/analytics.json" + "PERSISTENCE_DIR": "/tmp/persistence" }, "type": "npm", "engines": {