From 0a7f1f2589cfed4f329ef4be9d712114e1fe56fb Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Mon, 28 Nov 2022 14:22:36 +0000 Subject: [PATCH 1/5] Fine-tune price service metrics - Remove nodejs default metrics. We don't use them. - Remove `path` from response time metric. - Remove freshness metric and add gap metric. They are similar; however, freshness was measured upon user request but gap is measured upon receiving the next update. - Change receivedVaa to actually represent distinct vaa received. Prior to this, the older vaas, or vaas with same attestation time were not counted in this metric. This will also improve the performance. - Refactors the code a little. `vaaBytes` type was not string and was Buffer. It is fixed now. --- prometheus_config.yaml | 4 + .../pyth/price-service/package-lock.json | 90 +++++++++++++++---- third_party/pyth/price-service/package.json | 1 + .../price-service/src/__tests__/rest.test.ts | 2 +- .../price-service/src/__tests__/ws.test.ts | 2 +- third_party/pyth/price-service/src/listen.ts | 49 +++++++--- .../pyth/price-service/src/promClient.ts | 52 +++++------ third_party/pyth/price-service/src/rest.ts | 26 +----- 8 files changed, 143 insertions(+), 83 deletions(-) diff --git a/prometheus_config.yaml b/prometheus_config.yaml index 2c3ad33d5e..d7181ebc8c 100644 --- a/prometheus_config.yaml +++ b/prometheus_config.yaml @@ -3,3 +3,7 @@ scrape_configs: scrape_interval: 5s static_configs: - targets: ["p2w-attest:3000"] + - job_name: price_service + scrape_interval: 5s + static_configs: + - targets: ["pyth-price-service:8081"] diff --git a/third_party/pyth/price-service/package-lock.json b/third_party/pyth/price-service/package-lock.json index 2183b11a03..8657d2c216 100644 --- a/third_party/pyth/price-service/package-lock.json +++ b/third_party/pyth/price-service/package-lock.json @@ -25,6 +25,7 @@ "express-validation": "^4.0.1", "http-status-codes": "^2.2.0", "joi": "^17.6.0", + "lru-cache": "^7.14.1", "morgan": "^1.10.0", "prom-client": "^14.0.1", "response-time": "^2.3.2", @@ -56,7 +57,7 @@ "dependencies": { "@certusone/wormhole-sdk": "0.2.1", "@improbable-eng/grpc-web-node-http-transport": "^0.14.1", - "@pythnetwork/pyth-sdk-js": "^1.0.0" + "@pythnetwork/pyth-sdk-js": "^1.1.0" }, "devDependencies": { "@openzeppelin/contracts": "^4.2.0", @@ -6455,6 +6456,18 @@ "node": ">=8" } }, + "node_modules/jest-snapshot/node_modules/lru-cache": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", + "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", + "dev": true, + "dependencies": { + "yallist": "^4.0.0" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/jest-snapshot/node_modules/semver": { "version": "7.3.7", "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.7.tgz", @@ -6996,15 +7009,11 @@ } }, "node_modules/lru-cache": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", - "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", - "dev": true, - "dependencies": { - "yallist": "^4.0.0" - }, + "version": "7.14.1", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-7.14.1.tgz", + "integrity": "sha512-ysxwsnTKdAx96aTRdhDOCQfDgbHnt8SK0KY8SEjO0wHinhWOFTESbjVCMPbU1uGXg/ch4lifqx0wfjOawU2+WA==", "engines": { - "node": ">=10" + "node": ">=12" } }, "node_modules/make-dir": { @@ -8317,6 +8326,18 @@ } } }, + "node_modules/superagent/node_modules/lru-cache": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", + "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", + "dev": true, + "dependencies": { + "yallist": "^4.0.0" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/superagent/node_modules/mime": { "version": "2.6.0", "resolved": "https://registry.npmjs.org/mime/-/mime-2.6.0.tgz", @@ -8618,6 +8639,18 @@ } } }, + "node_modules/ts-jest/node_modules/lru-cache": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", + "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", + "dev": true, + "dependencies": { + "yallist": "^4.0.0" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/ts-jest/node_modules/semver": { "version": "7.3.7", "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.7.tgz", @@ -10532,7 +10565,7 @@ "@certusone/wormhole-sdk": "0.2.1", "@improbable-eng/grpc-web-node-http-transport": "^0.14.1", "@openzeppelin/contracts": "^4.2.0", - "@pythnetwork/pyth-sdk-js": "^1.0.0", + "@pythnetwork/pyth-sdk-js": "^1.1.0", "@typechain/ethers-v5": "^7.1.2", "@types/long": "^4.0.1", "@types/node": "^16.6.1", @@ -13857,6 +13890,15 @@ "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==", "dev": true }, + "lru-cache": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", + "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", + "dev": true, + "requires": { + "yallist": "^4.0.0" + } + }, "semver": { "version": "7.3.7", "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.7.tgz", @@ -14272,13 +14314,9 @@ } }, "lru-cache": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", - "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", - "dev": true, - "requires": { - "yallist": "^4.0.0" - } + "version": "7.14.1", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-7.14.1.tgz", + "integrity": "sha512-ysxwsnTKdAx96aTRdhDOCQfDgbHnt8SK0KY8SEjO0wHinhWOFTESbjVCMPbU1uGXg/ch4lifqx0wfjOawU2+WA==" }, "make-dir": { "version": "3.1.0", @@ -15251,6 +15289,15 @@ "ms": "2.1.2" } }, + "lru-cache": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", + "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", + "dev": true, + "requires": { + "yallist": "^4.0.0" + } + }, "mime": { "version": "2.6.0", "resolved": "https://registry.npmjs.org/mime/-/mime-2.6.0.tgz", @@ -15467,6 +15514,15 @@ "yargs-parser": "^20.x" }, "dependencies": { + "lru-cache": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", + "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", + "dev": true, + "requires": { + "yallist": "^4.0.0" + } + }, "semver": { "version": "7.3.7", "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.7.tgz", diff --git a/third_party/pyth/price-service/package.json b/third_party/pyth/price-service/package.json index 3646b0611f..0ffefd41b4 100644 --- a/third_party/pyth/price-service/package.json +++ b/third_party/pyth/price-service/package.json @@ -44,6 +44,7 @@ "express-validation": "^4.0.1", "http-status-codes": "^2.2.0", "joi": "^17.6.0", + "lru-cache": "^7.14.1", "morgan": "^1.10.0", "prom-client": "^14.0.1", "response-time": "^2.3.2", diff --git a/third_party/pyth/price-service/src/__tests__/rest.test.ts b/third_party/pyth/price-service/src/__tests__/rest.test.ts index b2a6ae8695..42e3ca8fa2 100644 --- a/third_party/pyth/price-service/src/__tests__/rest.test.ts +++ b/third_party/pyth/price-service/src/__tests__/rest.test.ts @@ -41,7 +41,7 @@ function dummyPriceInfoPair( priceFeed: dummyPriceFeed(id), attestationTime: 0, seqNum, - vaaBytes: Buffer.from(vaa, "hex").toString("binary"), + vaa: Buffer.from(vaa, "hex"), emitterChainId: 0, priceServiceReceiveTime: 0, }, diff --git a/third_party/pyth/price-service/src/__tests__/ws.test.ts b/third_party/pyth/price-service/src/__tests__/ws.test.ts index 8c4135b138..59ca6b69e5 100644 --- a/third_party/pyth/price-service/src/__tests__/ws.test.ts +++ b/third_party/pyth/price-service/src/__tests__/ws.test.ts @@ -43,7 +43,7 @@ function dummyPriceInfo( attestationTime: dummyPriceMetadataValue.attestation_time, emitterChainId: dummyPriceMetadataValue.emitter_chain, priceFeed: dummyPriceFeed(id), - vaaBytes: Buffer.from(vaa, "hex").toString("binary"), + vaa: Buffer.from(vaa, "hex"), priceServiceReceiveTime: dummyPriceMetadataValue.price_service_receive_time, }; } diff --git a/third_party/pyth/price-service/src/listen.ts b/third_party/pyth/price-service/src/listen.ts index e26bbb7817..622c0036fa 100644 --- a/third_party/pyth/price-service/src/listen.ts +++ b/third_party/pyth/price-service/src/listen.ts @@ -25,9 +25,10 @@ import { HexString, PriceFeed } from "@pythnetwork/pyth-sdk-js"; import { sleep, TimestampInSec } from "./helpers"; import { logger } from "./logging"; import { PromClient } from "./promClient"; +import LRUCache from "lru-cache"; export type PriceInfo = { - vaaBytes: string; + vaa: Buffer; seqNum: number; attestationTime: TimestampInSec; priceFeed: PriceFeed; @@ -52,6 +53,8 @@ type ListenerConfig = { readiness: ListenerReadinessConfig; }; +type VaaKey = string; + export class Listener implements PriceStore { // Mapping of Price Feed Id to Vaa private priceFeedVaaMap = new Map(); @@ -61,6 +64,7 @@ export class Listener implements PriceStore { private spyConnectionTime: TimestampInSec | undefined; private readinessConfig: ListenerReadinessConfig; private updateCallbacks: ((priceInfo: PriceInfo) => any)[]; + private observedVaas: LRUCache; constructor(config: ListenerConfig, promClient?: PromClient) { this.promClient = promClient; @@ -68,6 +72,10 @@ export class Listener implements PriceStore { this.loadFilters(config.filtersRaw); this.readinessConfig = config.readiness; this.updateCallbacks = []; + this.observedVaas = new LRUCache({ + max: 10000, // At most 10000 items + ttl: 60 * 1000, // 60 seconds + }); } private loadFilters(filtersRaw?: string) { @@ -114,7 +122,7 @@ export class Listener implements PriceStore { ); stream = await subscribeSignedVAA(client, { filters: this.filters }); - stream!.on("data", ({ vaaBytes }: { vaaBytes: string }) => { + stream!.on("data", ({ vaaBytes }: { vaaBytes: Buffer }) => { this.processVaa(vaaBytes); }); @@ -150,19 +158,28 @@ export class Listener implements PriceStore { } } - async processVaa(vaaBytes: string) { + async processVaa(vaa: Buffer) { const { parse_vaa } = await importCoreWasm(); - const parsedVAA = parse_vaa(hexToUint8Array(vaaBytes)); + + const vaaString = vaa.toString("hex"); + if (this.observedVaas.has(vaaString)) { + return; + } + + this.observedVaas.set(vaaString, true); + this.promClient?.incReceivedVaa(); + + const parsedVaa = parse_vaa(vaa); let batchAttestation; try { batchAttestation = await parseBatchPriceAttestation( - Buffer.from(parsedVAA.payload) + Buffer.from(parsedVaa.payload) ); } catch (e: any) { logger.error(e, e.stack); - logger.error("Parsing failed. Dropping vaa: %o", parsedVAA); + logger.error("Parsing failed. Dropping vaa: %o", parsedVaa); return; } @@ -194,15 +211,21 @@ export class Listener implements PriceStore { ) { const priceFeed = priceAttestationToPriceFeed(priceAttestation); const priceInfo = { - seqNum: parsedVAA.sequence, - vaaBytes, + seqNum: parsedVaa.sequence, + vaa, attestationTime: priceAttestation.attestationTime, priceFeed, - emitterChainId: parsedVAA.emitter_chain, + emitterChainId: parsedVaa.emitter_chain, priceServiceReceiveTime: Math.floor(new Date().getTime() / 1000), }; this.priceFeedVaaMap.set(key, priceInfo); + if (lastAttestationTime !== undefined) { + this.promClient?.addPriceUpdatesGap( + priceAttestation.attestationTime - lastAttestationTime + ); + } + for (const callback of this.updateCallbacks) { callback(priceInfo); } @@ -211,16 +234,14 @@ export class Listener implements PriceStore { logger.info( "Parsed a new Batch Price Attestation: [" + - parsedVAA.emitter_chain + + parsedVaa.emitter_chain + ":" + - uint8ArrayToHex(parsedVAA.emitter_address) + + uint8ArrayToHex(parsedVaa.emitter_address) + "], seqNum: " + - parsedVAA.sequence + + parsedVaa.sequence + ", Batch Summary: " + getBatchSummary(batchAttestation) ); - - this.promClient?.incReceivedVaa(); } getLatestPriceInfo(priceFeedId: string): PriceInfo | undefined { diff --git a/third_party/pyth/price-service/src/promClient.ts b/third_party/pyth/price-service/src/promClient.ts index f3b69d038d..510c497211 100644 --- a/third_party/pyth/price-service/src/promClient.ts +++ b/third_party/pyth/price-service/src/promClient.ts @@ -11,6 +11,13 @@ import { logger } from "./logging"; const SERVICE_PREFIX = "pyth__price_service__"; +type WebSocketInteractionType = + | "connection" + | "close" + | "timeout" + | "server_update" + | "client_message"; + export class PromClient { private register = new client.Registry(); private collectDefaultMetrics = client.collectDefaultMetrics; @@ -20,16 +27,15 @@ export class PromClient { name: `${SERVICE_PREFIX}vaas_received`, help: "number of Pyth VAAs received", }); + private priceUpdatesGapHistogram = new client.Histogram({ + name: `${SERVICE_PREFIX}price_updates_gap_seconds`, + help: "Summary of gaps between price updates", + buckets: [1, 3, 5, 10, 15, 30, 60, 120], + }); private apiResponseTimeSummary = new client.Summary({ name: `${SERVICE_PREFIX}api_response_time_ms`, help: "Response time of a VAA", - labelNames: ["path", "status"], - }); - private apiRequestsPriceFreshnessHistogram = new client.Histogram({ - name: `${SERVICE_PREFIX}api_requests_price_freshness_seconds`, - help: "Freshness time of Vaa (time difference of Vaa and request time)", - buckets: [1, 5, 10, 15, 30, 60, 120, 180], - labelNames: ["path", "price_id"], + labelNames: ["status"], }); private webSocketInteractionCounter = new client.Counter({ name: `${SERVICE_PREFIX}websocket_interaction`, @@ -51,14 +57,10 @@ export class PromClient { this.register.setDefaultLabels({ app: config.name, }); - this.collectDefaultMetrics({ - register: this.register, - prefix: SERVICE_PREFIX, - }); // Register each metric this.register.registerMetric(this.receivedVaaCounter); + this.register.registerMetric(this.priceUpdatesGapHistogram); this.register.registerMetric(this.apiResponseTimeSummary); - this.register.registerMetric(this.apiRequestsPriceFreshnessHistogram); this.register.registerMetric(this.webSocketInteractionCounter); // End registering metric @@ -70,31 +72,25 @@ export class PromClient { this.receivedVaaCounter.inc(); } - addResponseTime(path: string, status: number, duration: DurationInMs) { + addPriceUpdatesGap(gap: DurationInSec) { + this.priceUpdatesGapHistogram.observe(gap); + } + + // We have multiple paths and for the time being it is not important for us + // to capture it. We might consider capturing it in the future. + addResponseTime(_path: string, status: number, duration: DurationInMs) { this.apiResponseTimeSummary.observe( { - path, status, }, duration ); } - addApiRequestsPriceFreshness( - path: string, - priceId: string, - duration: DurationInSec + addWebSocketInteraction( + type: WebSocketInteractionType, + status: "ok" | "err" ) { - this.apiRequestsPriceFreshnessHistogram.observe( - { - path, - price_id: priceId, - }, - duration - ); - } - - addWebSocketInteraction(type: string, status: "ok" | "err") { this.webSocketInteractionCounter.inc({ type, status, diff --git a/third_party/pyth/price-service/src/rest.ts b/third_party/pyth/price-service/src/rest.ts index 2661eb0106..887c2aa1db 100644 --- a/third_party/pyth/price-service/src/rest.ts +++ b/third_party/pyth/price-service/src/rest.ts @@ -88,7 +88,7 @@ export class RestAPI { // Multiple price ids might share same vaa, we use sequence number as // key of a vaa and deduplicate using a map of seqnum to vaa bytes. - const vaaMap = new Map(); + const vaaMap = new Map(); const notFoundIds: string[] = []; @@ -104,24 +104,15 @@ export class RestAPI { continue; } - const freshness: DurationInSec = - new Date().getTime() / 1000 - - latestPriceInfo.priceFeed.getPriceUnchecked().publishTime; - this.promClient?.addApiRequestsPriceFreshness( - req.path, - id, - freshness - ); - - vaaMap.set(latestPriceInfo.seqNum, latestPriceInfo.vaaBytes); + vaaMap.set(latestPriceInfo.seqNum, latestPriceInfo.vaa); } if (notFoundIds.length > 0) { throw RestException.PriceFeedIdNotFound(notFoundIds); } - const jsonResponse = Array.from(vaaMap.values(), (vaaBytes) => - Buffer.from(vaaBytes, "binary").toString("base64") + const jsonResponse = Array.from(vaaMap.values(), (vaa) => + vaa.toString("base64") ); res.json(jsonResponse); @@ -163,15 +154,6 @@ export class RestAPI { continue; } - const freshness: DurationInSec = - new Date().getTime() / 1000 - - latestPriceInfo.priceFeed.getEmaPriceUnchecked().publishTime; - this.promClient?.addApiRequestsPriceFreshness( - req.path, - id, - freshness - ); - if (verbose) { responseJson.push({ ...latestPriceInfo.priceFeed.toJson(), From ed95f69bec979e1a99e42e6bcc9cb6b33af75395 Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Tue, 29 Nov 2022 17:30:23 +0000 Subject: [PATCH 2/5] Use vaaHash instead of vaa string for vaas set --- third_party/pyth/price-service/src/listen.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/third_party/pyth/price-service/src/listen.ts b/third_party/pyth/price-service/src/listen.ts index 622c0036fa..d8474bdc67 100644 --- a/third_party/pyth/price-service/src/listen.ts +++ b/third_party/pyth/price-service/src/listen.ts @@ -1,8 +1,4 @@ -import { - ChainId, - hexToUint8Array, - uint8ArrayToHex, -} from "@certusone/wormhole-sdk"; +import { ChainId, uint8ArrayToHex } from "@certusone/wormhole-sdk"; import { createSpyRPCServiceClient, @@ -11,6 +7,8 @@ import { import { importCoreWasm } from "@certusone/wormhole-sdk/lib/cjs/solana/wasm"; +import { createHash } from "crypto"; + import { getBatchSummary, parseBatchPriceAttestation, @@ -53,7 +51,7 @@ type ListenerConfig = { readiness: ListenerReadinessConfig; }; -type VaaKey = string; +type VaaHash = string; export class Listener implements PriceStore { // Mapping of Price Feed Id to Vaa @@ -64,7 +62,7 @@ export class Listener implements PriceStore { private spyConnectionTime: TimestampInSec | undefined; private readinessConfig: ListenerReadinessConfig; private updateCallbacks: ((priceInfo: PriceInfo) => any)[]; - private observedVaas: LRUCache; + private observedVaas: LRUCache; constructor(config: ListenerConfig, promClient?: PromClient) { this.promClient = promClient; @@ -161,12 +159,13 @@ export class Listener implements PriceStore { async processVaa(vaa: Buffer) { const { parse_vaa } = await importCoreWasm(); - const vaaString = vaa.toString("hex"); - if (this.observedVaas.has(vaaString)) { + const vaaHash: VaaHash = createHash("md5").update(vaa).digest("base64"); + + if (this.observedVaas.has(vaaHash)) { return; } - this.observedVaas.set(vaaString, true); + this.observedVaas.set(vaaHash, true); this.promClient?.incReceivedVaa(); const parsedVaa = parse_vaa(vaa); From 899454c89ad3e3b13eddddb680c6e877f0cb8486 Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Tue, 29 Nov 2022 17:46:27 +0000 Subject: [PATCH 3/5] Add publish time gap metric --- .../price-service/src/__tests__/rest.test.ts | 1 + .../price-service/src/__tests__/ws.test.ts | 1 + third_party/pyth/price-service/src/listen.ts | 12 +++++++++- .../pyth/price-service/src/promClient.ts | 22 ++++++++++++++----- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/third_party/pyth/price-service/src/__tests__/rest.test.ts b/third_party/pyth/price-service/src/__tests__/rest.test.ts index 42e3ca8fa2..d5a1c72428 100644 --- a/third_party/pyth/price-service/src/__tests__/rest.test.ts +++ b/third_party/pyth/price-service/src/__tests__/rest.test.ts @@ -39,6 +39,7 @@ function dummyPriceInfoPair( id, { priceFeed: dummyPriceFeed(id), + publishTime: 0, attestationTime: 0, seqNum, vaa: Buffer.from(vaa, "hex"), diff --git a/third_party/pyth/price-service/src/__tests__/ws.test.ts b/third_party/pyth/price-service/src/__tests__/ws.test.ts index 59ca6b69e5..b0e8868e26 100644 --- a/third_party/pyth/price-service/src/__tests__/ws.test.ts +++ b/third_party/pyth/price-service/src/__tests__/ws.test.ts @@ -40,6 +40,7 @@ function dummyPriceInfo( ): PriceInfo { return { seqNum: dummyPriceMetadataValue.sequence_number, + publishTime: 0, attestationTime: dummyPriceMetadataValue.attestation_time, emitterChainId: dummyPriceMetadataValue.emitter_chain, priceFeed: dummyPriceFeed(id), diff --git a/third_party/pyth/price-service/src/listen.ts b/third_party/pyth/price-service/src/listen.ts index d8474bdc67..0a7e233b97 100644 --- a/third_party/pyth/price-service/src/listen.ts +++ b/third_party/pyth/price-service/src/listen.ts @@ -28,6 +28,7 @@ import LRUCache from "lru-cache"; export type PriceInfo = { vaa: Buffer; seqNum: number; + publishTime: TimestampInSec; attestationTime: TimestampInSec; priceFeed: PriceFeed; emitterChainId: number; @@ -212,6 +213,7 @@ export class Listener implements PriceStore { const priceInfo = { seqNum: parsedVaa.sequence, vaa, + publishTime: priceAttestation.publishTime, attestationTime: priceAttestation.attestationTime, priceFeed, emitterChainId: parsedVaa.emitter_chain, @@ -220,11 +222,19 @@ export class Listener implements PriceStore { this.priceFeedVaaMap.set(key, priceInfo); if (lastAttestationTime !== undefined) { - this.promClient?.addPriceUpdatesGap( + this.promClient?.addPriceUpdatesAttestationTimeGap( priceAttestation.attestationTime - lastAttestationTime ); } + const lastPublishTime = this.priceFeedVaaMap.get(key)?.publishTime; + + if (lastPublishTime !== undefined) { + this.promClient?.addPriceUpdatesPublishTimeGap( + priceAttestation.publishTime - lastPublishTime + ); + } + for (const callback of this.updateCallbacks) { callback(priceInfo); } diff --git a/third_party/pyth/price-service/src/promClient.ts b/third_party/pyth/price-service/src/promClient.ts index 510c497211..1b80309ad7 100644 --- a/third_party/pyth/price-service/src/promClient.ts +++ b/third_party/pyth/price-service/src/promClient.ts @@ -27,9 +27,14 @@ export class PromClient { name: `${SERVICE_PREFIX}vaas_received`, help: "number of Pyth VAAs received", }); - private priceUpdatesGapHistogram = new client.Histogram({ - name: `${SERVICE_PREFIX}price_updates_gap_seconds`, - help: "Summary of gaps between price updates", + private priceUpdatesPublishTimeGapHistogram = new client.Histogram({ + name: `${SERVICE_PREFIX}price_updates_publish_time_gap_seconds`, + help: "Summary of publish time gaps between price updates", + buckets: [1, 3, 5, 10, 15, 30, 60, 120], + }); + private priceUpdatesAttestationTimeGapHistogram = new client.Histogram({ + name: `${SERVICE_PREFIX}price_updates_attestation_time_gap_seconds`, + help: "Summary of attestation time gaps between price updates", buckets: [1, 3, 5, 10, 15, 30, 60, 120], }); private apiResponseTimeSummary = new client.Summary({ @@ -59,7 +64,8 @@ export class PromClient { }); // Register each metric this.register.registerMetric(this.receivedVaaCounter); - this.register.registerMetric(this.priceUpdatesGapHistogram); + this.register.registerMetric(this.priceUpdatesPublishTimeGapHistogram); + this.register.registerMetric(this.priceUpdatesAttestationTimeGapHistogram); this.register.registerMetric(this.apiResponseTimeSummary); this.register.registerMetric(this.webSocketInteractionCounter); // End registering metric @@ -72,8 +78,12 @@ export class PromClient { this.receivedVaaCounter.inc(); } - addPriceUpdatesGap(gap: DurationInSec) { - this.priceUpdatesGapHistogram.observe(gap); + addPriceUpdatesPublishTimeGap(gap: DurationInSec) { + this.priceUpdatesPublishTimeGapHistogram.observe(gap); + } + + addPriceUpdatesAttestationTimeGap(gap: DurationInSec) { + this.priceUpdatesAttestationTimeGapHistogram.observe(gap); } // We have multiple paths and for the time being it is not important for us From 435b81531c45bcfdd1806cf6a671b77a19a040bd Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Tue, 29 Nov 2022 17:51:33 +0000 Subject: [PATCH 4/5] Add path back to the metrics. --- third_party/pyth/price-service/src/promClient.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/third_party/pyth/price-service/src/promClient.ts b/third_party/pyth/price-service/src/promClient.ts index 1b80309ad7..968a7848cb 100644 --- a/third_party/pyth/price-service/src/promClient.ts +++ b/third_party/pyth/price-service/src/promClient.ts @@ -1,4 +1,3 @@ -import { stat } from "fs"; import http = require("http"); import client = require("prom-client"); import { DurationInMs, DurationInSec } from "./helpers"; @@ -20,7 +19,6 @@ type WebSocketInteractionType = export class PromClient { private register = new client.Registry(); - private collectDefaultMetrics = client.collectDefaultMetrics; // Actual metrics private receivedVaaCounter = new client.Counter({ @@ -40,7 +38,7 @@ export class PromClient { private apiResponseTimeSummary = new client.Summary({ name: `${SERVICE_PREFIX}api_response_time_ms`, help: "Response time of a VAA", - labelNames: ["status"], + labelNames: ["path", "status"], }); private webSocketInteractionCounter = new client.Counter({ name: `${SERVICE_PREFIX}websocket_interaction`, @@ -86,11 +84,10 @@ export class PromClient { this.priceUpdatesAttestationTimeGapHistogram.observe(gap); } - // We have multiple paths and for the time being it is not important for us - // to capture it. We might consider capturing it in the future. - addResponseTime(_path: string, status: number, duration: DurationInMs) { + addResponseTime(path: string, status: number, duration: DurationInMs) { this.apiResponseTimeSummary.observe( { + path, status, }, duration From 40382081245c40929d039f1899d5da509e15b7af Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Wed, 30 Nov 2022 14:12:51 +0000 Subject: [PATCH 5/5] Remove response time metric It can be measured on ingress/service level. --- third_party/pyth/price-service/src/promClient.ts | 16 ---------------- third_party/pyth/price-service/src/rest.ts | 8 -------- 2 files changed, 24 deletions(-) diff --git a/third_party/pyth/price-service/src/promClient.ts b/third_party/pyth/price-service/src/promClient.ts index 968a7848cb..c9b00e3d7b 100644 --- a/third_party/pyth/price-service/src/promClient.ts +++ b/third_party/pyth/price-service/src/promClient.ts @@ -35,11 +35,6 @@ export class PromClient { help: "Summary of attestation time gaps between price updates", buckets: [1, 3, 5, 10, 15, 30, 60, 120], }); - private apiResponseTimeSummary = new client.Summary({ - name: `${SERVICE_PREFIX}api_response_time_ms`, - help: "Response time of a VAA", - labelNames: ["path", "status"], - }); private webSocketInteractionCounter = new client.Counter({ name: `${SERVICE_PREFIX}websocket_interaction`, help: "number of Web Socket interactions", @@ -64,7 +59,6 @@ export class PromClient { this.register.registerMetric(this.receivedVaaCounter); this.register.registerMetric(this.priceUpdatesPublishTimeGapHistogram); this.register.registerMetric(this.priceUpdatesAttestationTimeGapHistogram); - this.register.registerMetric(this.apiResponseTimeSummary); this.register.registerMetric(this.webSocketInteractionCounter); // End registering metric @@ -84,16 +78,6 @@ export class PromClient { this.priceUpdatesAttestationTimeGapHistogram.observe(gap); } - addResponseTime(path: string, status: number, duration: DurationInMs) { - this.apiResponseTimeSummary.observe( - { - path, - status, - }, - duration - ); - } - addWebSocketInteraction( type: WebSocketInteractionType, status: "ok" | "err" diff --git a/third_party/pyth/price-service/src/rest.ts b/third_party/pyth/price-service/src/rest.ts index 887c2aa1db..2e29361c54 100644 --- a/third_party/pyth/price-service/src/rest.ts +++ b/third_party/pyth/price-service/src/rest.ts @@ -63,14 +63,6 @@ export class RestAPI { app.use(morgan(MORGAN_LOG_FORMAT, { stream: winstonStream })); - app.use( - responseTime((req: Request, res: Response, time: DurationInMs) => { - if (res.statusCode !== StatusCodes.NOT_FOUND) { - this.promClient?.addResponseTime(req.path, res.statusCode, time); - } - }) - ); - const endpoints: string[] = []; const latestVaasInputSchema: schema = {