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

Classify more long response to timeout instead of 499 #23700

Merged
merged 7 commits into from
Feb 7, 2025
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,26 @@ export function jsonMorganLoggerMiddleware(
});
})
: undefined;
// HTTP Metric durationInMs should only track internal server time, so manually set it before waiting for response close.
const startTime = performance.now();
const httpMetric = Lumberjack.newLumberMetric(LumberEventName.HttpRequest);
morgan<express.Request, express.Response>((tokens, req, res) => {
let additionalProperties = {};
if (computeAdditionalProperties) {
additionalProperties = computeAdditionalProperties(tokens, req, res);
}
const durationInMs = performance.now() - startTime;
let statusCode = tokens.status(req, res);
if (!statusCode) {
// The effort of trying to distinguish client close vs server close can be tricky when it reaches proxy timeout.
// If proxy timeout happen a little before server timeout, it is actually more due to a server timeout issue.
// Therefore, we can assume it is server timeout (triggered by client) if duration is longer than 20s without
// a valid status code
if (res.locals.serverTimeout) {
statusCode = "Server Timeout";
} else if (res.locals.clientDisconnected) {
statusCode = "499";
statusCode =
durationInMs > 20_000 ? "Server Timeout - Client Disconnect" : "499";
} else {
statusCode = "STATUS_UNAVAILABLE";
}
Expand Down Expand Up @@ -171,9 +178,7 @@ export function jsonMorganLoggerMiddleware(
// Morgan middleware logs using the [on-finished](https://www.npmjs.com/package/on-finished) package, meaning that it will log
// request duration immediately on response 'finish' event. However, the gap between 'finish' and 'close' can be helpful for
// understanding response latency.
const endTime = performance.now();
// HTTP Metric durationInMs should only track internal server time, so manually set it before waiting for response close.
httpMetric.setProperty("durationInMs", endTime - startTime);
httpMetric.setProperty("durationInMs", durationInMs);
// Wait for response 'close' event to signal that the response is completed.
responseLatencyP
?.then((responseLatency) => {
Expand Down