Skip to content

Commit

Permalink
fix(metrics): Only collect metrics for download of tarball files. Met…
Browse files Browse the repository at this point in the history
…rics were previously collected for any GET request that looked like it was for a package.json or tarball install but this was less reliable as there were was no way to guarantee the request coming in was for an actual package.json (e.g. browser requests for favicon.ico). Also, Verdaccio would only hand off requests that generate a 401/403 to the middelware for non-tarball requests so metrics could be misleading.
  • Loading branch information
Ed Clement committed Dec 23, 2021
1 parent 11e8ac6 commit 67caa0f
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 215 deletions.
2 changes: 1 addition & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"extends": ["@verdaccio"],
"rules": {
"no-case-declarations": "off",
"no-case-declarations": 0,
"@typescript-eslint/ban-ts-ignore": 0,
"@typescript-eslint/ban-ts-comment": 0
}
Expand Down
7 changes: 5 additions & 2 deletions .npmignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
src/
test/
tests/
coverage/
.circleci/
.husky/

.babelrc
.editorconfig
Expand All @@ -9,5 +12,5 @@ test/
.npmignore
metrics.ts
jest.config.js
README.md
sonar-project.properties
tsconfig.json
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# verdaccio-metrics-middleware
Metrics middleware plugin for Verdaccio. Collects metrics specifically for package install/download requests and
Metrics middleware plugin for Verdaccio. Collects metrics specifically for package tarball install/download requests and
exposes them at a configurable metrics endpoint (defaults to `/-/metrics`). The metrics are produced in the standard
[prometheus metrics text format](https://prometheus.io/docs/instrumenting/exposition_formats/#text-format-example).

A [counter](https://prometheus.io/docs/concepts/metric_types/#counter) metric is used to track the number of package
installs/downloads. The following [labels](https://prometheus.io/docs/practices/naming/#labels) are applied to _every_
request:
tarball installs/downloads. The following [labels](https://prometheus.io/docs/practices/naming/#labels) are applied to
_every_ request:
- `username` - The Verdaccio username of the user attempting to install/download a package. If the request is
unauthenticated then the value `UNKNOWN` is used.
- `userAgentName` - The name of the user agent the client used to make the request. It is derived from the `user-agent`
Expand Down
50 changes: 25 additions & 25 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import MetricsPlugin, { API_PATH_PREFIX, REQUEST_COUNTER_OPTIONS } from './metrics';
import MetricsPlugin, { REQUEST_COUNTER_OPTIONS } from './metrics';

export default MetricsPlugin;
export { API_PATH_PREFIX, REQUEST_COUNTER_OPTIONS };
export { REQUEST_COUNTER_OPTIONS };
37 changes: 7 additions & 30 deletions src/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,18 @@ import { MetricsConfig, MetricsLabels } from '../types';

import { getUsername, getUserAgentData } from './utils';

export const API_PATH_PREFIX = '/-/';
export const REQUEST_COUNTER_OPTIONS = {
name: 'registry_requests',
help: 'HTTP requests made to the registry',
// Remember that every unique combination of key-value label pairs represents a new time series, which can
// dramatically increase the amount of data stored. Refer to: https://prometheus.io/docs/practices/naming/#labels
labelNames: ['username', 'userAgentName', 'packageGroup', 'statusCode'],
labelNames: ['username', 'userAgentName', 'statusCode', 'packageGroup'],
};

/**
* A Verdaccio middleware plugin implementation. If enabled the following functionality is added:
* 1. A single new metrics endpoint is exposed at a configurable path.
* 2. Metrics are collected related only to install/download of packages.
* 2. Metrics are collected related only to install/download of package tarballs.
* Refer to: https://verdaccio.org/docs/plugin-middleware/
*/
export default class VerdaccioMiddlewarePlugin implements IPluginMiddleware<MetricsConfig> {
Expand All @@ -36,7 +35,7 @@ export default class VerdaccioMiddlewarePlugin implements IPluginMiddleware<Metr
}

/**
* This is the function that Verdaccio invokes when the appropriate middleware configuration is to use this plugin.
* This is the function that Verdaccio invokes when the appropriate configuration is present to use this plugin.
* @param {Application} app - The Express application object.
* @param {IBasicAuth<MetricsConfig>} auth - The Verdaccio authentication service.
* @param {IStorageManager<MetricsConfig>} storage -The Verdaccio storage manager service.
Expand All @@ -48,7 +47,7 @@ export default class VerdaccioMiddlewarePlugin implements IPluginMiddleware<Metr
): void {
if (this.metricsEnabled) {
this.logger.info(`metrics: [register_middlewares] metrics are enabled and exposed at '${this.metricsPath}'`);
app.use(this.collectMetrics.bind(this));
app.get(/.*[.]tgz$/i, this.collectMetrics.bind(this));
app.get(this.metricsPath, this.getMetrics.bind(this));
} else {
this.logger.warn('metrics: [register_middlewares] metrics are disabled');
Expand Down Expand Up @@ -76,24 +75,7 @@ export default class VerdaccioMiddlewarePlugin implements IPluginMiddleware<Metr
* @returns {Promise<void>} - A promise that resolves to undefined since the function is async.
*/
public collectMetrics(req: Request, res: Response, next: NextFunction): void {
const { method, path } = req;

switch (true) {
case !this.metricsEnabled:
return next();
case method !== 'GET':
this.logger.debug(
{ path, method },
`metrics: [collectMetrics] request is not a 'GET' request: ${method} '${path}'`
);
return next();
case path === this.metricsPath:
case path.startsWith(API_PATH_PREFIX):
this.logger.debug({ path }, `metrics: [collectMetrics] request is for an excluded API path: '${path}'`);
return next();
default:
this.logger.debug(`metrics: [collectMetrics] collecting metrics for request: ${method} '${path}'`);
}
const { path } = req;

const authorization = req.header('authorization');
const userAgentString = req.header('user-agent');
Expand All @@ -103,11 +85,6 @@ export default class VerdaccioMiddlewarePlugin implements IPluginMiddleware<Metr
const [, packageGroup] =
Object.entries(this.packageGroups).find(([regex]: [string, string]) => new RegExp(regex).test(decodedPath)) || [];

this.logger.debug(
{ authType, username, userAgentName, userAgentVersion, packageGroup },
'metrics: [collectMetrics] initial request metrics collected'
);

// We won't know the final status code until the response is sent to the client. Because of this we don't collect
// the metrics for this request until the response 'finish' event is emitted.
res.once('finish', () => {
Expand All @@ -117,8 +94,8 @@ export default class VerdaccioMiddlewarePlugin implements IPluginMiddleware<Metr
metricLabels.packageGroup = packageGroup;
}
this.logger.info(
{ authType, userAgentVersion, ...metricLabels },
'metrics: [collectMetrics] final response metrics collected'
{ decodedPath, authType, userAgentString, userAgentVersion, ...metricLabels },
'metrics: [collectMetrics] response metrics collected'
);
// @ts-ignore: The type definitions for `labels` are not great so ignore the TypeScript error.
this.requestsCounter.labels(metricLabels).inc(1);
Expand Down
4 changes: 2 additions & 2 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Logger } from '@verdaccio/types';

export const UNKNOWN = 'UNKNOWN';

export const enum AuthType {
export enum AuthType {
jwt = 'jwt',
password = 'password',
}
Expand Down Expand Up @@ -37,7 +37,7 @@ export function getUsername(
const username = Buffer.from(authValue.trim(), 'base64').toString('utf8').split(':').shift() || UNKNOWN;
// The username is valid if it contains only ASCII characters. This covers the test case where the value is a
// NON base 64 encoded string.
const usernameValid = /^[\x00-\x7F]*$/.test(username || ''); // eslint-disable-line no-control-regex
const usernameValid = /^[\x00-\x7F]*$/.test(username); // eslint-disable-line no-control-regex
return {
username: usernameValid ? username : UNKNOWN,
authType: usernameValid ? AuthType.password : undefined,
Expand Down
Loading

0 comments on commit 67caa0f

Please sign in to comment.