Skip to content

Commit

Permalink
fix(metrics): use path pattern instead of raw path in path labels
Browse files Browse the repository at this point in the history
Signed-off-by: nflaig <nflaig@protonmail.com>
  • Loading branch information
nflaig authored and raymondfeng committed Jan 8, 2021
1 parent 8bce9e8 commit 80a07bc
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 2 deletions.
13 changes: 13 additions & 0 deletions extensions/metrics/src/__tests__/acceptance/metrics.acceptance.ts
Expand Up @@ -254,6 +254,19 @@ describe('Metrics (acceptance)', () => {
/loopback_invocation_duration_summary{quantile="0.01",targetName="MockController.prototype.success",method="GET",path="\/success",statusCode="204"}/,
);
});

it('uses the path pattern instead of the raw path in path labels', async () => {
await request.get('/path/1');
await request.get('/path/1/2');

const res = await request
.get('/metrics')
.expect(200)
.expect('content-type', /text/);

expect(res.text).to.match(/path="\/path\/{param}"/);
expect(res.text).to.match(/path="\/path\/{firstParam}\/{secondParam}"/);
});
});

context('with configured default labels', () => {
Expand Down
10 changes: 10 additions & 0 deletions extensions/metrics/src/__tests__/acceptance/mock.controller.ts
Expand Up @@ -8,6 +8,7 @@ import {
del,
get,
HttpErrors,
param,
patch,
post,
put,
Expand Down Expand Up @@ -63,4 +64,13 @@ export class MockController {
serverError() {
throw new Error();
}

@get('/path/{param}')
pathWithParam(@param.path.string('param') _param: string) {}

@get('/path/{firstParam}/{secondParam}')
pathWithParams(
@param.path.string('firstParam') _firstParam: string,
@param.path.string('secondParam') _secondParam: string,
) {}
}
22 changes: 20 additions & 2 deletions extensions/metrics/src/interceptors/metrics.interceptor.ts
Expand Up @@ -9,10 +9,17 @@ import {
injectable,
Interceptor,
InvocationContext,
InvocationSource,
Provider,
ValueOrPromise,
} from '@loopback/core';
import {HttpErrors, RequestContext, Response} from '@loopback/rest';
import {
HttpErrors,
Request,
RequestContext,
Response,
RouteSource,
} from '@loopback/rest';
import {
Counter,
Gauge,
Expand Down Expand Up @@ -90,8 +97,8 @@ export class MetricsInterceptor implements Provider<Interceptor> {
const labelValues: LabelValues<LabelNames> = {
targetName: invocationCtx.targetName,
method: request.method,
path: request.path,
};
labelValues.path = getPathPattern(invocationCtx, request);
const endGauge = MetricsInterceptor.gauge.startTimer();
const endHistogram = MetricsInterceptor.histogram.startTimer();
const endSummary = MetricsInterceptor.summary.startTimer();
Expand All @@ -111,6 +118,13 @@ export class MetricsInterceptor implements Provider<Interceptor> {
}
}

function getPathPattern({source}: InvocationContext, request: Request) {
// make sure to use path pattern instead of raw path
// this is important since paths can contain unbounded sets of values
// such as IDs which would create a new time series for each unique value
return isRouteSource(source) ? source.value.path : request.path;
}

function getStatusCodeFromResponse(response: Response, result?: unknown) {
// interceptors are invoked before result is written to response,
// the status code for 200 responses without a result should be 204
Expand All @@ -124,3 +138,7 @@ function getStatusCodeFromError(err: HttpErrors.HttpError) {
const notFound = err.code === 'ENTITY_NOT_FOUND';
return err.statusCode ?? err.status ?? (notFound ? 404 : 500);
}

function isRouteSource(source?: InvocationSource): source is RouteSource {
return source?.type === 'route';
}

0 comments on commit 80a07bc

Please sign in to comment.