mirror of
https://github.com/overleaf/overleaf.git
synced 2024-12-24 05:01:11 +00:00
Merge pull request #16731 from overleaf/dp-ip-rate-metrics
Add tracking of rate limit method to metrics GitOrigin-RevId: 3996c2a0ccb747018571ce402120be46fc52eace
This commit is contained in:
parent
a669228aca
commit
32d2603adb
8 changed files with 22 additions and 16 deletions
|
@ -69,7 +69,7 @@ const CollaboratorsInviteController = {
|
|||
const maxRequests = 10 * collabLimit
|
||||
const points = Math.floor(RATE_LIMIT_POINTS / maxRequests)
|
||||
try {
|
||||
await rateLimiter.consume(userId, points)
|
||||
await rateLimiter.consume(userId, points, { method: 'userId' })
|
||||
} catch (err) {
|
||||
if (err instanceof Error) {
|
||||
throw err
|
||||
|
|
|
@ -281,7 +281,7 @@ module.exports = CompileController = {
|
|||
const projectId = req.params.Project_id
|
||||
const rateLimit = function (callback) {
|
||||
pdfDownloadRateLimiter
|
||||
.consume(req.ip)
|
||||
.consume(req.ip, 1, { method: 'ip' })
|
||||
.then(() => {
|
||||
callback(null, true)
|
||||
})
|
||||
|
|
|
@ -310,7 +310,7 @@ module.exports = CompileManager = {
|
|||
Metrics.inc(`auto-compile-${compileGroup}`)
|
||||
const rateLimiter = getAutoCompileRateLimiter(compileGroup)
|
||||
rateLimiter
|
||||
.consume('global')
|
||||
.consume('global', 1, { method: 'global' })
|
||||
.then(() => {
|
||||
callback(null, true)
|
||||
})
|
||||
|
|
|
@ -116,7 +116,7 @@ async function checkCanSendEmail(options) {
|
|||
return true
|
||||
}
|
||||
try {
|
||||
await rateLimiter.consume(options.sendingUser_id)
|
||||
await rateLimiter.consume(options.sendingUser_id, 1, { method: 'userId' })
|
||||
} catch (err) {
|
||||
if (err instanceof Error) {
|
||||
throw err
|
||||
|
|
|
@ -8,7 +8,7 @@ const rateLimiter = new RateLimiter('login', {
|
|||
|
||||
function processLoginRequest(email, callback) {
|
||||
rateLimiter
|
||||
.consume(email)
|
||||
.consume(email, 1, { method: 'email' })
|
||||
.then(() => {
|
||||
callback(null, true)
|
||||
})
|
||||
|
|
|
@ -16,32 +16,33 @@ const settings = require('@overleaf/settings')
|
|||
* will rate limit each project_id separately.
|
||||
*
|
||||
* Unique clients are identified by user_id if logged in, and IP address if not.
|
||||
* The method label is used to identify this in our metrics.
|
||||
*/
|
||||
function rateLimit(rateLimiter, opts = {}) {
|
||||
const getUserId =
|
||||
opts.getUserId || (req => SessionManager.getLoggedInUserId(req.session))
|
||||
return function (req, res, next) {
|
||||
const userId = getUserId(req) || req.ip
|
||||
const clientId = opts.ipOnly ? req.ip : getUserId(req) || req.ip
|
||||
const method = clientId === req.ip ? 'ip' : 'userId'
|
||||
|
||||
if (
|
||||
settings.smokeTest &&
|
||||
settings.smokeTest.userId &&
|
||||
settings.smokeTest.userId.toString() === userId.toString()
|
||||
settings.smokeTest.userId.toString() === clientId.toString()
|
||||
) {
|
||||
// ignore smoke test user
|
||||
return next()
|
||||
}
|
||||
|
||||
let key
|
||||
if (opts.ipOnly) {
|
||||
key = req.ip
|
||||
} else {
|
||||
let key = clientId
|
||||
if (!opts.ipOnly) {
|
||||
const params = (opts.params || []).map(p => req.params[p])
|
||||
params.push(userId)
|
||||
params.push(clientId)
|
||||
key = params.join(':')
|
||||
}
|
||||
|
||||
rateLimiter
|
||||
.consume(key)
|
||||
.consume(key, 1, { method })
|
||||
.then(() => next())
|
||||
.catch(err => {
|
||||
if (err instanceof Error) {
|
||||
|
|
|
@ -264,7 +264,9 @@ async function resendInvite(req, res, next) {
|
|||
}
|
||||
|
||||
try {
|
||||
await rateLimiters.resendGroupInvite.consume(userEmail)
|
||||
await rateLimiters.resendGroupInvite.consume(userEmail, 1, {
|
||||
method: 'email',
|
||||
})
|
||||
|
||||
const existingUser = await UserGetter.promises.getUserByAnyEmail(userEmail)
|
||||
|
||||
|
|
|
@ -34,7 +34,7 @@ class RateLimiter {
|
|||
})
|
||||
}
|
||||
|
||||
async consume(key, points = 1, options = {}) {
|
||||
async consume(key, points = 1, options = { method: 'unknown' }) {
|
||||
if (Settings.disableRateLimits) {
|
||||
// Return a fake result in case it's used somewhere
|
||||
return {
|
||||
|
@ -57,7 +57,10 @@ class RateLimiter {
|
|||
if (err.consumedPoints - points <= this._rateLimiter.points) {
|
||||
logger.warn({ path: this.name, key }, 'rate limit exceeded')
|
||||
}
|
||||
Metrics.inc('rate-limit-hit', 1, { path: this.name })
|
||||
Metrics.inc('rate-limit-hit', 1, {
|
||||
path: this.name,
|
||||
method: options.method,
|
||||
})
|
||||
throw err
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue