feat(telemetry): export swingstore vat object metrics to datadog #8050
Conversation
toliaqat
force-pushed
the
tl-vat-objects-to-dd
branch
from
74f48af
to
1e5231a
Compare
toliaqat
force-pushed
the
tl-vat-objects-to-dd
branch
from
1e5231a
to
8767db0
Compare
| export const makeSwingstore = db => { | ||
| const dbTool = () => { | ||
| const prepare = (strings, ...params) => { | ||
| const dml = strings.join('?'); |
There was a problem hiding this comment.
wow, that's great, I knew quasiliterals could be used for this purpose but had never actually seen it done
| /** | ||
| * @param {import('better-sqlite3').Database} db | ||
| */ | ||
| export const makeSwingstore = db => { |
There was a problem hiding this comment.
let's name this slightly differently, to avoid confusion with the makeSwingStore defined in @agoric/swing-store.. how about makeDB ?
| @@ -0,0 +1,160 @@ | |||
| import { StatsD } from 'hot-shots'; | |||
There was a problem hiding this comment.
looks like this needs to be added to the package.json, and the yarn.lock updated
There was a problem hiding this comment.
Oh, it's already there. I bet the lint CI job doesn't believe in packages that aren't direct subdirectories of the top-level packages/ workspace.
Let's just disable lint on this line, I think by inserting
// eslint-disable-next-line
before it.. see if that appeases CI.
|
|
||
| const kvAll = key => sql.iterate`select * from kvStore where key GLOB ${key}`; | ||
|
|
||
| return Object.freeze({ |
There was a problem hiding this comment.
Let's consider using harden here.. if we're going to freeze it, we might as well go all the way. It's not so important for a CLI tool like this (this returned API object is not being exposed to any malicious/untrusted code), but it's not a bad habit to get into.
return harden({ .., and then the index.js needs to start with import '@endo/init', and we need to add @endo/init to the package.json. That's the pattern we use for our "Hardened JS" style. We usually harden everything exported by a module (export const fname = harden(() => { ..), as well as any object exposed to the importers of the module.
| count += 1; | ||
| const vatID = row.key.split('.')[0]; | ||
| const stat = vatStats.get(vatID) || newStat(); | ||
| vatStats.set(vatID, stat); |
There was a problem hiding this comment.
since there are a lot of rows, this might be a tiny bit faster if we skip the redundant .set, something like
let stat = vatStats.get(vatID);
if (!stat) {
stat = newStat();
vatStats.set(vatID, stat);
}| const stat = vatStats.get(vatID) || newStat(); | ||
| vatStats.set(vatID, stat); | ||
| stat.objectExportsReachable += | ||
| object.test(row.key) && exportReach.test(row.value); |
There was a problem hiding this comment.
likewise, there's a lot of redundant regexp execution here, I'd suggest hoisting the if (object. test(row.key)) check, as a single conditional around the block of four import/export/etc checks.
| // "v$NN.c.ko48": "_ o-7" # merely-recognizable import | ||
| // "v$NN.c.kp70": "R p+12" # promise (always "R", sometimes "p+" sometimes "p-") | ||
|
|
||
| const object = /^v\d+\.c\..*ko\d.*$/; // vNN.c.koNNNNN* |
There was a problem hiding this comment.
I think the trailing .*$ is redundant, might consider omitting it (although I imagine the regexp compiler discards it, so it's not like it's likely to make anything slower, and it does make it nicely clear what we're expecting).
| console.log(`Opening database at ${fullPath}`); | ||
| const kStore = makeSwingstore(dbOpen(fullPath, { readonly: true })); | ||
| const height = kStore.findHeight(); | ||
| const vatItems = kStore.findVatItems(); |
There was a problem hiding this comment.
maybe named this vatItemIterator to make it more obvious that we aren't reading the entire DB into RAM first
| @@ -0,0 +1,27 @@ | |||
| { | |||
| "name": "store-stats", | |||
There was a problem hiding this comment.
let's name it swingset-store-vatstats, and add a private: true field just to remind us to give it a better name (something under @agoric/, probably) before we publish it.
Description
Scan the swingset database for vat object imports/exports and report them to datadog.
We plan to run this tool periodically on a follower node to get telemetry for performance improvement work.
Security Considerations
Scaling Considerations
When database gets pretty large, then we might need to switch to better reading methods. Currently it takes less than 5 seconds to process around a million rows.
Documentation Considerations
Testing Considerations
Manual + unit testing