mirror of
https://github.com/yu-i-i/overleaf-cep.git
synced 2026-11-12 13:13:45 +01:00
remove sentry from backend services (#20752)
* remove sentry from backend services - no longer required * Remove Sentry integration from logging manager * Remove Sentry from clsi default settings * Remove `initializeErrorReporting` in libraries/logger * Remove `@sentry/node` from `libraries/logger` --------- Co-authored-by: Antoine Clausse <antoine.clausse@overleaf.com> GitOrigin-RevId: 8149a885f5258804b93ae39cde7b7333e992532a
This commit is contained in:
committed by
Copybot
co-authored by
Antoine Clausse
parent
3d0f5188ae
commit
6672372828
@@ -1,7 +1,6 @@
|
||||
const Stream = require('node:stream')
|
||||
const bunyan = require('bunyan')
|
||||
const GCPManager = require('./gcp-manager')
|
||||
const SentryManager = require('./sentry-manager')
|
||||
const Serializers = require('./serializers')
|
||||
const {
|
||||
FileLogLevelChecker,
|
||||
@@ -33,10 +32,6 @@ const LoggingManager = {
|
||||
return this
|
||||
},
|
||||
|
||||
initializeErrorReporting(dsn, options) {
|
||||
this.sentryManager = new SentryManager()
|
||||
},
|
||||
|
||||
/**
|
||||
* @param {Record<string, any>|string} attributes - Attributes to log (nice serialization for err, req, res)
|
||||
* @param {string} [message] - Optional message
|
||||
@@ -68,9 +63,6 @@ const LoggingManager = {
|
||||
})
|
||||
}
|
||||
this.logger.error(attributes, message, ...Array.from(args))
|
||||
if (this.sentryManager) {
|
||||
this.sentryManager.captureExceptionRateLimited(attributes, message)
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
@@ -98,9 +90,6 @@ const LoggingManager = {
|
||||
*/
|
||||
fatal(attributes, message) {
|
||||
this.logger.fatal(attributes, message)
|
||||
if (this.sentryManager) {
|
||||
this.sentryManager.captureException(attributes, message, 'fatal')
|
||||
}
|
||||
},
|
||||
|
||||
_getOutputStreamConfig() {
|
||||
|
||||
@@ -23,7 +23,6 @@
|
||||
"@google-cloud/logging-bunyan": "^5.1.0",
|
||||
"@overleaf/fetch-utils": "*",
|
||||
"@overleaf/o-error": "*",
|
||||
"@sentry/node": "^6.13.2",
|
||||
"bunyan": "^1.8.14"
|
||||
},
|
||||
"devDependencies": {
|
||||
|
||||
@@ -1,106 +0,0 @@
|
||||
const Serializers = require('./serializers')
|
||||
const RATE_LIMIT_MAX_ERRORS = 5
|
||||
const RATE_LIMIT_INTERVAL_MS = 60000
|
||||
|
||||
class SentryManager {
|
||||
constructor(dsn, options) {
|
||||
this.Sentry = require('@sentry/node')
|
||||
this.Sentry.init({ dsn, ...options })
|
||||
// for rate limiting on sentry reporting
|
||||
this.lastErrorTimeStamp = 0
|
||||
this.lastErrorCount = 0
|
||||
}
|
||||
|
||||
captureExceptionRateLimited(attributes, message) {
|
||||
const now = Date.now()
|
||||
// have we recently reported an error?
|
||||
const recentSentryReport =
|
||||
now - this.lastErrorTimeStamp < RATE_LIMIT_INTERVAL_MS
|
||||
// if so, increment the error count
|
||||
if (recentSentryReport) {
|
||||
this.lastErrorCount++
|
||||
} else {
|
||||
this.lastErrorCount = 0
|
||||
this.lastErrorTimeStamp = now
|
||||
}
|
||||
// only report 5 errors every minute to avoid overload
|
||||
if (this.lastErrorCount < RATE_LIMIT_MAX_ERRORS) {
|
||||
// add a note if the rate limit has been hit
|
||||
const note =
|
||||
this.lastErrorCount + 1 === RATE_LIMIT_MAX_ERRORS
|
||||
? '(rate limited)'
|
||||
: ''
|
||||
// report the exception
|
||||
this.captureException(attributes, message, `error${note}`)
|
||||
}
|
||||
}
|
||||
|
||||
captureException(attributes, message, level) {
|
||||
// handle case of logger.error "message"
|
||||
if (typeof attributes === 'string') {
|
||||
attributes = { err: new Error(attributes) }
|
||||
}
|
||||
|
||||
// extract any error object
|
||||
let error = Serializers.err(attributes.err || attributes.error)
|
||||
|
||||
// avoid reporting errors twice
|
||||
for (const key in attributes) {
|
||||
const value = attributes[key]
|
||||
if (value instanceof Error && value.reportedToSentry) {
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// include our log message in the error report
|
||||
if (error == null) {
|
||||
if (typeof message === 'string') {
|
||||
error = { message }
|
||||
}
|
||||
} else if (message != null) {
|
||||
attributes.description = message
|
||||
}
|
||||
|
||||
// report the error
|
||||
if (error != null) {
|
||||
// capture attributes and use *_id objects as tags
|
||||
const tags = {}
|
||||
const extra = {}
|
||||
for (const key in attributes) {
|
||||
let value = attributes[key]
|
||||
if (Serializers[key]) {
|
||||
value = Serializers[key](value)
|
||||
}
|
||||
if (key.match(/_id/) && typeof value === 'string') {
|
||||
tags[key] = value
|
||||
}
|
||||
extra[key] = value
|
||||
}
|
||||
|
||||
// OError integration
|
||||
extra.info = error.info
|
||||
delete error.info
|
||||
|
||||
// Sentry wants to receive an Error instance.
|
||||
const errInstance = new Error(error.message)
|
||||
Object.assign(errInstance, error)
|
||||
|
||||
try {
|
||||
// send the error to sentry
|
||||
this.Sentry.captureException(errInstance, { tags, extra, level })
|
||||
|
||||
// put a flag on the errors to avoid reporting them multiple times
|
||||
for (const key in attributes) {
|
||||
const value = attributes[key]
|
||||
if (value instanceof Error) {
|
||||
value.reportedToSentry = true
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
// ignore Sentry errors
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = SentryManager
|
||||
@@ -43,20 +43,14 @@ describe('LoggingManager', function () {
|
||||
.stub()
|
||||
.returns(this.GCEMetadataLogLevelChecker),
|
||||
}
|
||||
this.SentryManager = {
|
||||
captureException: sinon.stub(),
|
||||
captureExceptionRateLimited: sinon.stub(),
|
||||
}
|
||||
this.LoggingManager = SandboxedModule.require(MODULE_PATH, {
|
||||
requires: {
|
||||
bunyan: this.Bunyan,
|
||||
'./log-level-checker': this.LogLevelChecker,
|
||||
'./sentry-manager': sinon.stub().returns(this.SentryManager),
|
||||
},
|
||||
})
|
||||
this.loggerName = 'test'
|
||||
this.logger = this.LoggingManager.initialize(this.loggerName)
|
||||
this.logger.initializeErrorReporting('test_dsn')
|
||||
})
|
||||
|
||||
afterEach(function () {
|
||||
@@ -160,13 +154,6 @@ describe('LoggingManager', function () {
|
||||
})
|
||||
})
|
||||
|
||||
describe('logger.error', function () {
|
||||
it('should report errors to Sentry', function () {
|
||||
this.logger.error({ foo: 'bar' }, 'message')
|
||||
expect(this.SentryManager.captureExceptionRateLimited).to.have.been.called
|
||||
})
|
||||
})
|
||||
|
||||
describe('ringbuffer', function () {
|
||||
beforeEach(function () {
|
||||
this.logBufferMock = [
|
||||
|
||||
@@ -1,247 +0,0 @@
|
||||
const Path = require('node:path')
|
||||
const SandboxedModule = require('sandboxed-module')
|
||||
const { expect } = require('chai')
|
||||
const sinon = require('sinon')
|
||||
|
||||
const MODULE_PATH = Path.join(__dirname, '../../sentry-manager.js')
|
||||
|
||||
describe('SentryManager', function () {
|
||||
beforeEach(function () {
|
||||
this.clock = sinon.useFakeTimers(Date.now())
|
||||
this.Sentry = {
|
||||
init: sinon.stub(),
|
||||
captureException: sinon.stub(),
|
||||
}
|
||||
this.SentryManager = SandboxedModule.require(MODULE_PATH, {
|
||||
requires: {
|
||||
'@sentry/node': this.Sentry,
|
||||
},
|
||||
})
|
||||
this.sentryManager = new this.SentryManager('test_dsn')
|
||||
})
|
||||
|
||||
afterEach(function () {
|
||||
this.clock.restore()
|
||||
})
|
||||
|
||||
describe('captureExceptionRateLimited', function () {
|
||||
it('should report a single error to sentry', function () {
|
||||
this.sentryManager.captureExceptionRateLimited({ foo: 'bar' }, 'message')
|
||||
expect(this.Sentry.captureException).to.have.been.calledOnce
|
||||
})
|
||||
|
||||
it('should report the same error to sentry only once', function () {
|
||||
const error1 = new Error('this is the error')
|
||||
this.sentryManager.captureExceptionRateLimited(
|
||||
{ foo: error1 },
|
||||
'first message'
|
||||
)
|
||||
this.sentryManager.captureExceptionRateLimited(
|
||||
{ bar: error1 },
|
||||
'second message'
|
||||
)
|
||||
expect(this.Sentry.captureException).to.have.been.calledOnce
|
||||
})
|
||||
|
||||
it('should report two different errors to sentry individually', function () {
|
||||
const error1 = new Error('this is the error')
|
||||
const error2 = new Error('this is the error')
|
||||
this.sentryManager.captureExceptionRateLimited(
|
||||
{ foo: error1 },
|
||||
'first message'
|
||||
)
|
||||
this.sentryManager.captureExceptionRateLimited(
|
||||
{ bar: error2 },
|
||||
'second message'
|
||||
)
|
||||
expect(this.Sentry.captureException).to.have.been.calledTwice
|
||||
})
|
||||
|
||||
it('for multiple errors should only report a maximum of 5 errors to sentry', function () {
|
||||
for (let i = 0; i < 10; i++) {
|
||||
this.sentryManager.captureExceptionRateLimited(
|
||||
{ foo: 'bar' },
|
||||
'message'
|
||||
)
|
||||
}
|
||||
expect(this.Sentry.captureException).to.have.callCount(5)
|
||||
})
|
||||
|
||||
it('for multiple errors with a minute delay should report 10 errors to sentry', function () {
|
||||
for (let i = 0; i < 10; i++) {
|
||||
this.sentryManager.captureExceptionRateLimited(
|
||||
{ foo: 'bar' },
|
||||
'message'
|
||||
)
|
||||
}
|
||||
expect(this.Sentry.captureException).to.have.callCount(5)
|
||||
|
||||
// allow a minute to pass
|
||||
this.clock.tick(61 * 1000)
|
||||
|
||||
for (let i = 0; i < 10; i++) {
|
||||
this.sentryManager.captureExceptionRateLimited(
|
||||
{ foo: 'bar' },
|
||||
'message'
|
||||
)
|
||||
}
|
||||
expect(this.Sentry.captureException).to.have.callCount(10)
|
||||
})
|
||||
})
|
||||
|
||||
describe('captureException', function () {
|
||||
it('should remove the path from fs errors', function () {
|
||||
const fsError = new Error(
|
||||
"Error: ENOENT: no such file or directory, stat '/tmp/3279b8d0-da10-11e8-8255-efd98985942b'"
|
||||
)
|
||||
fsError.path = '/tmp/3279b8d0-da10-11e8-8255-efd98985942b'
|
||||
this.sentryManager.captureException({ err: fsError }, 'message', 'error')
|
||||
expect(this.Sentry.captureException).to.have.been.calledWith(
|
||||
sinon.match.has(
|
||||
'message',
|
||||
'Error: ENOENT: no such file or directory, stat'
|
||||
)
|
||||
)
|
||||
})
|
||||
|
||||
it('should sanitize error', function () {
|
||||
const err = {
|
||||
name: 'CustomError',
|
||||
message: 'hello',
|
||||
_oErrorTags: [{ stack: 'here:1', info: { one: 1 } }],
|
||||
stack: 'here:0',
|
||||
info: { key: 'value' },
|
||||
code: 42,
|
||||
signal: 9,
|
||||
path: '/foo',
|
||||
}
|
||||
this.sentryManager.captureException({ err }, 'message', 'error')
|
||||
const expectedErr = {
|
||||
name: 'CustomError',
|
||||
message: 'hello',
|
||||
stack: 'here:0\nhere:1',
|
||||
code: 42,
|
||||
signal: 9,
|
||||
path: '/foo',
|
||||
}
|
||||
expect(this.Sentry.captureException).to.have.been.calledWith(
|
||||
sinon.match(expectedErr),
|
||||
sinon.match({
|
||||
tags: sinon.match.any,
|
||||
level: sinon.match.any,
|
||||
extra: {
|
||||
description: 'message',
|
||||
info: sinon.match({
|
||||
one: 1,
|
||||
key: 'value',
|
||||
}),
|
||||
},
|
||||
})
|
||||
)
|
||||
// Chai is very picky with comparing Error instances. Go the long way of comparing all the fields manually.
|
||||
const gotErr = this.Sentry.captureException.args[0][0]
|
||||
for (const [key, wanted] of Object.entries(expectedErr)) {
|
||||
expect(gotErr).to.have.property(key, wanted)
|
||||
}
|
||||
})
|
||||
it('should sanitize request', function () {
|
||||
const req = {
|
||||
ip: '1.2.3.4',
|
||||
method: 'GET',
|
||||
url: '/foo',
|
||||
headers: {
|
||||
referer: 'abc',
|
||||
'content-length': 1337,
|
||||
'user-agent': 'curl',
|
||||
authorization: '42',
|
||||
},
|
||||
}
|
||||
this.sentryManager.captureException({ req }, 'message', 'error')
|
||||
const expectedReq = {
|
||||
remoteAddress: '1.2.3.4',
|
||||
method: 'GET',
|
||||
url: '/foo',
|
||||
headers: {
|
||||
referer: 'abc',
|
||||
'content-length': 1337,
|
||||
'user-agent': 'curl',
|
||||
},
|
||||
}
|
||||
expect(this.Sentry.captureException).to.have.been.calledWith(
|
||||
sinon.match({
|
||||
message: 'message',
|
||||
}),
|
||||
sinon.match({
|
||||
tags: sinon.match.any,
|
||||
level: sinon.match.any,
|
||||
extra: {
|
||||
info: sinon.match.any,
|
||||
req: expectedReq,
|
||||
},
|
||||
})
|
||||
)
|
||||
expect(this.Sentry.captureException.args[0][1].extra.req).to.deep.equal(
|
||||
expectedReq
|
||||
)
|
||||
})
|
||||
it('should sanitize response', function () {
|
||||
const res = {
|
||||
statusCode: 417,
|
||||
body: Buffer.from('foo'),
|
||||
getHeader(key) {
|
||||
expect(key).to.be.oneOf(['content-length'])
|
||||
if (key === 'content-length') return 1337
|
||||
},
|
||||
}
|
||||
this.sentryManager.captureException({ res }, 'message', 'error')
|
||||
const expectedRes = {
|
||||
statusCode: 417,
|
||||
headers: {
|
||||
'content-length': 1337,
|
||||
},
|
||||
}
|
||||
expect(this.Sentry.captureException).to.have.been.calledWith(
|
||||
sinon.match({
|
||||
message: 'message',
|
||||
}),
|
||||
sinon.match({
|
||||
tags: sinon.match.any,
|
||||
level: sinon.match.any,
|
||||
extra: {
|
||||
info: sinon.match.any,
|
||||
res: expectedRes,
|
||||
},
|
||||
})
|
||||
)
|
||||
expect(this.Sentry.captureException.args[0][1].extra.res).to.deep.equal(
|
||||
expectedRes
|
||||
)
|
||||
})
|
||||
|
||||
describe('reportedToSentry', function () {
|
||||
it('should mark the error as reported to sentry', function () {
|
||||
const err = new Error()
|
||||
this.sentryManager.captureException({ err }, 'message')
|
||||
expect(this.Sentry.captureException).to.have.been.called
|
||||
expect(err.reportedToSentry).to.equal(true)
|
||||
})
|
||||
|
||||
it('should mark two errors as reported to sentry', function () {
|
||||
const err1 = new Error()
|
||||
const err2 = new Error()
|
||||
this.sentryManager.captureException({ err: err1, err2 }, 'message')
|
||||
expect(this.Sentry.captureException).to.have.been.called
|
||||
expect(err1.reportedToSentry).to.equal(true)
|
||||
expect(err2.reportedToSentry).to.equal(true)
|
||||
})
|
||||
|
||||
it('should not mark arbitrary objects as reported to sentry', function () {
|
||||
const err = new Error()
|
||||
const ctx = { foo: 'bar' }
|
||||
this.sentryManager.captureException({ err, ctx }, 'message')
|
||||
expect(this.Sentry.captureException).to.have.been.called
|
||||
expect(ctx.reportedToSentry).not.to.exist
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user