From 1d1106bc679e344653c5feef96dca4a3124d6a38 Mon Sep 17 00:00:00 2001
From: Simon Detheridge <s@sd.ai>
Date: Thu, 5 Dec 2019 15:25:35 +0000
Subject: [PATCH] Add metric for s3 egress

---
 .../app/coffee/S3PersistorManager.coffee      |  5 +++
 services/filestore/docker-compose.ci.yml      |  1 +
 services/filestore/docker-compose.yml         |  1 +
 .../acceptance/coffee/SendingFileTest.coffee  | 33 ++++++++++++++-----
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee
index 5ee9e25865..14ab1bffe5 100644
--- a/services/filestore/app/coffee/S3PersistorManager.coffee
+++ b/services/filestore/app/coffee/S3PersistorManager.coffee
@@ -9,6 +9,7 @@ https.globalAgent.maxSockets = 300
 settings = require("settings-sharelatex")
 request = require("request")
 logger = require("logger-sharelatex")
+metrics = require("metrics-sharelatex")
 fs = require("fs")
 knox = require("knox")
 path = require("path")
@@ -73,7 +74,9 @@ module.exports =
 
 	sendFile: (bucketName, key, fsPath, callback)->
 		s3Client = getKnoxClient(bucketName)
+		uploaded = 0
 		putEventEmiter = s3Client.putFile fsPath, key, (err, res)->
+			metrics.count 's3.egress', uploaded
 			if err?
 				logger.err err:err,  bucketName:bucketName, key:key, fsPath:fsPath,"something went wrong uploading file to s3"
 				return callback(err)
@@ -88,6 +91,8 @@ module.exports =
 		putEventEmiter.on "error", (err)->
 			logger.err err:err,  bucketName:bucketName, key:key, fsPath:fsPath, "error emmited on put of file"
 			callback err
+		putEventEmiter.on "progress", (progress)->
+			uploaded = progress.written
 
 	sendStream: (bucketName, key, readStream, callback)->
 		logger.log bucketName:bucketName, key:key, "sending file to s3"
diff --git a/services/filestore/docker-compose.ci.yml b/services/filestore/docker-compose.ci.yml
index 790109f70b..d7b97ac011 100644
--- a/services/filestore/docker-compose.ci.yml
+++ b/services/filestore/docker-compose.ci.yml
@@ -25,6 +25,7 @@ services:
       ENABLE_CONVERSIONS: "true"
       MOCHA_GREP: ${MOCHA_GREP}
       NODE_ENV: test
+      USE_PROM_METRICS: "true"
       AWS_KEY: fake
       AWS_SECRET: fake
       AWS_S3_USER_FILES_BUCKET_NAME: fake_user_files
diff --git a/services/filestore/docker-compose.yml b/services/filestore/docker-compose.yml
index cd03a9daf4..c2c2ed565d 100644
--- a/services/filestore/docker-compose.yml
+++ b/services/filestore/docker-compose.yml
@@ -31,6 +31,7 @@ services:
       ENABLE_CONVERSIONS: "true"
       LOG_LEVEL: ERROR
       NODE_ENV: test
+      USE_PROM_METRICS: "true"
       AWS_KEY: fake
       AWS_SECRET: fake
       AWS_S3_USER_FILES_BUCKET_NAME: fake_user_files
diff --git a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
index b77afb866b..fb03697cb0 100644
--- a/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
+++ b/services/filestore/test/acceptance/coffee/SendingFileTest.coffee
@@ -10,8 +10,14 @@ request = require("request")
 settings = require("settings-sharelatex")
 FilestoreApp = require "./FilestoreApp"
 
-describe "Filestore", ->
 
+getMetric = (filestoreUrl, metric, cb) ->
+	request.get "#{filestoreUrl}/metrics", (err, res) ->
+		expect(res.statusCode).to.equal 200
+		metricRegex = new RegExp("^#{metric}{[^}]+} ([0-9]+)$", "m")
+		cb(parseInt(metricRegex.exec(res.body)?[1] || '0'))
+
+describe "Filestore", ->
 	before (done)->
 		@localFileReadPath = "/tmp/filestore_acceptence_tests_file_read.txt"
 		@localFileWritePath = "/tmp/filestore_acceptence_tests_file_write.txt"
@@ -27,10 +33,10 @@ describe "Filestore", ->
 
 	beforeEach (done)->
 		FilestoreApp.ensureRunning =>
-			fs.unlink @localFileWritePath, ->
-				done()
-
-
+			fs.unlink @localFileWritePath, =>
+				getMetric @filestoreUrl, 's3_egress', (metric) =>
+					@previousEgress = metric
+					done()
 
 	it "should send a 200 for status endpoint", (done)->
 		request "#{@filestoreUrl}/status", (err, response, body)->
@@ -59,6 +65,11 @@ describe "Filestore", ->
 				response.statusCode.should.equal 404
 				done()
 
+		it 'should record an egress metric for the upload', (done) ->
+			getMetric @filestoreUrl, 's3_egress', (metric) =>
+				expect(metric - @previousEgress).to.equal @constantFileContent.length
+				done()
+
 		it "should return the file size on a HEAD request", (done) ->
 			expectedLength = Buffer.byteLength(@constantFileContent)
 			request.head @fileUrl, (err, res) =>
@@ -128,11 +139,17 @@ describe "Filestore", ->
 			@file_id = Math.random()
 			@fileUrl = "#{@filestoreUrl}/project/acceptence_tests/file/#{@file_id}"
 			@localFileReadPath = __dirname + '/../../fixtures/test.pdf'
+			fs.stat @localFileReadPath, (err, stat) =>
+				@localFileSize = stat.size
+				writeStream = request.post(@fileUrl)
 
-			writeStream = request.post(@fileUrl)
+				writeStream.on "end", done
+				fs.createReadStream(@localFileReadPath).pipe writeStream
 
-			writeStream.on "end", done
-			fs.createReadStream(@localFileReadPath).pipe writeStream
+		it 'should record an egress metric for the upload', (done) ->
+			getMetric @filestoreUrl, 's3_egress', (metric) =>
+				expect(metric - @previousEgress).to.equal @localFileSize
+				done()
 
 		it "should be able get the file back", (done)->
 			@timeout(1000 * 10)