Skip to content

Commit

Permalink
fix(core): Respect prefix for all Prometheus metrics (n8n-io#10130)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivov authored Jul 22, 2024
1 parent a7ae23b commit b1816db
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 11 deletions.
10 changes: 10 additions & 0 deletions packages/cli/BREAKING-CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

This list shows all the versions which include breaking changes and how to upgrade.

## 1.52.0

### What changed?

Prometheus metrics enabled via `N8N_METRICS_INCLUDE_DEFAULT_METRICS` and `N8N_METRICS_INCLUDE_API_ENDPOINTS` were fixed to include the default `n8n_` prefix.

### When is action necessary?

If you are using Prometheus metrics from these categories and are using a non-empty prefix, please update those metrics to match their new prefixed names.

## 1.47.0

### What changed?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('PrometheusMetricsService', () => {
expect(service.counters.cacheUpdatesTotal?.inc).toHaveBeenCalledWith(0);
});

it('should set up API metrics with `express-prom-bundle`', async () => {
it('should set up route metrics with `express-prom-bundle`', async () => {
config.set('endpoints.metrics.includeApiEndpoints', true);
config.set('endpoints.metrics.includeApiPathLabel', true);
config.set('endpoints.metrics.includeApiMethodLabel', true);
Expand Down
41 changes: 33 additions & 8 deletions packages/cli/src/metrics/prometheus-metrics.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import { CacheService } from '@/services/cache/cache.service';
import { MessageEventBus } from '@/eventbus/MessageEventBus/MessageEventBus';
import { EventMessageTypeNames } from 'n8n-workflow';
import type { EventMessageTypes } from '@/eventbus';

type MetricCategory = 'default' | 'api' | 'cache' | 'logs';
import type { Includes, MetricCategory, MetricLabel } from './types';

@Service()
export class PrometheusMetricsService {
Expand All @@ -24,10 +23,10 @@ export class PrometheusMetricsService {

private readonly prefix = config.getEnv('endpoints.metrics.prefix');

private readonly includes = {
private readonly includes: Includes = {
metrics: {
default: config.getEnv('endpoints.metrics.includeDefaultMetrics'),
api: config.getEnv('endpoints.metrics.includeApiEndpoints'),
routes: config.getEnv('endpoints.metrics.includeApiEndpoints'),
cache: config.getEnv('endpoints.metrics.includeCacheMetrics'),
logs: config.getEnv('endpoints.metrics.includeMessageEventBusMetrics'),
},
Expand Down Expand Up @@ -60,8 +59,20 @@ export class PrometheusMetricsService {
}

disableAllMetrics() {
for (const metric of Object.keys(this.includes.metrics) as MetricCategory[]) {
this.includes.metrics[metric] = false;
for (const metric in this.includes.metrics) {
this.includes.metrics[metric as MetricCategory] = false;
}
}

enableLabels(labels: MetricLabel[]) {
for (const label of labels) {
this.includes.labels[label] = true;
}
}

disableAllLabels() {
for (const label in this.includes.labels) {
this.includes.labels[label as MetricLabel] = false;
}
}

Expand Down Expand Up @@ -98,7 +109,7 @@ export class PrometheusMetricsService {
* Set up metrics for server routes with `express-prom-bundle`
*/
private initRouteMetrics(app: express.Application) {
if (!this.includes.metrics.api) return;
if (!this.includes.metrics.routes) return;

const metricsMiddleware = promBundle({
autoregister: false,
Expand Down Expand Up @@ -126,11 +137,25 @@ export class PrometheusMetricsService {
private mountMetricsEndpoint(app: express.Application) {
app.get('/metrics', async (_req: express.Request, res: express.Response) => {
const metrics = await promClient.register.metrics();
const prefixedMetrics = this.addPrefixToMetrics(metrics);
res.setHeader('Content-Type', promClient.register.contentType);
res.send(metrics).end();
res.send(prefixedMetrics).end();
});
}

private addPrefixToMetrics(metrics: string) {
return metrics
.split('\n')
.map((rawLine) => {
const line = rawLine.trim();

if (!line || line.startsWith('#') || line.startsWith(this.prefix)) return rawLine;

return this.prefix + line;
})
.join('\n');
}

/**
* Set up cache metrics: `n8n_cache_hits_total`, `n8n_cache_misses_total`, and
* `n8n_cache_updates_total`
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/metrics/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export type MetricCategory = 'default' | 'routes' | 'cache' | 'logs';

export type MetricLabel =
| 'credentialsType'
| 'nodeType'
| 'workflowId'
| 'apiPath'
| 'apiMethod'
| 'apiStatusCode';

export type Includes = {
metrics: Record<MetricCategory, boolean>;
labels: Record<MetricLabel, boolean>;
};
67 changes: 65 additions & 2 deletions packages/cli/test/integration/prometheus-metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('Metrics', () => {

beforeEach(() => {
prometheusService.disableAllMetrics();
prometheusService.disableAllLabels();
});

it('should return n8n version', async () => {
Expand All @@ -51,7 +52,9 @@ describe('Metrics', () => {

const { version, major, minor, patch } = n8nVersion;

expect(toLines(response)).toContain(
const lines = toLines(response);

expect(lines).toContain(
`n8n_test_version_info{version="v${version}",major="${major}",minor="${minor}",patch="${patch}"} 1`,
);
});
Expand All @@ -73,7 +76,10 @@ describe('Metrics', () => {
*/
expect(response.status).toEqual(200);
expect(response.type).toEqual('text/plain');
expect(toLines(response)).toContain('nodejs_heap_space_size_total_bytes{space="read_only"} 0');

const lines = toLines(response);

expect(lines).toContain('n8n_test_nodejs_heap_space_size_total_bytes{space="read_only"} 0');
});

it('should not return default metrics if disabled', async () => {
Expand Down Expand Up @@ -146,4 +152,61 @@ describe('Metrics', () => {
expect(lines).not.toContain('n8n_test_cache_misses_total 0');
expect(lines).not.toContain('n8n_test_cache_updates_total 0');
});

it('should return route metrics if enabled', async () => {
/**
* Arrange
*/
prometheusService.enableMetric('routes');
await prometheusService.init(server.app);
await agent.get('/api/v1/workflows');

/**
* Act
*/
const response = await agent.get('/metrics');

/**
* Assert
*/
expect(response.status).toEqual(200);
expect(response.type).toEqual('text/plain');

const lines = toLines(response);

expect(lines).toContain('n8n_test_http_request_duration_seconds_count 1');
expect(lines).toContainEqual(
expect.stringContaining('n8n_test_http_request_duration_seconds_sum'),
);
expect(lines).toContainEqual(
expect.stringContaining('n8n_test_http_request_duration_seconds_bucket'),
);
});

it('should return labels in route metrics if enabled', async () => {
/**
* ARrange
*/
prometheusService.enableMetric('routes');
prometheusService.enableLabels(['apiMethod', 'apiPath', 'apiStatusCode']);
await prometheusService.init(server.app);
await agent.get('/webhook-test/some-uuid');

/**
* Act
*/
const response = await agent.get('/metrics');

/**
* Assert
*/
expect(response.status).toEqual(200);
expect(response.type).toEqual('text/plain');

const lines = toLines(response);

expect(lines).toContainEqual(expect.stringContaining('method="GET"'));
expect(lines).toContainEqual(expect.stringContaining('path="/webhook-test/some-uuid"'));
expect(lines).toContainEqual(expect.stringContaining('status_code="404"'));
});
});

0 comments on commit b1816db

Please sign in to comment.