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

fix: fix cron-nft-ttr logging, multiple gateway support #2006

Closed
wants to merge 8 commits into from
Closed
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
4 changes: 2 additions & 2 deletions .github/workflows/cron-nft-ttr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ jobs:
PUSHGATEWAY_URL: https://pushgateway.k8s.locotorp.info/metrics/job/nftstorage_ci/instance/github_action
PUSHGATEWAY_BASIC_AUTH: ${{ secrets.PUSHGATEWAY_BASIC_AUTH }}
run: |
yarn workspace cron run start:nft-ttr measure --logConfigAndExit
yarn workspace cron run start:nft-ttr measure \
--minImageSizeBytes=10000000 \
--gateways https://nftstorage.link https://dweb.link \
--gateway https://nftstorage.link \
--gateway https://dweb.link \
--metricsPushGateway $PUSHGATEWAY_URL \
--metricsPushGatewayJobName $PUSHGATEWAY_JOBNAME \

Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/cron.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ on:
- '.github/workflows/cron.yml'
- 'yarn.lock'
pull_request:
branches:
- main
paths:
- 'packages/cron/**'
- '.github/workflows/cron.yml'
Expand Down
3 changes: 3 additions & 0 deletions packages/cron/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@
"eslint-plugin-prettier": "^4.0.0",
"npm-run-all": "^4.1.5"
},
"ava": {
"workerThreads": false
},
"eslintConfig": {
"plugins": [
"@typescript-eslint"
Expand Down
35 changes: 18 additions & 17 deletions packages/cron/src/bin/nft-ttr.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ export function createMeasureSecretsFromEnv(env) {
export function createMeasureOptionsFromSade(sadeOptions, secrets) {
// build gateways
assert.ok(
hasOwnProperty(sadeOptions, 'gateways'),
'expected sadeOptions to have gateways'
hasOwnProperty(sadeOptions, 'gateway'),
'expected sadeOptions to have gateway'
)
const gatewaysArgv = sadeOptions.gateways
assert.ok(Array.isArray(gatewaysArgv) || typeof gatewaysArgv === 'string')
const gatewaysArgvsArray = Array.isArray(gatewaysArgv)
? gatewaysArgv.map(String)
: [...gatewaysArgv.split(' ')]
const gatewayArgv = sadeOptions.gateway
assert.ok(Array.isArray(gatewayArgv) || typeof gatewayArgv === 'string')
const gatewaysArgvsArray = Array.isArray(gatewayArgv)
? gatewayArgv.map(String)
: gatewayArgv.split(' ')
const gateways = gatewaysArgvsArray.map((g) => {
try {
return new URL(g)
Expand Down Expand Up @@ -117,6 +117,10 @@ export function createMeasureOptionsFromSade(sadeOptions, secrets) {
const metricsPushGatewayJobName =
hasOwnProperty(sadeOptions, 'metricsPushGatewayJobName') &&
sadeOptions.metricsPushGatewayJobName
assert.ok(
metricsPushGatewayJobName,
'expected metricsPushGatewayJobName to be set'
)
assert.ok(typeof metricsPushGatewayJobName === 'string')

// build pushRetrieveMetrics
Expand Down Expand Up @@ -192,13 +196,8 @@ export async function* cli(
'nft-ttr'
)
.option(
'--logConfigAndExit',
'if provided, this job will log the config and exit without doing much else. Intended for debugging',
false
)
.option(
'--gateways',
'IPFS gateway(s) to use to measure time to retrieval of the upload',
'--gateway',
'IPFS gateway to use to measure time to retrieval of the upload',
'https://nftstorage.link'
)
.option(
Expand All @@ -210,7 +209,9 @@ export async function* cli(
iterable = measureNftTimeToRetrievability({
secrets,
...createMeasureOptionsFromSade(opts, secrets),
...options,
...(options.fetchImage ? { fetchImage: options.fetchImage } : {}),
...(options.store ? { store: options.store } : {}),
console: options.console,
})
})
argParser.parse(argv)
Expand All @@ -234,8 +235,8 @@ function parseBasicAuth(basicAuthEnvVarString) {
* @param {string[]} argv
*/
async function main(argv) {
// eslint-disable-next-line no-empty,@typescript-eslint/no-unused-vars,no-unused-vars
for await (const _ of cli(argv)) {
for await (const _ of cli(argv, { console, env: process.env })) {
console.debug(_)
}
}

Expand Down
62 changes: 4 additions & 58 deletions packages/cron/src/bin/nft-ttr.spec.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
import { test } from '../lib/testing.js'
import {
cli as binNftTtr,
createMeasureOptionsFromSade,
createMeasureSecretsFromEnv,
} from './nft-ttr.js'
import {
createStubbedImageFetcher,
createStubStoreFunction,
} from '../jobs/measureNftTimeToRetrievability.js'
import all from 'it-all'
import { Writable } from 'node:stream'

const defaultTestMinImageSizeBytes = 10 * 1e6

test('createMeasureOptionsFromSade', (t) => {
const sampleSade = {
Expand All @@ -21,14 +12,15 @@ test('createMeasureOptionsFromSade', (t) => {
'https://pushgateway.k8s.locotorp.info/metrics/job/nftstorage_ci/instance/github_action',
url: 'https://nft.storage',
metricsPushGatewayJobName: 'nft-ttr',
gateways: 'https://nftstorage.link/',
gateway: ['https://nftstorage.link/', 'https://dweb.link/'],
}
const options = createMeasureOptionsFromSade(sampleSade, {
metricsPushGatewayAuthorization: '',
})
t.assert(options)
t.is(options.gateways.length, 1)
t.is(options.gateways[0].toString(), sampleSade.gateways)
t.is(options.gateways.length, 2)
t.is(options.gateways[0].toString(), sampleSade.gateway[0])
t.is(options.gateways[1].toString(), sampleSade.gateway[1])
t.is(options.metricsPushGateway?.toString(), sampleSade.metricsPushGateway)
t.is(options.metricsPushGatewayJobName, sampleSade.metricsPushGatewayJobName)
})
Expand All @@ -52,49 +44,3 @@ test('createMeasureSecretsFromEnv', (t) => {
'Basic dXNlcjpwYXNz'
)
})

test(`bin/nft-ttr works with --minImageSizeBytes=${defaultTestMinImageSizeBytes} and multiple gateways`, async (t) => {
const minImageSizeBytes = defaultTestMinImageSizeBytes
const gateways = ['https://nftstorage.link', 'https://dweb.link']
const command = [
'fakeNodePath',
'fakeScriptPath',
'measure',
`--minImageSizeBytes=${minImageSizeBytes}`,
`--gateways=${gateways.join(' ')}`,
]
const activities = await all(
binNftTtr(command, {
console: new console.Console(new Writable(), new Writable()),
env: {
NFT_STORAGE_API_KEY: '',
},
store: createStubStoreFunction(),
fetchImage: createStubbedImageFetcher(minImageSizeBytes),
})
)
let retrieveCount = 0
const gatewaysNeedingRetrieval = new Set(gateways)
for (const activity of activities) {
if (activity.type !== 'retrieve') {
continue
}
const retrieve = activity
retrieveCount++
t.assert(retrieve)
t.is(
typeof retrieve.duration.size,
'number',
'expected retrieve duration size to be a number'
)
t.assert(retrieve.contentLength > minImageSizeBytes)
for (const gateway of gatewaysNeedingRetrieval) {
if (retrieve.url.toString().startsWith(gateway)) {
gatewaysNeedingRetrieval.delete(gateway)
break
}
}
}
t.is(gatewaysNeedingRetrieval.size, 0)
t.is(retrieveCount, gateways.length)
})
33 changes: 16 additions & 17 deletions packages/cron/src/jobs/measureNftTimeToRetrievability.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const EXAMPLE_NFT_IMG_URL = new URL(
* @property {string} image
* @property {"retrieve"} type
* @property {URL} url
* @property {URL} gateway
* @property {number} contentLength
* @property {Date} startTime
* @property {Milliseconds} duration
Expand Down Expand Up @@ -67,7 +68,6 @@ export function createStubStoreFunction() {
* @property {AsyncIterable<Blob>} images - images to upload/retrieve
* @property {StoreFunction} [store] - function to store nft
* @property {string} [url] - URL to nft.storage to measure
* @property {boolean} [logConfigAndExit] - if true, log config and exit
* @property {URL} [metricsPushGateway] - Server to send metrics to. should reference a https://github.com/prometheus/pushgateway
* @property {URL[]} gateways - IPFS Gateway to test retrieval from
* @property {Console} console - logger
Expand Down Expand Up @@ -101,15 +101,18 @@ function readMeasureTtrOptions(options) {
*/

/**
* @typedef StoreLog
* @property {"store"} type
* @typedef StoreStartLog
* @property {"store/start"} type
* @property {string} image
* @property {Date} startTime
* @property {Milliseconds} duration
*/

/**
* @typedef {StoreLog|Activity<"start"|"finish">|RetrieveLog} MeasureTtrLog
* @typedef StoreLog
* @property {"store"} type
* @property {string} image
* @property {Date} startTime
* @property {Milliseconds} duration
*/

/**
Expand All @@ -118,22 +121,15 @@ function readMeasureTtrOptions(options) {
* * upload to nft.storage
* * retrieve image through ipfs gateway
* @param {MeasureTtrOptions} options
* @returns {AsyncIterable<
* StoreLog|Activity<"start"|"finish">|RetrieveLog
* >}
* @returns {AsyncIterable<StoreStartLog|StoreLog|Activity<"start"|"finish">|RetrieveLog>}
*/
export async function* measureNftTimeToRetrievability(options) {
// separate secrets and config to avoid logging secrets
const { secrets, config } = readMeasureTtrOptions(options)
if (config.logConfigAndExit) {
config.console.log(config)
return
}
/** @type {Activity<"start">} */
const start = {
type: 'start',
}
config.console.debug(start)
yield start
for await (const image of config.images) {
const imageId = Number(new Date()).toString()
Expand All @@ -149,12 +145,13 @@ export async function* measureNftTimeToRetrievability(options) {
const storeStartedAt = now()
/** @type {StoreFunction} */
const store = config.store || ((nft) => client.store(nft))
const storeBeforeLog = {
type: 'store/before',
/** @type {StoreStartLog} */
const storeStartLog = {
type: 'store/start',
image: imageId,
startTime: new Date(),
}
config.console.debug(storeBeforeLog)
yield storeStartLog
const metadata = await store(nft)
const storeEndAt = now()
/** @type {StoreLog} */
Expand All @@ -165,14 +162,14 @@ export async function* measureNftTimeToRetrievability(options) {
duration: Milliseconds.subtract(storeEndAt, storeStartedAt),
}
yield storeLog
config.console.debug(storeLog)
for (const gateway of config.gateways) {
/** @type {RetrieveLog} */
let retrieval
try {
retrieval = await retrieve(options, {
id: imageId,
url: createGatewayRetrievalUrl(gateway, metadata.ipnft),
gateway,
})
} catch (error) {
console.error('error retrieving', error)
Expand Down Expand Up @@ -225,6 +222,7 @@ export const httpImageFetcher = (fetch) => async (url) => {
* @param {object} image
* @param {string} image.id
* @param {URL} image.url
* @param {URL} image.gateway
*/
async function retrieve(options, image) {
const retrieveFetchDate = new Date()
Expand All @@ -239,6 +237,7 @@ async function retrieve(options, image) {
type: 'retrieve',
url: image.url,
image: image.id,
gateway: image.gateway,
/** length in bytes */
contentLength: retrievedImage.size,
startTime: retrieveFetchDate,
Expand Down