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: gateway histogram response time only updated on 200 #80
fix: gateway histogram response time only updated on 200 #80
Conversation
Note: Should go after #79 to avoid messing up with DO migrations on release |
@@ -19,11 +19,12 @@ import { | |||
*/ | |||
|
|||
const GATEWAY_METRICS_ID = 'gateway_metrics' | |||
const SUCCESS_STATUS = 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have HTTP_STATUS_SUCCESS
in constants.js
@@ -85,6 +86,16 @@ export class GatewayMetrics0 { | |||
this.gatewayMetrics.totalWinnerRequests += 1 | |||
} | |||
|
|||
// Only update response time histogram if response is successful | |||
if (stats.status === SUCCESS_STATUS) { | |||
this._updateResponseTimeHistogram(stats) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's now not obvious that this statistic is for successful responses - perhaps we should rename to reflect the change here?
Deploying with Cloudflare Pages
|
63c7f2e
to
d0a9d02
Compare
Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
Noticed that Pinata was having really fast responses but number did not make sense. Turns out 🤦🏼♂️ we were counting other status codes for the response, which since we introduced pinata.nftstorage.link with out own rate limit errors were resulting in lot's of really fast 429 responses.