Skip to content

Commit

Permalink
fix: add appName to http response time metrics (#2117)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivarconr committed Sep 30, 2022
1 parent bd9172b commit 5141e77
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 6 deletions.
2 changes: 2 additions & 0 deletions src/lib/__snapshots__/create-config.test.ts.snap
Expand Up @@ -73,6 +73,7 @@ exports[`should create default config 1`] = `
"embedProxyFrontend": false,
"personalAccessTokens": false,
"publicSignup": false,
"responseTimeWithAppName": false,
},
},
"flagResolver": FlagResolver {
Expand All @@ -84,6 +85,7 @@ exports[`should create default config 1`] = `
"embedProxyFrontend": false,
"personalAccessTokens": false,
"publicSignup": false,
"responseTimeWithAppName": false,
},
"externalResolver": {
"isEnabled": [Function],
Expand Down
2 changes: 1 addition & 1 deletion src/lib/app.ts
Expand Up @@ -42,7 +42,7 @@ export default async function getApp(
app.set('port', config.server.port);
app.locals.baseUriPath = baseUriPath;
if (config.server.serverMetrics && config.eventBus) {
app.use(responseTimeMetrics(config.eventBus));
app.use(responseTimeMetrics(config.eventBus, config.flagResolver));
}

app.use(requestLogger(config));
Expand Down
2 changes: 1 addition & 1 deletion src/lib/metrics.test.ts
Expand Up @@ -53,7 +53,7 @@ test('should collect metrics for requests', async () => {

const metrics = await prometheusRegister.metrics();
expect(metrics).toMatch(
/http_request_duration_milliseconds{quantile="0\.99",path="somePath",method="GET",status="200"}.*1337/,
/http_request_duration_milliseconds{quantile="0\.99",path="somePath",method="GET",status="200",appName="undefined"}.*1337/,
);
});

Expand Down
8 changes: 5 additions & 3 deletions src/lib/metrics.ts
Expand Up @@ -57,7 +57,7 @@ export default class MetricsMonitor {
const requestDuration = new client.Summary({
name: 'http_request_duration_milliseconds',
help: 'App response time',
labelNames: ['path', 'method', 'status'],
labelNames: ['path', 'method', 'status', 'appName'],
percentiles: [0.1, 0.5, 0.9, 0.95, 0.99],
maxAgeSeconds: 600,
ageBuckets: 5,
Expand Down Expand Up @@ -143,8 +143,10 @@ export default class MetricsMonitor {

eventBus.on(
events.REQUEST_TIME,
({ path, method, time, statusCode }) => {
requestDuration.labels(path, method, statusCode).observe(time);
({ path, method, time, statusCode, appName }) => {
requestDuration
.labels(path, method, statusCode, appName)
.observe(time);
},
);

Expand Down
12 changes: 11 additions & 1 deletion src/lib/middleware/response-time-metrics.ts
@@ -1,21 +1,31 @@
import * as responseTime from 'response-time';
import EventEmitter from 'events';
import { REQUEST_TIME } from '../metric-events';
import { IFlagResolver } from '../types/experimental';

// eslint-disable-next-line @typescript-eslint/naming-convention
const _responseTime = responseTime.default;

export function responseTimeMetrics(eventBus: EventEmitter): any {
export function responseTimeMetrics(
eventBus: EventEmitter,
flagResolver: IFlagResolver,
): any {
return _responseTime((req, res, time) => {
const { statusCode } = res;

const pathname = req.route ? req.baseUrl + req.route.path : '(hidden)';

let appName;
if (flagResolver.isEnabled('responseTimeWithAppName')) {
appName = req.headers['unleash-appname'];
}

const timingInfo = {
path: pathname,
method: req.method,
statusCode,
time,
appName,
};
eventBus.emit(REQUEST_TIME, timingInfo);
});
Expand Down
4 changes: 4 additions & 0 deletions src/lib/types/experimental.ts
Expand Up @@ -26,6 +26,10 @@ export const defaultExperimentalOptions = {
process.env.UNLEASH_EXPERIMENTAL_PUBLIC_SIGNUP,
false,
),
responseTimeWithAppName: parseEnvVarBoolean(
process.env.UNLEASH_EXPERIMENTAL_RESPONSE_TIME_WITH_APP_NAME,
false,
),
},
externalResolver: { isEnabled: (): boolean => false },
};
Expand Down
1 change: 1 addition & 0 deletions src/server-dev.ts
Expand Up @@ -37,6 +37,7 @@ process.nextTick(async () => {
embedProxyFrontend: true,
batchMetrics: true,
anonymiseEventLog: false,
responseTimeWithAppName: true,
},
},
authentication: {
Expand Down

0 comments on commit 5141e77

Please sign in to comment.