Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Add Sentry to batch submitter #480

Merged
merged 8 commits into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/honest-lions-trade.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@eth-optimism/batch-submitter": patch
"@eth-optimism/core-utils": patch
"@eth-optimism/data-transport-layer": patch
---

add Sentry to TypeScript services for error tracking
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ cache-ovm
l2geth/build/bin
packages/contracts/deployments/custom

packages/contracts/data-transport-layer/db
packages/data-transport-layer/db

.env
12 changes: 8 additions & 4 deletions packages/batch-submitter/.env.example
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
# Logging
# Logging & monitoring
DEBUG=info*,error*,warn*,debug*
# Leave the SENTRY_DSN variable unset during local development
SENTRY_DSN=

L1_NODE_WEB3_URL=http://localhost:9545
L2_NODE_WEB3_URL=http://localhost:8545

MAX_TX_SIZE=90000
MIN_TX_SIZE=0
MAX_BATCH_SIZE=50
MAX_L1_TX_SIZE=90000
MIN_L1_TX_SIZE=0
MAX_TX_BATCH_SIZE=50
MAX_STATE_BATCH_COUNT=2000
MAX_TX_BATCH_COUNT=250
MAX_BATCH_SUBMISSION_TIME=0
POLL_INTERVAL=15000
NUM_CONFIRMATIONS=0
Expand Down
8 changes: 5 additions & 3 deletions packages/batch-submitter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@
"url": "https://github.com/ethereum-optimism/optimism-monorepo.git"
},
"dependencies": {
"old-contracts": "npm:@eth-optimism/contracts@^0.0.2-alpha.7",
"@eth-optimism/contracts": "^0.2.2",
"@eth-optimism/core-utils": "^0.2.0",
"@eth-optimism/ynatm": "^0.2.2",
"@ethersproject/abstract-provider": "^5.0.5",
"@ethersproject/providers": "^5.0.14",
"bluebird": "^3.7.2",
"dotenv": "^8.2.0",
"ethers": "5.0.0",
"@eth-optimism/contracts": "^0.2.2"
"old-contracts": "npm:@eth-optimism/contracts@^0.0.2-alpha.7",
annieke marked this conversation as resolved.
Show resolved Hide resolved
"pino-sentry": "^0.6.1"
},
"devDependencies": {
"@eth-optimism/smock": "^1.0.0",
Expand All @@ -64,7 +65,8 @@
"typescript": "^4.2.3"
},
"resolutions": {
"ganache-core": "^2.13.2"
"ganache-core": "^2.13.2",
"**/@sentry/node": "^6.2.5"
},
"publishConfig": {
"access": "public"
Expand Down
7 changes: 6 additions & 1 deletion packages/batch-submitter/src/exec/run-batch-submitter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* External Imports */
import { Logger, injectL2Context } from '@eth-optimism/core-utils'
import { createWriteStream } from 'pino-sentry'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should maybe make this used in core-utils's logger.ts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^yeah this might be a good idea! pino-sentry uses an older version of pino that i can't consistently overwrite, so i wanted to test out the differences between bs (uses this older version with logging) and dtl (uses the express plugins without logging) during this initial phase, while figuring out the versioning headache

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this still isn't resolved: yarnpkg/yarn#4874

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since i can't update to latest sentry, thoughts on keeping dtl on regular sentry for now and testing out pino-sentry through batch submitter? i'm down to tweak this as we go, just don't know what features we might be missing out on yet. i can also make a PR or fork pino-sentry to update

Copy link
Contributor

@gakonst gakonst Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the right approach is to make a PR upstream and update their dep, if not a lot of work? Not a blocker for this PR

Copy link
Contributor Author

@annieke annieke Apr 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import { exit } from 'process'
import { Signer, Wallet } from 'ethers'
import { JsonRpcProvider, TransactionReceipt } from '@ethersproject/providers'
Expand All @@ -16,7 +17,11 @@ import {
} from '..'

/* Logger */
const log = new Logger({ name: 'oe:batch-submitter:init' })
const destination = createWriteStream({
dsn: process.env.SENTRY_DSN,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if this is not set? We need to make sure this works for local development and CI without sending messages to sentry

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far I've tested that Sentry doesn't report errors without a dsn. We won't have pesky alerts, but I've created an issue here to add environment management so we can do away with running these monitoring services at all during localdev: #491

Sorry I missed this comment, if its not set and its a noop then its all good

tracesSampleRate: 0.05,
})
const log = new Logger({ name: 'oe:batch-submitter:init', destination })

interface RequiredEnvVars {
// The HTTP provider URL for L1.
Expand Down
3 changes: 2 additions & 1 deletion packages/core-utils/src/common/logger.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import pino, {
LoggerOptions as PinoLoggerOptions,
DestinationObjectOptions,
DestinationStream,
} from 'pino'

export type LogLevel = 'trace' | 'debug' | 'info' | 'warn' | 'error' | 'fatal'

export interface LoggerOptions {
name: string
level?: LogLevel
destination?: DestinationObjectOptions
destination?: DestinationObjectOptions | DestinationStream
}

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/data-transport-layer/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ DATA_TRANSPORT_LAYER__L2_RPC_ENDPOINT=
DATA_TRANSPORT_LAYER__TRANSACTIONS_PER_POLLING_INTERVAL=1000
DATA_TRANSPORT_LAYER__L2_CHAIN_ID=69
DATA_TRANSPORT_LAYER__LEGACY_SEQUENCER_COMPATIBILITY=false

# Monitoring
# Leave the SENTRY_DSN variable unset during local development
DATA_TRANSPORT_LAYER__SENTRY_DSN=
2 changes: 2 additions & 0 deletions packages/data-transport-layer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
"@eth-optimism/contracts": "^0.2.2",
"@eth-optimism/core-utils": "^0.2.2",
"@ethersproject/providers": "^5.0.21",
"@sentry/node": "^6.2.5",
"@sentry/tracing": "^6.2.5",
"@types/express": "^4.17.11",
"bcfg": "^0.1.6",
"browser-or-node": "^1.3.0",
Expand Down
1 change: 1 addition & 0 deletions packages/data-transport-layer/src/services/main/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export interface L1DataTransportServiceOptions {
transactionsPerPollingInterval: number
legacySequencerCompatibility: boolean
stopL2SyncAtBlock?: number
sentryDsn?: string
}

const optionSettings = {
Expand Down
1 change: 1 addition & 0 deletions packages/data-transport-layer/src/services/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ interface Bcfg {
false
),
stopL2SyncAtBlock: config.uint('stopL2SyncAtBlock'),
sentryDsn: config.str('sentryDsn'),
})

await service.start()
Expand Down
15 changes: 15 additions & 0 deletions packages/data-transport-layer/src/services/server/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import cors from 'cors'
import { BigNumber } from 'ethers'
import { JsonRpcProvider } from '@ethersproject/providers'
import { LevelUp } from 'levelup'
import * as Sentry from '@sentry/node'
import * as Tracing from '@sentry/tracing'

/* Imports: Internal */
import { TransportDB } from '../../db/transport-db'
Expand Down Expand Up @@ -101,8 +103,21 @@ export class L1TransportServer extends BaseService<L1TransportServerOptions> {
private _initializeApp() {
// TODO: Maybe pass this in as a parameter instead of creating it here?
this.state.app = express()
Sentry.init({
dsn: this.options.sentryDsn,
integrations: [
new Sentry.Integrations.Http({ tracing: true }),
new Tracing.Integrations.Express({
app: this.state.app,
}),
],
tracesSampleRate: 0.05,
})
this.state.app.use(Sentry.Handlers.requestHandler())
this.state.app.use(Sentry.Handlers.tracingHandler())
this.state.app.use(cors())
this._registerAllRoutes()
this.state.app.use(Sentry.Handlers.errorHandler())
}

/**
Expand Down
117 changes: 114 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,17 @@
"@sentry/utils" "5.30.0"
tslib "^1.9.3"

"@sentry/[email protected]":
version "6.2.5"
resolved "https://registry.yarnpkg.com/@sentry/core/-/core-6.2.5.tgz#e75093f8598becc0a4a0be927f32f7ac49e8588f"
integrity sha512-I+AkgIFO6sDUoHQticP6I27TT3L+i6TUS03in3IEtpBcSeP2jyhlxI8l/wdA7gsBqUPdQ4GHOOaNgtFIcr8qag==
dependencies:
"@sentry/hub" "6.2.5"
"@sentry/minimal" "6.2.5"
"@sentry/types" "6.2.5"
"@sentry/utils" "6.2.5"
tslib "^1.9.3"

"@sentry/[email protected]":
version "5.30.0"
resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-5.30.0.tgz#2453be9b9cb903404366e198bd30c7ca74cdc100"
Expand All @@ -1742,6 +1753,15 @@
"@sentry/utils" "5.30.0"
tslib "^1.9.3"

"@sentry/[email protected]":
version "6.2.5"
resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-6.2.5.tgz#324cae0c90d736cd1032e94104bf3f18becec4d6"
integrity sha512-YlEFdEhcfqpl2HC+/dWXBsBJEljyMzFS7LRRjCk8QANcOdp9PhwQjwebUB4/ulOBjHPP2WZk7fBBd/IKDasTUg==
dependencies:
"@sentry/types" "6.2.5"
"@sentry/utils" "6.2.5"
tslib "^1.9.3"

"@sentry/[email protected]":
version "5.30.0"
resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-5.30.0.tgz#ce3d3a6a273428e0084adcb800bc12e72d34637b"
Expand All @@ -1751,7 +1771,16 @@
"@sentry/types" "5.30.0"
tslib "^1.9.3"

"@sentry/node@^5.18.1":
"@sentry/[email protected]":
version "6.2.5"
resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-6.2.5.tgz#3e963e868bfa68e97581403521fd4e09a8965b02"
integrity sha512-RKP4Qx3p7Cv0oX1cPKAkNVFYM7p2k1t32cNk1+rrVQS4hwlJ7Eg6m6fsqsO+85jd6Ne/FnyYsfo9cDD3ImTlWQ==
dependencies:
"@sentry/hub" "6.2.5"
"@sentry/types" "6.2.5"
tslib "^1.9.3"

"@sentry/node@^5.18.1", "@sentry/node@^5.21.1":
version "5.30.0"
resolved "https://registry.yarnpkg.com/@sentry/node/-/node-5.30.0.tgz#4ca479e799b1021285d7fe12ac0858951c11cd48"
integrity sha512-Br5oyVBF0fZo6ZS9bxbJZG4ApAjRqAnqFFurMVJJdunNb80brh7a5Qva2kjhm+U6r9NJAB5OmDyPkA1Qnt+QVg==
Expand All @@ -1766,6 +1795,21 @@
lru_map "^0.3.3"
tslib "^1.9.3"

"@sentry/node@^6.2.5":
version "6.2.5"
resolved "https://registry.yarnpkg.com/@sentry/node/-/node-6.2.5.tgz#6e6694c0c3ce6ca231710f40da0cac7fd5c645ef"
integrity sha512-/iM3khzGnUH713VFhZBAEYJhb/saEQSVz7Udogml+O7mFQ4rutnwJhgoGcB9YYrwMv2m7qOSszkdZbemDV6k2g==
dependencies:
"@sentry/core" "6.2.5"
"@sentry/hub" "6.2.5"
"@sentry/tracing" "6.2.5"
"@sentry/types" "6.2.5"
"@sentry/utils" "6.2.5"
cookie "^0.4.1"
https-proxy-agent "^5.0.0"
lru_map "^0.3.3"
tslib "^1.9.3"

"@sentry/[email protected]":
version "5.30.0"
resolved "https://registry.yarnpkg.com/@sentry/tracing/-/tracing-5.30.0.tgz#501d21f00c3f3be7f7635d8710da70d9419d4e1f"
Expand All @@ -1777,11 +1821,27 @@
"@sentry/utils" "5.30.0"
tslib "^1.9.3"

"@sentry/[email protected]", "@sentry/tracing@^6.2.5":
version "6.2.5"
resolved "https://registry.yarnpkg.com/@sentry/tracing/-/tracing-6.2.5.tgz#3f5dadfdccdb5c1fb2eef68458c7c34329b0a34a"
integrity sha512-j/hM0BoHxfrNLxPeEJ5Vq4R34hO/TOHMEpLR3FdnunBXbsmjoKMMygIkPxnpML5XWtvukAehbwpDXldwMYz83w==
dependencies:
"@sentry/hub" "6.2.5"
"@sentry/minimal" "6.2.5"
"@sentry/types" "6.2.5"
"@sentry/utils" "6.2.5"
tslib "^1.9.3"

"@sentry/[email protected]":
version "5.30.0"
resolved "https://registry.yarnpkg.com/@sentry/types/-/types-5.30.0.tgz#19709bbe12a1a0115bc790b8942917da5636f402"
integrity sha512-R8xOqlSTZ+htqrfteCWU5Nk0CDN5ApUTvrlvBuiH1DyP6czDZ4ktbZB0hAgBlVcK0U+qpD3ag3Tqqpa5Q67rPw==

"@sentry/[email protected]":
version "6.2.5"
resolved "https://registry.yarnpkg.com/@sentry/types/-/types-6.2.5.tgz#34b75285b149e0b9bc5fd54fcc2c445d978c7f2e"
integrity sha512-1Sux6CLYrV9bETMsGP/HuLFLouwKoX93CWzG8BjMueW+Di0OGxZphYjXrGuDs8xO8bAKEVGCHgVQdcB2jevS0w==

"@sentry/[email protected]":
version "5.30.0"
resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-5.30.0.tgz#9a5bd7ccff85ccfe7856d493bffa64cabc41e980"
Expand All @@ -1790,6 +1850,14 @@
"@sentry/types" "5.30.0"
tslib "^1.9.3"

"@sentry/[email protected]":
version "6.2.5"
resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-6.2.5.tgz#be90d056b09ed1216097d7a29e3e81ba39238e1b"
integrity sha512-fJoLUZHrd5MPylV1dT4qL74yNFDl1Ur/dab+pKNSyvnHPnbZ/LRM7aJ8VaRY/A7ZdpRowU+E14e/Yeem2c6gtQ==
dependencies:
"@sentry/types" "6.2.5"
tslib "^1.9.3"

"@sindresorhus/is@^0.14.0":
version "0.14.0"
resolved "https://registry.yarnpkg.com/@sindresorhus/is/-/is-0.14.0.tgz#9fb3a3cf3132328151f353de4632e01e52102bea"
Expand Down Expand Up @@ -3976,7 +4044,7 @@ [email protected]:
resolved "https://registry.yarnpkg.com/commander/-/commander-3.0.2.tgz#6837c3fb677ad9933d1cfba42dd14d5117d6b39e"
integrity sha512-Gar0ASD4BDyKC4hl4DwHqDrmvjoxWKZigVnAbn5H1owvm4CxCPdb0HQDehwNYMJpla5+M2tPmPARzhtYuwpHow==

commander@^2.12.1:
commander@^2.12.1, commander@^2.20.0:
version "2.20.3"
resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33"
integrity sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ==
Expand Down Expand Up @@ -4700,6 +4768,16 @@ duplexer@^0.1.1:
resolved "https://registry.yarnpkg.com/duplexer/-/duplexer-0.1.2.tgz#3abe43aef3835f8ae077d136ddce0f276b0400e6"
integrity sha512-jtD6YG370ZCIi/9GTaJKQxWTZD045+4R4hTk/x1UyoqadyJ9x9CgSi1RlVDQF8U2sxLLSnFkCaMihqljHIWgMg==

duplexify@^4.1.1:
version "4.1.1"
resolved "https://registry.yarnpkg.com/duplexify/-/duplexify-4.1.1.tgz#7027dc374f157b122a8ae08c2d3ea4d2d953aa61"
integrity sha512-DY3xVEmVHTv1wSzKNbwoU6nVjzI369Y6sPoqfYr0/xlx3IdX2n94xIszTcjPO8W8ZIv0Wb0PXNcjuZyT4wiICA==
dependencies:
end-of-stream "^1.4.1"
inherits "^2.0.3"
readable-stream "^3.1.1"
stream-shift "^1.0.0"

ecc-jsbn@~0.1.1:
version "0.1.2"
resolved "https://registry.yarnpkg.com/ecc-jsbn/-/ecc-jsbn-0.1.2.tgz#3a83a904e54353287874c564b7549386849a98c9"
Expand Down Expand Up @@ -9867,6 +9945,17 @@ pino-pretty@^4.7.1:
split2 "^3.1.1"
strip-json-comments "^3.1.1"

pino-sentry@^0.6.1:
version "0.6.1"
resolved "https://registry.yarnpkg.com/pino-sentry/-/pino-sentry-0.6.1.tgz#ba3c6b0f904f918a55d99e4fd87cc92da4ba5164"
integrity sha512-UWA+rR7ybFFePqMw+hBL93hIureAWw+WfnxTjT3xg7an0lTnZRZ6rSdxIBgNb0pyKx9goHFaQbYU7Y6vSuIIXg==
dependencies:
"@sentry/node" "^5.21.1"
commander "^2.20.0"
pumpify "^2.0.1"
split2 "^3.1.1"
through2 "^3.0.1"

pino-std-serializers@^3.1.0:
version "3.2.0"
resolved "https://registry.yarnpkg.com/pino-std-serializers/-/pino-std-serializers-3.2.0.tgz#b56487c402d882eb96cd67c257868016b61ad671"
Expand Down Expand Up @@ -10126,6 +10215,15 @@ pump@^3.0.0:
end-of-stream "^1.1.0"
once "^1.3.1"

pumpify@^2.0.1:
version "2.0.1"
resolved "https://registry.yarnpkg.com/pumpify/-/pumpify-2.0.1.tgz#abfc7b5a621307c728b551decbbefb51f0e4aa1e"
integrity sha512-m7KOje7jZxrmutanlkS1daj1dS6z6BgslzOXmcSEpIlCxM3VJH7lG5QLeck/6hgF6F4crFf01UtQmNsJfweTAw==
dependencies:
duplexify "^4.1.1"
inherits "^2.0.3"
pump "^3.0.0"

[email protected]:
version "1.3.2"
resolved "https://registry.yarnpkg.com/punycode/-/punycode-1.3.2.tgz#9653a036fb7c1ee42342f2325cceefea3926c48d"
Expand Down Expand Up @@ -10369,7 +10467,7 @@ read@1, read@~1.0.1:
dependencies:
mute-stream "~0.0.4"

readable-stream@3, readable-stream@^3.0.0, readable-stream@^3.0.2, readable-stream@^3.0.6, readable-stream@^3.1.0, readable-stream@^3.1.1, readable-stream@^3.4.0, readable-stream@^3.6.0:
"readable-stream@2 || 3", readable-stream@3, readable-stream@^3.0.0, readable-stream@^3.0.2, readable-stream@^3.0.6, readable-stream@^3.1.0, readable-stream@^3.1.1, readable-stream@^3.4.0, readable-stream@^3.6.0:
version "3.6.0"
resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-3.6.0.tgz#337bbda3adc0706bd3e024426a286d4b4b2c9198"
integrity sha512-BViHy7LKeTz4oNnkcLJ+lVSL6vpiFeX6/d3oSH8zCW7UxP2onchk+vTGB143xuFjHS3deTgkKoXXymXqymiIdA==
Expand Down Expand Up @@ -11355,6 +11453,11 @@ static-extend@^0.1.1:
resolved "https://registry.yarnpkg.com/statuses/-/statuses-1.5.0.tgz#161c7dac177659fd9811f43771fa99381478628c"
integrity sha1-Fhx9rBd2Wf2YEfQ3cfqZOBR4Yow=

stream-shift@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/stream-shift/-/stream-shift-1.0.1.tgz#d7088281559ab2778424279b0877da3c392d5a3d"
integrity sha512-AiisoFqQ0vbGcZgQPY1cdP2I76glaVA/RauYR4G4thNFgkTqr90yXTo4LYX60Jl+sIlPNHHdGSwo01AvbKUSVQ==

stream-to-pull-stream@^1.7.1:
version "1.7.3"
resolved "https://registry.yarnpkg.com/stream-to-pull-stream/-/stream-to-pull-stream-1.7.3.tgz#4161aa2d2eb9964de60bfa1af7feaf917e874ece"
Expand Down Expand Up @@ -11732,6 +11835,14 @@ through2@^2.0.0, through2@^2.0.1, through2@^2.0.3:
readable-stream "~2.3.6"
xtend "~4.0.1"

through2@^3.0.1:
version "3.0.2"
resolved "https://registry.yarnpkg.com/through2/-/through2-3.0.2.tgz#99f88931cfc761ec7678b41d5d7336b5b6a07bf4"
integrity sha512-enaDQ4MUyP2W6ZyT6EsMzqBPZaM/avg8iuo+l2d3QCs0J+6RaqkHV/2/lOwDTueBHeJ/2LG9lrLW3d5rWPucuQ==
dependencies:
inherits "^2.0.4"
readable-stream "2 || 3"

through2@^4.0.0:
version "4.0.2"
resolved "https://registry.yarnpkg.com/through2/-/through2-4.0.2.tgz#a7ce3ac2a7a8b0b966c80e7c49f0484c3b239764"
Expand Down