test(cosmic-swingset): Add test coverage for Prometheus output #10954
Conversation
gibson042
force-pushed
the
gibson-10911-prometheus-snapshot
branch
from
af8cdb2 to
799daa7
Compare
Deploying agoric-sdk with
|
| Latest commit: |
e32baf0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://be62467e.agoric-sdk.pages.dev |
| Branch Preview URL: | https://gibson-10911-prometheus-snap.agoric-sdk.pages.dev |
gibson042
changed the base branch from
master
to
gibson-10937-update-opentelemetry
… in defaultInitMessage
gibson042
force-pushed
the
gibson-10911-prometheus-snapshot
branch
from
799daa7 to
452567e
Compare
|
Base branch is changed to master. Please re-run the integration tests by adding 'force:integration' label. |
There was a problem hiding this comment.
This looks like the beginning of some awesome test framework for cosmic-swingset. Pretty amazing!
I am however concerned the hangover / savedChainSends logic got hurt in the refactor. (it was really hard to track where things moved, I kinda wish that refactor commit could have been split in multiple commits to facilitate diffing)
| waitForIdleBridge: async () => | ||
| stateSyncExport?.exporter?.onStarted().then(ignore, ignore), | ||
| }); |
There was a problem hiding this comment.
Name is a bit misleading here. It's not really wait for idle bridge, but our last chance to delay swing-store commit in case the state-sync export hasn't had a chance to open a read tx on it.
If there is any "idle bridge" it's actually the finally after doBlockingSend which I am moving in my upcoming PR to be explicitly at the end of END_BLOCK.
|
|
||
| const { blockingSend, shutdown } = s; | ||
| ({ writeSlogObject, savedChainSends } = s); |
There was a problem hiding this comment.
I don't see how savedChainSends is initialized from the value returned by launch. Did that get dropped? The hangover logic is notoriously lacking testing and quite brittle (we have know bugs in it)
| * Generalize Node.js util.promisify to support callbacks with multiple | ||
| * post-error parameters, which fulfill the returned promise as an array rather | ||
| * than as a single value. |
There was a problem hiding this comment.
Post-error parameter? Maybe multiple parameters of which the last is an error?
Also what kind of node callback API does this!?
| } else if (result.length === 1) { | ||
| resolve(/** @type {any} */ (result[0])); | ||
| } else { | ||
| resolve(result); |
There was a problem hiding this comment.
I dislike changing the type of resolved values dynamically like that but I'll allow it here for backwards compat (and because I think the types get it).
But I can't shake the feeling this is trying to be too smart.
| import '@endo/eventual-send/shim.js'; | ||
|
|
||
| try { | ||
| lockdown({ __hardenTaming__: 'unsafe' }); |
There was a problem hiding this comment.
Sad, we can't even test if the intrinsics are frozen with unsafe harden.
There was a problem hiding this comment.
Should the file's name capture this is doing an unsafe lockdown ?
There was a problem hiding this comment.
Also the original @endo/init also tames async_hooks which this doesn't do. See endojs/endo#2711
Extends #10941
Fixes #10911
Description
Extracts a testable
launchChainunit in packages/cosmic-swingset/src/chain-main.js, and uses that for an ava snapshot test of localhost:${OTEL_EXPORTER_PROMETHEUS_PORT}/metrics. Includes incidental fixes and utility additions along the way (and therefore best reviewed by commit).Security Considerations
None known.
Scaling Considerations
n/a
Documentation Considerations
n/a
Testing Considerations
Just the usual
--update-snapshotswhen things change.Upgrade Considerations
n/a