Add ability to request stats for a vat #446
Conversation
There was a problem hiding this comment.
Looks great. I'd love to see one extra test: send a message to the new vat, wait for it to complete, then check the stats again. I think the transcript should be one larger, and the promise count should be one larger too.
I'm also curious to know what would happen if the admin service requests adminStats() for a bogus vatId. User code shouldn't be able to trigger this (it never learns or supplies the vatId, that's encapsulated in the adminNode closure), but once we manage to delete vats (and if we delete all the historical storage associated with them), then we should prepare for the old holder of the adminNode calling adminData after calling terminate. Ideally this just results in a broken promise, but I don't know if we've correctly wired D() up to handle exceptions in the device-side code (really it'll be an exception in the endowment that the device-side code is running, specifically from that vat-keeper vatStats function as it tries to do a get on a non-existent key, which will either throw an exception or return undefined and throw once Nat sees it). But feel free to file a reminder ticket to address that, rather than trying to check/fix it right now (I'm not even sure how we'd write a test for it before having vat-deleting implemented).
| @@ -101,6 +107,20 @@ export function makeVatKeeper( | |||
| storage.set(`${vatID}.t.${id}`, JSON.stringify(msg)); | |||
| } | |||
|
|
|||
| function vatStats() { | |||
| const objectCount = storage.get(`${vatID}.o.nextID`) - FIRST_OBJECT_ID; | |||
There was a problem hiding this comment.
the storage API returns strings, so rather than relying upon JS's coercion rules here, let's use Nat(Number(storage.get(..))) - FIRST_OBJECT_ID.
| const transcriptCount = | ||
| storage.get(`${vatID}.t.nextID`) - FIRST_TRANSCRIPT_ID; | ||
| return harden({ | ||
| objectCount: Nat(Number(objectCount)), |
There was a problem hiding this comment.
and then I think we can remove the Number() from these lines (although it's a good idea to retain the Nat check)
Changes from a previous version: * Subtract the starting index to give counts * ask the vat manager rather than the kernelKeeper. remaining question: is the serialized object containing stats safe, or should it be turned into text?
|
Added the first test. You're right the second test isn't possible now, so I added a new issue for it. |
Chris-Hibbert
force-pushed
the
vatStats2
branch
from
5f6d335 to
9dab5a6
Compare
These may not be all the stats we care about, but they show how the kernel can respond to a request from the vatAdmin facet.