From 06d2ebb89f85876b9e640097f8d81e016f920784 Mon Sep 17 00:00:00 2001 From: Daniel Khan Date: Sat, 28 Sep 2019 20:25:56 +0200 Subject: [PATCH 01/24] fix(http-plugin): move node-sdk to dev deps --- packages/opentelemetry-plugin-http/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-plugin-http/package.json b/packages/opentelemetry-plugin-http/package.json index 89cbb99a68..a1a91aa9bd 100644 --- a/packages/opentelemetry-plugin-http/package.json +++ b/packages/opentelemetry-plugin-http/package.json @@ -48,6 +48,7 @@ "@types/sinon": "^7.0.13", "@types/superagent": "^4.1.3", "@opentelemetry/basic-tracer": "^0.0.1", + "@opentelemetry/node-sdk": "^0.0.1", "@opentelemetry/scope-base": "^0.0.1", "axios": "^0.19.0", "got": "^9.6.0", @@ -69,7 +70,6 @@ }, "dependencies": { "@opentelemetry/core": "^0.0.1", - "@opentelemetry/node-sdk": "^0.0.1", "@opentelemetry/types": "^0.0.1", "semver": "^6.3.0", "shimmer": "^1.2.1" From e70342c40bc6a3fbb6e9f5df89f4467ae75a8d24 Mon Sep 17 00:00:00 2001 From: Daniel Khan Date: Fri, 18 Oct 2019 15:45:18 +0200 Subject: [PATCH 02/24] feat: update dependencies --- .../package.json | 3 ++- .../src/prometheus.ts | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 packages/opentelemetry-exporter-prometheus/src/prometheus.ts diff --git a/packages/opentelemetry-exporter-prometheus/package.json b/packages/opentelemetry-exporter-prometheus/package.json index 0e3380448e..cd335d5739 100644 --- a/packages/opentelemetry-exporter-prometheus/package.json +++ b/packages/opentelemetry-exporter-prometheus/package.json @@ -45,13 +45,14 @@ "mocha": "^6.2.0", "nyc": "^14.1.1", "rimraf": "^3.0.0", - "tslint-microsoft-contrib": "^6.2.0", "ts-mocha": "^6.0.0", "ts-node": "^8.3.0", + "tslint-microsoft-contrib": "^6.2.0", "typescript": "^3.6.3" }, "dependencies": { "@opentelemetry/core": "^0.1.0", + "@opentelemetry/metrics": "^0.1.1", "@opentelemetry/types": "^0.1.0" } } diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts new file mode 100644 index 0000000000..f886a0c9cf --- /dev/null +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -0,0 +1,16 @@ +/*! + * Copyright 2019, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + From 5e99e8dcc198adb1c036046c58352543c1a7e7ba Mon Sep 17 00:00:00 2001 From: Daniel Khan Date: Sun, 20 Oct 2019 10:24:48 +0200 Subject: [PATCH 03/24] feat: server implementation and tests --- .../package.json | 4 +- .../src/index.ts | 2 +- .../src/prometheus.ts | 105 ++++++++++++ .../src/types.ts | 40 +++++ .../test/prometheus.test.ts | 149 ++++++++++++++++++ .../tslint.json | 4 + 6 files changed, 302 insertions(+), 2 deletions(-) create mode 100644 packages/opentelemetry-exporter-prometheus/src/types.ts create mode 100644 packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts create mode 100644 packages/opentelemetry-exporter-prometheus/tslint.json diff --git a/packages/opentelemetry-exporter-prometheus/package.json b/packages/opentelemetry-exporter-prometheus/package.json index ee87842684..102c69011b 100644 --- a/packages/opentelemetry-exporter-prometheus/package.json +++ b/packages/opentelemetry-exporter-prometheus/package.json @@ -40,9 +40,11 @@ "devDependencies": { "@types/mocha": "^5.2.7", "@types/node": "^12.6.9", + "chai": "^4.2.0", + "chai-http": "^4.3.0", "codecov": "^3.5.0", "gts": "^1.1.0", - "mocha": "^6.2.0", + "mocha": "^6.2.2", "nyc": "^14.1.1", "rimraf": "^3.0.0", "ts-mocha": "^6.0.0", diff --git a/packages/opentelemetry-exporter-prometheus/src/index.ts b/packages/opentelemetry-exporter-prometheus/src/index.ts index ab4fd7cc33..449a2843b1 100644 --- a/packages/opentelemetry-exporter-prometheus/src/index.ts +++ b/packages/opentelemetry-exporter-prometheus/src/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -// +export * from './prometheus'; diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index f886a0c9cf..8d0bcf4113 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -14,3 +14,108 @@ * limitations under the License. */ + +import * as url from 'url'; + +import { createServer, Server, IncomingMessage, ServerResponse } from 'http'; + +import * as types from '@opentelemetry/types'; + +import { + +} from '@opentelemetry/metrics' + +import { ExporterConfig } from './types'; +import { NoopLogger } from '@opentelemetry/core'; +import { + Registry, +} from 'prom-client'; + +export class PrometheusExporter { + + static readonly DEFAULT_OPTIONS = { + port: 9464, + startServer: false, + endpoint: '/metrics', + prefix: '', + }; + + private readonly _registry = new Registry(); + private readonly _logger: types.Logger; + private readonly _port: number; + private readonly _endpoint: string; + private _server?: Server; + // private readonly _prefix: string; + + // Histogram cannot have a label named 'le' + // private static readonly RESERVED_HISTOGRAM_LABEL = 'le'; + + /** + * Constructor + * @param config Exporter configuration + * @param callback Callback to be called after a server was started + */ + constructor(config: ExporterConfig = {}, callback?: () => void) { + this._logger = config.logger || new NoopLogger(); + this._port = config.port || PrometheusExporter.DEFAULT_OPTIONS.port; + // this._prefix = config.prefix || PrometheusExporter.DEFAULT_OPTIONS.prefix; + + let endpoint = config.endpoint || PrometheusExporter.DEFAULT_OPTIONS.endpoint; + + if (!endpoint.startsWith('/')) { + endpoint = `/${endpoint}`; + } + this._endpoint = endpoint; + + if (config.startServer || PrometheusExporter.DEFAULT_OPTIONS.startServer) { + this.startServer(callback); + } + } + + /** + * Stops the Prometheus exporter server + * @param callback A callback that will be executed once the server is stopped + */ + stopServer(callback?: () => void) { + if (!this._server) { + this._logger.debug(`Prometheus stopServer() was called but server was never started.`); + if (callback) { + callback(); + } + } else { + this._server.close(() => { + this._logger.debug(`Prometheus exporter was stopped`); + if (callback) { + callback(); + } + }); + } + } + + /** + * Starts the Prometheus exporter server and registers the request handler + * @param callback A callback that will be called once the server is reade + */ + startServer(callback?: () => void) { + const requestHandler = ((request: IncomingMessage, response: ServerResponse) => { + const parsedUrl = url.parse(request.url as string); + if (parsedUrl.pathname === this._endpoint) { + response.statusCode = 200; + response.setHeader('content-type', this._registry.contentType); + response.end(this._registry.metrics()); + } else { + response.statusCode = 404; + response.end(); + } + + }); + this._server = createServer(requestHandler); + this._server.listen(this._port, () => { + this._logger.debug(`Prometheus exporter started on port ${this._port} + at endpoint ${this._endpoint}`); + if (callback) { + callback(); + } + }); + } +} diff --git a/packages/opentelemetry-exporter-prometheus/src/types.ts b/packages/opentelemetry-exporter-prometheus/src/types.ts new file mode 100644 index 0000000000..8091dbf639 --- /dev/null +++ b/packages/opentelemetry-exporter-prometheus/src/types.ts @@ -0,0 +1,40 @@ +/*! + * Copyright 2019, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as types from '@opentelemetry/types'; + +export interface ExporterConfig { + + /** App prefix for metrics, if needed */ + prefix?: string; + + /** Endpoint the metrics should be exposed at with preceeding / */ + endpoint?: string; + + /** + * Port number for Prometheus exporter server + * Default registered port is 9464: + * https://github.com/prometheus/prometheus/wiki/Default-port-allocations + */ + port?: number; + + /** + * Define if the Prometheus exporter server will be started - default false + */ + startServer?: boolean; + + logger?: types.Logger; +} diff --git a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts new file mode 100644 index 0000000000..ee31b10acc --- /dev/null +++ b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts @@ -0,0 +1,149 @@ +/*! + * Copyright 2019, OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as assert from 'assert'; +import * as chai from 'chai'; +import * as http from 'http'; +import chaiHttp = require('chai-http'); + +chai.use(chaiHttp); + +import { PrometheusExporter } from '../src'; +import { ConsoleLogger } from '@opentelemetry/core'; + +describe('PrometheusExporter', () => { + describe('constructor', () => { + it('should construct an exporter', () => { + const exporter = new PrometheusExporter(); + assert.ok(typeof exporter.startServer === 'function'); + assert.ok(typeof exporter.stopServer === 'function'); + }); + + it('should start the server if startServer is passed as an option', (done) => { + const port = PrometheusExporter.DEFAULT_OPTIONS.port; + const endpoint = PrometheusExporter.DEFAULT_OPTIONS.endpoint; + const exporter = new PrometheusExporter({ + startServer: true, + }, () => { + const url = `http://localhost:${port}${endpoint}`; + http.get(url, function (res: any) { + assert.equal(res.statusCode, 200); + exporter.stopServer(() => { + return done(); + }); + }); + }); + }); + + }); + + describe('server', () => { + it('it should start on startServer() and call the callback', (done) => { + const exporter = new PrometheusExporter({ + port: 9722, + }); + exporter.startServer(() => { + exporter.stopServer(() => { + return done(); + }); + }); + }); + + it('it should listen on the default port and default endpoint', (done) => { + const port = PrometheusExporter.DEFAULT_OPTIONS.port; + const endpoint = PrometheusExporter.DEFAULT_OPTIONS.endpoint; + const exporter = new PrometheusExporter(); + + exporter.startServer(() => { + const url = `http://localhost:${port}${endpoint}`; + http.get(url, function (res: any) { + assert.equal(res.statusCode, 200); + exporter.stopServer(() => { + return done(); + }); + }); + }); + }); + + it('it should listen on a custom port and endpoint if provided', (done) => { + const port = 9991; + const endpoint = '/metric'; + + const exporter = new PrometheusExporter({ + port, + endpoint, + }); + + exporter.startServer(() => { + const url = `http://localhost:${port}${endpoint}`; + http.get(url, function (res: any) { + assert.equal(res.statusCode, 200); + exporter.stopServer(() => { + return done(); + }); + }); + }); + }); + + it('it should lnormalize an endpoint that doesn\'t start with a slash', (done) => { + const port = 9991; + const endpoint = 'metric'; + + const exporter = new PrometheusExporter({ + port, + endpoint, + }); + + exporter.startServer(() => { + const url = `http://localhost:${port}/${endpoint}`; + http.get(url, function (res: any) { + assert.equal(res.statusCode, 200); + exporter.stopServer(() => { + return done(); + }); + }); + }); + }); + + it('it should return a HTTP status 404 if the endpoint does not match', (done) => { + const port = 9912; + const endpoint = '/metricss'; + const exporter = new PrometheusExporter({ + port, + endpoint, + }); + exporter.startServer(() => { + const url = + `http://localhost:${port}/metrics`; + + http.get(url, function (res: any) { + assert.equal(res.statusCode, 404); + exporter.stopServer(() => { + return done(); + }); + }); + }); + }); + + it('should call a provided callback regardless of if the server is running', (done) => { + const exporter = new PrometheusExporter(); + exporter.stopServer(() => { + return done(); + }); + }); + }); +}); + diff --git a/packages/opentelemetry-exporter-prometheus/tslint.json b/packages/opentelemetry-exporter-prometheus/tslint.json new file mode 100644 index 0000000000..0710b135d0 --- /dev/null +++ b/packages/opentelemetry-exporter-prometheus/tslint.json @@ -0,0 +1,4 @@ +{ + "rulesDirectory": ["node_modules/tslint-microsoft-contrib"], + "extends": ["../../tslint.base.js", "./node_modules/tslint-consistent-codestyle"] +} From 26592985d461c328b7a7cc6c7b7f6c1e5a325bd0 Mon Sep 17 00:00:00 2001 From: Daniel Khan Date: Thu, 24 Oct 2019 14:48:25 +0200 Subject: [PATCH 04/24] fix: remove console logger --- .../opentelemetry-exporter-prometheus/test/prometheus.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts index ee31b10acc..4e2de0d67f 100644 --- a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts @@ -22,7 +22,6 @@ import chaiHttp = require('chai-http'); chai.use(chaiHttp); import { PrometheusExporter } from '../src'; -import { ConsoleLogger } from '@opentelemetry/core'; describe('PrometheusExporter', () => { describe('constructor', () => { From 2dd51d9a983a313b96c5adcbe30d91844dc71baf Mon Sep 17 00:00:00 2001 From: Daniel Khan Date: Tue, 29 Oct 2019 14:38:50 +0100 Subject: [PATCH 05/24] fix: add types --- .../src/prometheus.ts | 39 +++++++++++++++++-- packages/opentelemetry-metrics/src/index.ts | 1 + 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 8d0bcf4113..4429a5702b 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -21,12 +21,13 @@ import { createServer, Server, IncomingMessage, ServerResponse } from 'http'; import * as types from '@opentelemetry/types'; -import { - -} from '@opentelemetry/metrics' +import { ReadableMetric, MetricDescriptor } from '@opentelemetry/metrics'; import { ExporterConfig } from './types'; import { NoopLogger } from '@opentelemetry/core'; + +import * as Prometheus from 'prom-client'; + import { Registry, } from 'prom-client'; @@ -45,7 +46,7 @@ export class PrometheusExporter { private readonly _port: number; private readonly _endpoint: string; private _server?: Server; - // private readonly _prefix: string; + private readonly _prefix: string; // Histogram cannot have a label named 'le' // private static readonly RESERVED_HISTOGRAM_LABEL = 'le'; @@ -72,6 +73,36 @@ export class PrometheusExporter { } } + export(metrics: ReadableMetric[]) { + + metrics.forEach((metric) => { + this._updateMetric(metric); + }); + } + + private _updateMetric(metric: ReadableMetric) { + this._registerMetric(metric); + } + + private _registerMetric(metric: ReadableMetric): Prometheus.Metric { + const metricName = this._getPrometheusMetricName(metric.descriptor); + } + + private _getPrometheusMetricName(descriptor: MetricDescriptor): string { + let metricName; + if (this._prefix) { + metricName = `${this._prefix}_${descriptor.name}`; + } else { + metricName = descriptor.name; + } + return this._sanitizePrometheusMetricName(metricName); + } + + private _sanitizePrometheusMetricName(name: string): string { + return name.replace(/\W/g, '_'); + } + + /** * Stops the Prometheus exporter server * @param callback A callback that will be executed once the server is stopped diff --git a/packages/opentelemetry-metrics/src/index.ts b/packages/opentelemetry-metrics/src/index.ts index 95ea05565f..6c1d1c23b9 100644 --- a/packages/opentelemetry-metrics/src/index.ts +++ b/packages/opentelemetry-metrics/src/index.ts @@ -17,3 +17,4 @@ export * from './Handle'; export * from './Meter'; export * from './Metric'; +export * from './export/types'; From b26f810de48f8a87cb29a21df5fe343d56ec0744 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 30 Oct 2019 09:28:00 -0400 Subject: [PATCH 06/24] feat(prometheus-exporter): counter and gauge --- .../package.json | 1 + .../src/prometheus.ts | 121 ++++++++++++++---- 2 files changed, 97 insertions(+), 25 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/package.json b/packages/opentelemetry-exporter-prometheus/package.json index 102c69011b..e6423a56b5 100644 --- a/packages/opentelemetry-exporter-prometheus/package.json +++ b/packages/opentelemetry-exporter-prometheus/package.json @@ -54,6 +54,7 @@ }, "dependencies": { "@opentelemetry/core": "^0.1.1", + "@opentelemetry/base": "^0.1.1", "@opentelemetry/metrics": "^0.1.1", "@opentelemetry/types": "^0.1.1", "prom-client": "^11.5.3" diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 4429a5702b..9fd0c6b285 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -21,19 +21,15 @@ import { createServer, Server, IncomingMessage, ServerResponse } from 'http'; import * as types from '@opentelemetry/types'; -import { ReadableMetric, MetricDescriptor } from '@opentelemetry/metrics'; +import { ReadableMetric, MetricDescriptor, MetricExporter, MetricDescriptorType, LabelValue } from '@opentelemetry/metrics'; import { ExporterConfig } from './types'; import { NoopLogger } from '@opentelemetry/core'; +import { ExportResult } from '@opentelemetry/base'; import * as Prometheus from 'prom-client'; -import { - Registry, -} from 'prom-client'; - -export class PrometheusExporter { - +export class PrometheusExporter implements MetricExporter { static readonly DEFAULT_OPTIONS = { port: 9464, startServer: false, @@ -41,12 +37,12 @@ export class PrometheusExporter { prefix: '', }; - private readonly _registry = new Registry(); + private readonly _registry = new Prometheus.Registry(); private readonly _logger: types.Logger; private readonly _port: number; private readonly _endpoint: string; private _server?: Server; - private readonly _prefix: string; + private readonly _prefix?: string; // Histogram cannot have a label named 'le' // private static readonly RESERVED_HISTOGRAM_LABEL = 'le'; @@ -59,7 +55,7 @@ export class PrometheusExporter { constructor(config: ExporterConfig = {}, callback?: () => void) { this._logger = config.logger || new NoopLogger(); this._port = config.port || PrometheusExporter.DEFAULT_OPTIONS.port; - // this._prefix = config.prefix || PrometheusExporter.DEFAULT_OPTIONS.prefix; + this._prefix = config.prefix || PrometheusExporter.DEFAULT_OPTIONS.prefix; let endpoint = config.endpoint || PrometheusExporter.DEFAULT_OPTIONS.endpoint; @@ -73,29 +69,104 @@ export class PrometheusExporter { } } - export(metrics: ReadableMetric[]) { + export(readableMetrics: ReadableMetric[], cb: (result: ExportResult) => void) { + this._logger.debug('Prometheus exporter export'); - metrics.forEach((metric) => { - this._updateMetric(metric); - }); + for (const readableMetric of readableMetrics) { + this._updateMetric(readableMetric); + } + + cb(ExportResult.SUCCESS); } - private _updateMetric(metric: ReadableMetric) { - this._registerMetric(metric); + private _updateMetric(readableMetric: ReadableMetric) { + const metric = this._registerMetric(readableMetric); + if (!metric) return; + + const labelKeys = readableMetric.descriptor.labelKeys; + + if (metric instanceof Prometheus.Counter) { + for (const ts of readableMetric.timeseries) { + metric.inc( + this._getLabelValues(labelKeys, ts.labelValues), + ts.points[0].value as number + ); + } + } + + if (metric instanceof Prometheus.Gauge) { + for (const ts of readableMetric.timeseries) { + metric.set( + this._getLabelValues(labelKeys, ts.labelValues), + ts.points[0].value as number + ); + } + } + + // TODO: only counter and gauge are implemented in metrics so far } - private _registerMetric(metric: ReadableMetric): Prometheus.Metric { - const metricName = this._getPrometheusMetricName(metric.descriptor); + private _getLabelValues(keys: string[], values: LabelValue[]) { + const labelValues: Prometheus.labelValues = {}; + for (let i = 0; i < keys.length; i++) { + if (values[i].value !== null) { + labelValues[keys[i]] = values[i].value!; + } + } + return labelValues; } - private _getPrometheusMetricName(descriptor: MetricDescriptor): string { - let metricName; - if (this._prefix) { - metricName = `${this._prefix}_${descriptor.name}`; - } else { - metricName = descriptor.name; + private _registerMetric(readableMetric: ReadableMetric): Prometheus.Metric | undefined { + const metricName = this._getPrometheusMetricName(readableMetric.descriptor); + const metric = this._registry.getSingleMetric(metricName); + if (metric) return metric; + + const newMetric = this._newMetric(readableMetric, metricName); + if (!newMetric) return; + + this._registry.registerMetric(newMetric); + return newMetric; + } + + private _newMetric(readableMetric: ReadableMetric, name: string): Prometheus.Metric | undefined { + const metricObject = { + name, + help: readableMetric.descriptor.description, + labelNames: readableMetric.descriptor.labelKeys + }; + + switch (readableMetric.descriptor.type) { + case MetricDescriptorType.COUNTER_DOUBLE: + case MetricDescriptorType.COUNTER_INT64: + return new Prometheus.Counter(metricObject); + case MetricDescriptorType.GAUGE_DOUBLE: + case MetricDescriptorType.GAUGE_INT64: + return new Prometheus.Gauge(metricObject); + case MetricDescriptorType.GAUGE_HISTOGRAM: + case MetricDescriptorType.CUMULATIVE_HISTOGRAM: + const histogramConfig = { + ...metricObject, + buckets: this._getHistogramBoundaries(readableMetric) + }; + return new Prometheus.Histogram(histogramConfig); + case MetricDescriptorType.SUMMARY: + return new Prometheus.Summary(metricObject); + case MetricDescriptorType.UNSPECIFIED: + this._logger.error("Do not use UNSPECIFIED readable metric type"); + return; } - return this._sanitizePrometheusMetricName(metricName); + + } + + private _getHistogramBoundaries(_readableMetric: ReadableMetric): number[] { + // TODO + throw new Error('Unimplemented') + } + + private _getPrometheusMetricName(descriptor: MetricDescriptor): string { + return this._sanitizePrometheusMetricName( + this._prefix ? `${this._prefix}_${descriptor.name}` : descriptor.name + ); } private _sanitizePrometheusMetricName(name: string): string { From b5c77581be2d215e6cdd1d8bd7ffec291ce580f8 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 4 Nov 2019 09:46:07 -0500 Subject: [PATCH 07/24] feat(prometheus-exporter): implement counter and gauge --- .../package.json | 7 +- .../src/prometheus.ts | 191 ++++++++++++------ .../src/types.ts | 14 +- .../test/prometheus.test.ts | 76 ++++--- .../opentelemetry-metrics/src/export/types.ts | 3 + 5 files changed, 181 insertions(+), 110 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/package.json b/packages/opentelemetry-exporter-prometheus/package.json index e6423a56b5..912326fef0 100644 --- a/packages/opentelemetry-exporter-prometheus/package.json +++ b/packages/opentelemetry-exporter-prometheus/package.json @@ -40,8 +40,6 @@ "devDependencies": { "@types/mocha": "^5.2.7", "@types/node": "^12.6.9", - "chai": "^4.2.0", - "chai-http": "^4.3.0", "codecov": "^3.5.0", "gts": "^1.1.0", "mocha": "^6.2.2", @@ -53,10 +51,11 @@ "typescript": "^3.6.3" }, "dependencies": { - "@opentelemetry/core": "^0.1.1", "@opentelemetry/base": "^0.1.1", + "@opentelemetry/core": "^0.1.1", "@opentelemetry/metrics": "^0.1.1", "@opentelemetry/types": "^0.1.1", - "prom-client": "^11.5.3" + "prom-client": "^11.5.3", + "tslint-consistent-codestyle": "^1.16.0" } } diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 9fd0c6b285..e38c34a8bf 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -14,20 +14,20 @@ * limitations under the License. */ - -import * as url from 'url'; - -import { createServer, Server, IncomingMessage, ServerResponse } from 'http'; - -import * as types from '@opentelemetry/types'; - -import { ReadableMetric, MetricDescriptor, MetricExporter, MetricDescriptorType, LabelValue } from '@opentelemetry/metrics'; - -import { ExporterConfig } from './types'; -import { NoopLogger } from '@opentelemetry/core'; import { ExportResult } from '@opentelemetry/base'; - +import { NoopLogger } from '@opentelemetry/core'; +import { + LabelValue, + MetricDescriptor, + MetricDescriptorType, + MetricExporter, + ReadableMetric, +} from '@opentelemetry/metrics'; +import * as types from '@opentelemetry/types'; +import { createServer, IncomingMessage, Server, ServerResponse } from 'http'; import * as Prometheus from 'prom-client'; +import * as url from 'url'; +import { ExporterConfig } from './types'; export class PrometheusExporter implements MetricExporter { static readonly DEFAULT_OPTIONS = { @@ -41,9 +41,10 @@ export class PrometheusExporter implements MetricExporter { private readonly _logger: types.Logger; private readonly _port: number; private readonly _endpoint: string; - private _server?: Server; + private _server: Server; private readonly _prefix?: string; + // This will be required when histogram is implemented. Leaving here so it is not forgotten // Histogram cannot have a label named 'le' // private static readonly RESERVED_HISTOGRAM_LABEL = 'le'; @@ -56,20 +57,42 @@ export class PrometheusExporter implements MetricExporter { this._logger = config.logger || new NoopLogger(); this._port = config.port || PrometheusExporter.DEFAULT_OPTIONS.port; this._prefix = config.prefix || PrometheusExporter.DEFAULT_OPTIONS.prefix; + this._server = createServer(this._requestHandler); - let endpoint = config.endpoint || PrometheusExporter.DEFAULT_OPTIONS.endpoint; - - if (!endpoint.startsWith('/')) { - endpoint = `/${endpoint}`; - } - this._endpoint = endpoint; + this._endpoint = ( + config.endpoint || PrometheusExporter.DEFAULT_OPTIONS.endpoint + ).replace(/^([^/])/, '/$1'); if (config.startServer || PrometheusExporter.DEFAULT_OPTIONS.startServer) { this.startServer(callback); + } else if (callback) { + callback() } } - export(readableMetrics: ReadableMetric[], cb: (result: ExportResult) => void) { + /** + * Save current metric state so that it can be pulled by the metrics backend. + * + * @todo reach into metrics to pull metric values on endpoint + * In it's current state, the exporter saves the current values of all metrics when export + * is called and returns them when the export endpoint is called. In the future, this should + * be a no-op and the exporter should reach into the metrics when the export endpoint is + * called. As there is currently no interface to do this, this is our only option. + * + * @param readableMetrics Metrics to be sent to the prometheus backend + * @param cb result callback to be called on finish + */ + export( + readableMetrics: ReadableMetric[], + cb: (result: ExportResult) => void + ) { + if (!this._server) { + // It is conceivable that the _server may not be started as it is an async startup + // However unlikely, if this happens the caller may retry the export + cb(ExportResult.FAILED_RETRYABLE); + return; + } + this._logger.debug('Prometheus exporter export'); for (const readableMetric of readableMetrics) { @@ -79,6 +102,18 @@ export class PrometheusExporter implements MetricExporter { cb(ExportResult.SUCCESS); } + /** + * Shut down the export server + */ + shutdown() { + this.stopServer(); + } + + /** + * Save the value of one metric to be exported + * + * @param readableMetric Metric value to be saved + */ private _updateMetric(readableMetric: ReadableMetric) { const metric = this._registerMetric(readableMetric); if (!metric) return; @@ -87,19 +122,19 @@ export class PrometheusExporter implements MetricExporter { if (metric instanceof Prometheus.Counter) { for (const ts of readableMetric.timeseries) { - metric.inc( - this._getLabelValues(labelKeys, ts.labelValues), - ts.points[0].value as number - ); + // Prometheus counter saves internal state and increments by given value. + // ReadableMetric value is the current state, not the delta to be incremented by. + // Currently, _registerMetric creates a new counter every time the value changes, + // so the increment here behaves as a set value (increment from 0) + metric.inc(this._getLabelValues(labelKeys, ts.labelValues), ts.points[0] + .value as number); } } if (metric instanceof Prometheus.Gauge) { for (const ts of readableMetric.timeseries) { - metric.set( - this._getLabelValues(labelKeys, ts.labelValues), - ts.points[0].value as number - ); + metric.set(this._getLabelValues(labelKeys, ts.labelValues), ts.points[0] + .value as number); } } @@ -116,10 +151,24 @@ export class PrometheusExporter implements MetricExporter { return labelValues; } - private _registerMetric(readableMetric: ReadableMetric): Prometheus.Metric | undefined { + private _registerMetric( + readableMetric: ReadableMetric + ): Prometheus.Metric | undefined { const metricName = this._getPrometheusMetricName(readableMetric.descriptor); const metric = this._registry.getSingleMetric(metricName); - if (metric) return metric; + + /** + * Prometheus library does aggregation, which means its inc method must be called with + * the value to be incremented by. It does not have a set method. As our ReadableMetric + * contains the current value, not the value to be incremented by, we destroy and + * recreate counters when they are updated. + * + * This works because counters are identified by their name and no other internal ID + * https://prometheus.io/docs/instrumenting/exposition_formats/ + */ + if (metric instanceof Prometheus.Counter) { + this._registry.removeSingleMetric(metricName); + } else if (metric) return metric; const newMetric = this._newMetric(readableMetric, metricName); if (!newMetric) return; @@ -128,11 +177,14 @@ export class PrometheusExporter implements MetricExporter { return newMetric; } - private _newMetric(readableMetric: ReadableMetric, name: string): Prometheus.Metric | undefined { + private _newMetric( + readableMetric: ReadableMetric, + name: string + ): Prometheus.Metric | undefined { const metricObject = { name, help: readableMetric.descriptor.description, - labelNames: readableMetric.descriptor.labelKeys + labelNames: readableMetric.descriptor.labelKeys, }; switch (readableMetric.descriptor.type) { @@ -142,25 +194,10 @@ export class PrometheusExporter implements MetricExporter { case MetricDescriptorType.GAUGE_DOUBLE: case MetricDescriptorType.GAUGE_INT64: return new Prometheus.Gauge(metricObject); - case MetricDescriptorType.GAUGE_HISTOGRAM: - case MetricDescriptorType.CUMULATIVE_HISTOGRAM: - const histogramConfig = { - ...metricObject, - buckets: this._getHistogramBoundaries(readableMetric) - }; - return new Prometheus.Histogram(histogramConfig); - case MetricDescriptorType.SUMMARY: - return new Prometheus.Summary(metricObject); - case MetricDescriptorType.UNSPECIFIED: - this._logger.error("Do not use UNSPECIFIED readable metric type"); - return; + default: + // Other metric types are currently unimplemented + return undefined; } - - } - - private _getHistogramBoundaries(_readableMetric: ReadableMetric): number[] { - // TODO - throw new Error('Unimplemented') } private _getPrometheusMetricName(descriptor: MetricDescriptor): string { @@ -173,14 +210,15 @@ export class PrometheusExporter implements MetricExporter { return name.replace(/\W/g, '_'); } - /** * Stops the Prometheus exporter server * @param callback A callback that will be executed once the server is stopped */ stopServer(callback?: () => void) { if (!this._server) { - this._logger.debug(`Prometheus stopServer() was called but server was never started.`); + this._logger.debug( + `Prometheus stopServer() was called but server was never started.` + ); if (callback) { callback(); } @@ -199,25 +237,44 @@ export class PrometheusExporter implements MetricExporter { * @param callback A callback that will be called once the server is reade */ startServer(callback?: () => void) { - const requestHandler = ((request: IncomingMessage, response: ServerResponse) => { - const parsedUrl = url.parse(request.url as string); - if (parsedUrl.pathname === this._endpoint) { - response.statusCode = 200; - response.setHeader('content-type', this._registry.contentType); - response.end(this._registry.metrics()); - } else { - response.statusCode = 404; - response.end(); - } - - }); - this._server = createServer(requestHandler); this._server.listen(this._port, () => { - this._logger.debug(`Prometheus exporter started on port ${this._port} - at endpoint ${this._endpoint}`); + this._logger.debug( + `Prometheus exporter started on port ${this._port} at endpoint ${this._endpoint}` + ); if (callback) { callback(); } }); } + + /** + * Route request based on incoming message url + */ + private _requestHandler = ( + request: IncomingMessage, + response: ServerResponse + ) => { + if (url.parse(request.url!).pathname === this._endpoint) { + this._exportMetrics(response); + } else { + this._notFound(response); + } + }; + + /** + * Respond to incoming message with current state of all metrics + */ + private _exportMetrics = (response: ServerResponse) => { + response.statusCode = 200; + response.setHeader('content-type', this._registry.contentType); + response.end(this._registry.metrics()); + }; + + /** + * Respond with 404 status code to all requests that do not match the configured endpoint + */ + private _notFound = (response: ServerResponse) => { + response.statusCode = 404; + response.end(); + }; } diff --git a/packages/opentelemetry-exporter-prometheus/src/types.ts b/packages/opentelemetry-exporter-prometheus/src/types.ts index 8091dbf639..4f5866d2d1 100644 --- a/packages/opentelemetry-exporter-prometheus/src/types.ts +++ b/packages/opentelemetry-exporter-prometheus/src/types.ts @@ -17,7 +17,6 @@ import * as types from '@opentelemetry/types'; export interface ExporterConfig { - /** App prefix for metrics, if needed */ prefix?: string; @@ -25,16 +24,15 @@ export interface ExporterConfig { endpoint?: string; /** - * Port number for Prometheus exporter server - * Default registered port is 9464: - * https://github.com/prometheus/prometheus/wiki/Default-port-allocations - */ + * Port number for Prometheus exporter server + * Default registered port is 9464: + * https://github.com/prometheus/prometheus/wiki/Default-port-allocations + */ port?: number; - /** - * Define if the Prometheus exporter server will be started - default false - */ + /** Define if the Prometheus exporter server will be started - default false */ startServer?: boolean; + /** Standard logging interface */ logger?: types.Logger; } diff --git a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts index 4e2de0d67f..c2da1daf4e 100644 --- a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts @@ -15,11 +15,7 @@ */ import * as assert from 'assert'; -import * as chai from 'chai'; import * as http from 'http'; -import chaiHttp = require('chai-http'); - -chai.use(chaiHttp); import { PrometheusExporter } from '../src'; @@ -31,26 +27,33 @@ describe('PrometheusExporter', () => { assert.ok(typeof exporter.stopServer === 'function'); }); - it('should start the server if startServer is passed as an option', (done) => { + it('should start the server if startServer is passed as an option', done => { const port = PrometheusExporter.DEFAULT_OPTIONS.port; const endpoint = PrometheusExporter.DEFAULT_OPTIONS.endpoint; - const exporter = new PrometheusExporter({ - startServer: true, - }, () => { - const url = `http://localhost:${port}${endpoint}`; - http.get(url, function (res: any) { - assert.equal(res.statusCode, 200); - exporter.stopServer(() => { - return done(); + const exporter = new PrometheusExporter( + { + startServer: true, + }, + () => { + const url = `http://localhost:${port}${endpoint}`; + http.get(url, function(res: any) { + assert.equal(res.statusCode, 200); + exporter.stopServer(() => { + return done(); + }); }); - }); - }); + } + ); }); + it('should not start the server by default', () => { + const exporter = new PrometheusExporter(); + assert.ok(exporter["_server"]!.listening === false); + }); }); describe('server', () => { - it('it should start on startServer() and call the callback', (done) => { + it('it should start on startServer() and call the callback', done => { const exporter = new PrometheusExporter({ port: 9722, }); @@ -61,14 +64,14 @@ describe('PrometheusExporter', () => { }); }); - it('it should listen on the default port and default endpoint', (done) => { + it('it should listen on the default port and default endpoint', done => { const port = PrometheusExporter.DEFAULT_OPTIONS.port; const endpoint = PrometheusExporter.DEFAULT_OPTIONS.endpoint; const exporter = new PrometheusExporter(); exporter.startServer(() => { const url = `http://localhost:${port}${endpoint}`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 200); exporter.stopServer(() => { return done(); @@ -77,7 +80,7 @@ describe('PrometheusExporter', () => { }); }); - it('it should listen on a custom port and endpoint if provided', (done) => { + it('it should listen on a custom port and endpoint if provided', done => { const port = 9991; const endpoint = '/metric'; @@ -88,7 +91,7 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}${endpoint}`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 200); exporter.stopServer(() => { return done(); @@ -97,7 +100,7 @@ describe('PrometheusExporter', () => { }); }); - it('it should lnormalize an endpoint that doesn\'t start with a slash', (done) => { + it('it should not require endpoints to start with a slash', done => { const port = 9991; const endpoint = 'metric'; @@ -107,28 +110,40 @@ describe('PrometheusExporter', () => { }); exporter.startServer(() => { - const url = `http://localhost:${port}/${endpoint}`; - http.get(url, function (res: any) { + const url = `http://localhost:${port}/metric`; + http.get(url, function(res: any) { assert.equal(res.statusCode, 200); exporter.stopServer(() => { - return done(); + const exporter2 = new PrometheusExporter({ + port, + endpoint: `/${endpoint}`, + }); + + exporter2.startServer(() => { + const url = `http://localhost:${port}/metric`; + http.get(url, function(res: any) { + assert.equal(res.statusCode, 200); + exporter2.stopServer(() => { + return done(); + }); + }); + }); }); }); }); }); - it('it should return a HTTP status 404 if the endpoint does not match', (done) => { + it('it should return a HTTP status 404 if the endpoint does not match', done => { const port = 9912; - const endpoint = '/metricss'; + const endpoint = '/metrics'; const exporter = new PrometheusExporter({ port, endpoint, }); exporter.startServer(() => { - const url = - `http://localhost:${port}/metrics`; + const url = `http://localhost:${port}/invalid`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 404); exporter.stopServer(() => { return done(); @@ -137,7 +152,7 @@ describe('PrometheusExporter', () => { }); }); - it('should call a provided callback regardless of if the server is running', (done) => { + it('should call a provided callback regardless of if the server is running', done => { const exporter = new PrometheusExporter(); exporter.stopServer(() => { return done(); @@ -145,4 +160,3 @@ describe('PrometheusExporter', () => { }); }); }); - diff --git a/packages/opentelemetry-metrics/src/export/types.ts b/packages/opentelemetry-metrics/src/export/types.ts index 582d3b85a4..fd38522420 100644 --- a/packages/opentelemetry-metrics/src/export/types.ts +++ b/packages/opentelemetry-metrics/src/export/types.ts @@ -347,4 +347,7 @@ export interface MetricExporter { metrics: ReadableMetric[], resultCallback: (result: ExportResult) => void ): void; + + /** Stops the exporter. */ + shutdown(): void; } From f313cbd9962612674fbaeb7a8495812092e678af Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 4 Nov 2019 10:02:01 -0500 Subject: [PATCH 08/24] feat(prometheus-exporter): export configuration interface --- .../opentelemetry-exporter-prometheus/src/{ => export}/types.ts | 0 packages/opentelemetry-exporter-prometheus/src/index.ts | 1 + packages/opentelemetry-exporter-prometheus/src/prometheus.ts | 2 +- 3 files changed, 2 insertions(+), 1 deletion(-) rename packages/opentelemetry-exporter-prometheus/src/{ => export}/types.ts (100%) diff --git a/packages/opentelemetry-exporter-prometheus/src/types.ts b/packages/opentelemetry-exporter-prometheus/src/export/types.ts similarity index 100% rename from packages/opentelemetry-exporter-prometheus/src/types.ts rename to packages/opentelemetry-exporter-prometheus/src/export/types.ts diff --git a/packages/opentelemetry-exporter-prometheus/src/index.ts b/packages/opentelemetry-exporter-prometheus/src/index.ts index 449a2843b1..f919d97aa0 100644 --- a/packages/opentelemetry-exporter-prometheus/src/index.ts +++ b/packages/opentelemetry-exporter-prometheus/src/index.ts @@ -15,3 +15,4 @@ */ export * from './prometheus'; +export * from './export/types' diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index e38c34a8bf..8a21e638fa 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -27,7 +27,7 @@ import * as types from '@opentelemetry/types'; import { createServer, IncomingMessage, Server, ServerResponse } from 'http'; import * as Prometheus from 'prom-client'; import * as url from 'url'; -import { ExporterConfig } from './types'; +import { ExporterConfig } from './export/types'; export class PrometheusExporter implements MetricExporter { static readonly DEFAULT_OPTIONS = { From fa1d0176ea97e0a03b2571f4191034936e15aaaa Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 4 Nov 2019 14:54:45 -0500 Subject: [PATCH 09/24] fix: linting --- packages/opentelemetry-exporter-prometheus/src/index.ts | 2 +- packages/opentelemetry-exporter-prometheus/src/prometheus.ts | 2 +- .../opentelemetry-exporter-prometheus/test/prometheus.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/index.ts b/packages/opentelemetry-exporter-prometheus/src/index.ts index f919d97aa0..ff3d57cb44 100644 --- a/packages/opentelemetry-exporter-prometheus/src/index.ts +++ b/packages/opentelemetry-exporter-prometheus/src/index.ts @@ -15,4 +15,4 @@ */ export * from './prometheus'; -export * from './export/types' +export * from './export/types'; diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 8a21e638fa..43ea9b0bd4 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -66,7 +66,7 @@ export class PrometheusExporter implements MetricExporter { if (config.startServer || PrometheusExporter.DEFAULT_OPTIONS.startServer) { this.startServer(callback); } else if (callback) { - callback() + callback(); } } diff --git a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts index c2da1daf4e..90bbd012d9 100644 --- a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts @@ -48,7 +48,7 @@ describe('PrometheusExporter', () => { it('should not start the server by default', () => { const exporter = new PrometheusExporter(); - assert.ok(exporter["_server"]!.listening === false); + assert.ok(exporter['_server']!.listening === false); }); }); From bc6216721b9620a240cceb9962511921677b3b99 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 4 Nov 2019 15:51:25 -0500 Subject: [PATCH 10/24] fix: typo Co-Authored-By: Mayur Kale --- packages/opentelemetry-exporter-prometheus/src/prometheus.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 43ea9b0bd4..b4ab710ef7 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -74,7 +74,7 @@ export class PrometheusExporter implements MetricExporter { * Save current metric state so that it can be pulled by the metrics backend. * * @todo reach into metrics to pull metric values on endpoint - * In it's current state, the exporter saves the current values of all metrics when export + * In its current state, the exporter saves the current values of all metrics when export * is called and returns them when the export endpoint is called. In the future, this should * be a no-op and the exporter should reach into the metrics when the export endpoint is * called. As there is currently no interface to do this, this is our only option. From 39d53a5297fb1fa790dbbc0464071a00d5dc9c3f Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 4 Nov 2019 15:56:54 -0500 Subject: [PATCH 11/24] chore: document ExporterConfig --- .../src/export/types.ts | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/export/types.ts b/packages/opentelemetry-exporter-prometheus/src/export/types.ts index 4f5866d2d1..0630a35841 100644 --- a/packages/opentelemetry-exporter-prometheus/src/export/types.ts +++ b/packages/opentelemetry-exporter-prometheus/src/export/types.ts @@ -16,21 +16,36 @@ import * as types from '@opentelemetry/types'; +/** + * Configuration interface for prometheus exporter + */ export interface ExporterConfig { - /** App prefix for metrics, if needed */ + /** + * App prefix for metrics, if needed + * + * @default '' + * */ prefix?: string; - /** Endpoint the metrics should be exposed at with preceeding / */ + /** + * Endpoint the metrics should be exposed at with preceeding slash + * @default '/metrics' + */ endpoint?: string; /** * Port number for Prometheus exporter server + * * Default registered port is 9464: * https://github.com/prometheus/prometheus/wiki/Default-port-allocations + * @default 9464 */ port?: number; - /** Define if the Prometheus exporter server will be started - default false */ + /** + * Define if the Prometheus exporter server will be started + * @default false + */ startServer?: boolean; /** Standard logging interface */ From 9248b23f64a70df0d0231a84b0244e3be86359d2 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 4 Nov 2019 15:57:04 -0500 Subject: [PATCH 12/24] fix: dependencies --- packages/opentelemetry-exporter-prometheus/package.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/package.json b/packages/opentelemetry-exporter-prometheus/package.json index d1b5cbbf60..bc89e8bc28 100644 --- a/packages/opentelemetry-exporter-prometheus/package.json +++ b/packages/opentelemetry-exporter-prometheus/package.json @@ -1,7 +1,6 @@ { "name": "@opentelemetry/exporter-prometheus", "version": "0.2.0", - "private": true, "description": "OpenTelemetry Exporter Prometheus provides a metrics endpoint for Prometheus", "main": "build/src/index.js", "types": "build/src/index.d.ts", @@ -47,6 +46,7 @@ "rimraf": "^3.0.0", "ts-mocha": "^6.0.0", "ts-node": "^8.3.0", + "tslint-consistent-codestyle": "^1.16.0", "tslint-microsoft-contrib": "^6.2.0", "typescript": "^3.6.3" }, @@ -55,7 +55,6 @@ "@opentelemetry/core": "^0.2.0", "@opentelemetry/metrics": "^0.2.0", "@opentelemetry/types": "^0.2.0", - "prom-client": "^11.5.3", - "tslint-consistent-codestyle": "^1.16.0" + "prom-client": "^11.5.3" } } From ba9c1fe646844a1dd1c2f11d3f232bbfb15fd876 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 4 Nov 2019 20:12:55 -0500 Subject: [PATCH 13/24] fix: typo Co-Authored-By: Mayur Kale --- packages/opentelemetry-exporter-prometheus/src/prometheus.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index b4ab710ef7..4329b88b14 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -234,7 +234,7 @@ export class PrometheusExporter implements MetricExporter { /** * Starts the Prometheus exporter server and registers the request handler - * @param callback A callback that will be called once the server is reade + * @param callback A callback that will be called once the server is ready */ startServer(callback?: () => void) { this._server.listen(this._port, () => { From 07c648fc92ed4b1a8ab7bbe53649746982d666c2 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 09:51:41 -0500 Subject: [PATCH 14/24] chore: document sanitize method and make behavior more strict --- .../src/export/types.ts | 2 +- .../src/prometheus.ts | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/export/types.ts b/packages/opentelemetry-exporter-prometheus/src/export/types.ts index 0630a35841..69ac40cb13 100644 --- a/packages/opentelemetry-exporter-prometheus/src/export/types.ts +++ b/packages/opentelemetry-exporter-prometheus/src/export/types.ts @@ -45,7 +45,7 @@ export interface ExporterConfig { /** * Define if the Prometheus exporter server will be started * @default false - */ + */ startServer?: boolean; /** Standard logging interface */ diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index b4ab710ef7..0cdf0a2613 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -206,8 +206,22 @@ export class PrometheusExporter implements MetricExporter { ); } + /** + * Remove characters that are invalid in prometheus metric names. + * + * https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels + * + * 1. Names must match `[a-zA-Z_:][a-zA-Z0-9_:]*` + * + * 2. Colons are reserved for user defined recording rules. + * They should not be used by exporters or direct instrumentation. + * + * @param name name to be sanitized + */ private _sanitizePrometheusMetricName(name: string): string { - return name.replace(/\W/g, '_'); + return name + .replace(/^([^a-zA-Z_].+)/, '_$1') // starts with [a-zA-Z_] + .replace(/[^a-zA-Z0-9_]/g, '_'); // replace all invalid characters with '_' } /** From c60bc21d2ff70ab0d13b8d33dfc3957ed0da88a9 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 12:01:10 -0500 Subject: [PATCH 15/24] chore: do not use global prom-client registry --- .../src/prometheus.ts | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index c896a533df..2d2e6f222b 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -25,7 +25,7 @@ import { } from '@opentelemetry/metrics'; import * as types from '@opentelemetry/types'; import { createServer, IncomingMessage, Server, ServerResponse } from 'http'; -import * as Prometheus from 'prom-client'; +import { Counter, Gauge, labelValues, Metric, Registry } from 'prom-client'; import * as url from 'url'; import { ExporterConfig } from './export/types'; @@ -37,7 +37,7 @@ export class PrometheusExporter implements MetricExporter { prefix: '', }; - private readonly _registry = new Prometheus.Registry(); + private readonly _registry = new Registry(); private readonly _logger: types.Logger; private readonly _port: number; private readonly _endpoint: string; @@ -105,8 +105,9 @@ export class PrometheusExporter implements MetricExporter { /** * Shut down the export server */ - shutdown() { - this.stopServer(); + shutdown(cb?: () => void) { + this._registry.clear(); + this.stopServer(cb); } /** @@ -120,7 +121,7 @@ export class PrometheusExporter implements MetricExporter { const labelKeys = readableMetric.descriptor.labelKeys; - if (metric instanceof Prometheus.Counter) { + if (metric instanceof Counter) { for (const ts of readableMetric.timeseries) { // Prometheus counter saves internal state and increments by given value. // ReadableMetric value is the current state, not the delta to be incremented by. @@ -131,7 +132,7 @@ export class PrometheusExporter implements MetricExporter { } } - if (metric instanceof Prometheus.Gauge) { + if (metric instanceof Gauge) { for (const ts of readableMetric.timeseries) { metric.set(this._getLabelValues(labelKeys, ts.labelValues), ts.points[0] .value as number); @@ -142,7 +143,7 @@ export class PrometheusExporter implements MetricExporter { } private _getLabelValues(keys: string[], values: LabelValue[]) { - const labelValues: Prometheus.labelValues = {}; + const labelValues: labelValues = {}; for (let i = 0; i < keys.length; i++) { if (values[i].value !== null) { labelValues[keys[i]] = values[i].value!; @@ -151,9 +152,7 @@ export class PrometheusExporter implements MetricExporter { return labelValues; } - private _registerMetric( - readableMetric: ReadableMetric - ): Prometheus.Metric | undefined { + private _registerMetric(readableMetric: ReadableMetric): Metric | undefined { const metricName = this._getPrometheusMetricName(readableMetric.descriptor); const metric = this._registry.getSingleMetric(metricName); @@ -166,34 +165,32 @@ export class PrometheusExporter implements MetricExporter { * This works because counters are identified by their name and no other internal ID * https://prometheus.io/docs/instrumenting/exposition_formats/ */ - if (metric instanceof Prometheus.Counter) { + if (metric instanceof Counter) { this._registry.removeSingleMetric(metricName); } else if (metric) return metric; - const newMetric = this._newMetric(readableMetric, metricName); - if (!newMetric) return; - - this._registry.registerMetric(newMetric); - return newMetric; + return this._newMetric(readableMetric, metricName); } private _newMetric( readableMetric: ReadableMetric, name: string - ): Prometheus.Metric | undefined { + ): Metric | undefined { const metricObject = { name, help: readableMetric.descriptor.description, labelNames: readableMetric.descriptor.labelKeys, + // list of registries to register the newly created metric + registers: [this._registry], }; switch (readableMetric.descriptor.type) { case MetricDescriptorType.COUNTER_DOUBLE: case MetricDescriptorType.COUNTER_INT64: - return new Prometheus.Counter(metricObject); + return new Counter(metricObject); case MetricDescriptorType.GAUGE_DOUBLE: case MetricDescriptorType.GAUGE_INT64: - return new Prometheus.Gauge(metricObject); + return new Gauge(metricObject); default: // Other metric types are currently unimplemented return undefined; @@ -209,7 +206,7 @@ export class PrometheusExporter implements MetricExporter { /** * Remove characters that are invalid in prometheus metric names. * - * https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels + * https://io/docs/concepts/data_model/#metric-names-and-labels * * 1. Names must match `[a-zA-Z_:][a-zA-Z0-9_:]*` * From a67dc36e21735b1e3fc1cd547df26d70658cad1a Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 12:01:36 -0500 Subject: [PATCH 16/24] chore: add defaults to description and metric endpoint --- packages/opentelemetry-exporter-prometheus/src/prometheus.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 2d2e6f222b..45934a53a5 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -178,7 +178,8 @@ export class PrometheusExporter implements MetricExporter { ): Metric | undefined { const metricObject = { name, - help: readableMetric.descriptor.description, + // prom-client throws with empty description which is our default + help: readableMetric.descriptor.description || 'description missing', labelNames: readableMetric.descriptor.labelKeys, // list of registries to register the newly created metric registers: [this._registry], @@ -278,7 +279,7 @@ export class PrometheusExporter implements MetricExporter { private _exportMetrics = (response: ServerResponse) => { response.statusCode = 200; response.setHeader('content-type', this._registry.contentType); - response.end(this._registry.metrics()); + response.end(this._registry.metrics() || '# no registered metrics'); }; /** From 14ac445e0f4ff7fda05657da4795a302a706d1a5 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 12:02:36 -0500 Subject: [PATCH 17/24] test: export couter, gauge, multiple, and none --- .../test/prometheus.test.ts | 203 ++++++++++++++++-- 1 file changed, 188 insertions(+), 15 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts index 90bbd012d9..548b8fd798 100644 --- a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts @@ -14,9 +14,9 @@ * limitations under the License. */ +import { CounterMetric, GaugeMetric, Meter } from '@opentelemetry/metrics'; import * as assert from 'assert'; import * as http from 'http'; - import { PrometheusExporter } from '../src'; describe('PrometheusExporter', () => { @@ -24,7 +24,7 @@ describe('PrometheusExporter', () => { it('should construct an exporter', () => { const exporter = new PrometheusExporter(); assert.ok(typeof exporter.startServer === 'function'); - assert.ok(typeof exporter.stopServer === 'function'); + assert.ok(typeof exporter.shutdown === 'function'); }); it('should start the server if startServer is passed as an option', done => { @@ -36,9 +36,9 @@ describe('PrometheusExporter', () => { }, () => { const url = `http://localhost:${port}${endpoint}`; - http.get(url, function(res: any) { + http.get(url, function (res: any) { assert.equal(res.statusCode, 200); - exporter.stopServer(() => { + exporter.shutdown(() => { return done(); }); }); @@ -58,7 +58,7 @@ describe('PrometheusExporter', () => { port: 9722, }); exporter.startServer(() => { - exporter.stopServer(() => { + exporter.shutdown(() => { return done(); }); }); @@ -71,9 +71,9 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}${endpoint}`; - http.get(url, function(res: any) { + http.get(url, function (res: any) { assert.equal(res.statusCode, 200); - exporter.stopServer(() => { + exporter.shutdown(() => { return done(); }); }); @@ -91,9 +91,9 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}${endpoint}`; - http.get(url, function(res: any) { + http.get(url, function (res: any) { assert.equal(res.statusCode, 200); - exporter.stopServer(() => { + exporter.shutdown(() => { return done(); }); }); @@ -111,9 +111,9 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}/metric`; - http.get(url, function(res: any) { + http.get(url, function (res: any) { assert.equal(res.statusCode, 200); - exporter.stopServer(() => { + exporter.shutdown(() => { const exporter2 = new PrometheusExporter({ port, endpoint: `/${endpoint}`, @@ -121,7 +121,7 @@ describe('PrometheusExporter', () => { exporter2.startServer(() => { const url = `http://localhost:${port}/metric`; - http.get(url, function(res: any) { + http.get(url, function (res: any) { assert.equal(res.statusCode, 200); exporter2.stopServer(() => { return done(); @@ -143,9 +143,9 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}/invalid`; - http.get(url, function(res: any) { + http.get(url, function (res: any) { assert.equal(res.statusCode, 404); - exporter.stopServer(() => { + exporter.shutdown(() => { return done(); }); }); @@ -154,9 +154,182 @@ describe('PrometheusExporter', () => { it('should call a provided callback regardless of if the server is running', done => { const exporter = new PrometheusExporter(); - exporter.stopServer(() => { + exporter.shutdown(() => { return done(); }); }); }); + + describe('export', () => { + let exporter: PrometheusExporter; + let meter: Meter; + + beforeEach(done => { + exporter = new PrometheusExporter(); + meter = new Meter(); + exporter.startServer(done); + }); + + afterEach(done => { + exporter.shutdown(done); + }); + + it('should export a count aggregation', done => { + const counter = meter.createCounter('counter', { + description: 'a test description', + labelKeys: ['key1'], + }) as CounterMetric; + + const handle = counter.getHandle(['labelValue1']); + handle.add(10); + exporter.export([counter.get()!], () => { + // This is to test the special case where counters are destroyed + // and recreated in the exporter in order to get around prom-client's + // aggregation and use ours. + handle.add(10); + exporter.export([counter.get()!], () => { + http + .get('http://localhost:9464/metrics', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.strictEqual( + lines[0], + '# HELP counter a test description' + ); + + assert.deepEqual(lines, [ + '# HELP counter a test description', + '# TYPE counter counter', + 'counter{key1="labelValue1"} 20', + '', + ]); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); + }); + + it('should export a gauge aggregation', done => { + const gauge = meter.createGauge('gauge', { + description: 'a test description', + labelKeys: ['key1'], + }) as GaugeMetric; + + const handle = gauge.getHandle(['labelValue1']); + handle.set(10); + exporter.export([gauge.get()!], () => { + http + .get('http://localhost:9464/metrics', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.deepEqual(lines, [ + '# HELP gauge a test description', + '# TYPE gauge gauge', + 'gauge{key1="labelValue1"} 10', + '', + ]); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); + + it('should export a multiple aggregations', done => { + const gauge = meter.createGauge('gauge', { + description: 'a test description', + labelKeys: ['gaugeKey1'], + }) as GaugeMetric; + + const counter = meter.createCounter('counter', { + description: 'a test description', + labelKeys: ['counterKey1'], + }) as CounterMetric; + + gauge.getHandle(['labelValue1']).set(10); + counter.getHandle(['labelValue1']).add(10); + exporter.export([gauge.get()!, counter.get()!], () => { + http + .get('http://localhost:9464/metrics', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.deepEqual(lines, [ + '# HELP gauge a test description', + '# TYPE gauge gauge', + 'gauge{gaugeKey1="labelValue1"} 10', + '', + '# HELP counter a test description', + '# TYPE counter counter', + 'counter{counterKey1="labelValue1"} 10', + '', + ]); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); + + it('should export a comment if no metrics are registered', done => { + exporter.export([], () => { + http + .get('http://localhost:9464/metrics', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.deepEqual(lines, ['# no registered metrics']); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); + + it('should add a description if missing', done => { + const gauge = meter.createGauge('gauge') as GaugeMetric; + + + const handle = gauge.getHandle(['labelValue1']); + handle.set(10); + exporter.export([gauge.get()!], () => { + http + .get('http://localhost:9464/metrics', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.deepEqual(lines, [ + '# HELP gauge description missing', + '# TYPE gauge gauge', + 'gauge 10', + '', + ]); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); + }); }); + +function errorHandler(done: Mocha.Done): (err: Error) => void { + return () => { + assert.ok(false, 'error getting metrics'); + done(); + }; +} From 9720452a698ddef4d5f4d694767645b62ea62b42 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 12:16:55 -0500 Subject: [PATCH 18/24] test: sanitize prom metric names --- .../src/prometheus.ts | 10 +++-- .../test/prometheus.test.ts | 38 +++++++++++++++---- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 45934a53a5..6b6ddaf81e 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -43,6 +43,7 @@ export class PrometheusExporter implements MetricExporter { private readonly _endpoint: string; private _server: Server; private readonly _prefix?: string; + private readonly _invalidCharacterRegex = /[^a-z0-9_]/gi; // This will be required when histogram is implemented. Leaving here so it is not forgotten // Histogram cannot have a label named 'le' @@ -214,12 +215,15 @@ export class PrometheusExporter implements MetricExporter { * 2. Colons are reserved for user defined recording rules. * They should not be used by exporters or direct instrumentation. * + * OpenTelemetry metric names are already validated in the Meter when they are created, + * and they match the format `[a-zA-Z][a-zA-Z0-9_.\-]*` which is very close to a valid + * prometheus metric name, so we only need to strip characters valid in OpenTelemetry + * but not valid in prometheus and replace them with '_'. + * * @param name name to be sanitized */ private _sanitizePrometheusMetricName(name: string): string { - return name - .replace(/^([^a-zA-Z_].+)/, '_$1') // starts with [a-zA-Z_] - .replace(/[^a-zA-Z0-9_]/g, '_'); // replace all invalid characters with '_' + return name.replace(this._invalidCharacterRegex, '_'); // replace all invalid characters with '_' } /** diff --git a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts index 548b8fd798..a328c34b5a 100644 --- a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts @@ -36,7 +36,7 @@ describe('PrometheusExporter', () => { }, () => { const url = `http://localhost:${port}${endpoint}`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 200); exporter.shutdown(() => { return done(); @@ -71,7 +71,7 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}${endpoint}`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 200); exporter.shutdown(() => { return done(); @@ -91,7 +91,7 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}${endpoint}`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 200); exporter.shutdown(() => { return done(); @@ -111,7 +111,7 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}/metric`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 200); exporter.shutdown(() => { const exporter2 = new PrometheusExporter({ @@ -121,7 +121,7 @@ describe('PrometheusExporter', () => { exporter2.startServer(() => { const url = `http://localhost:${port}/metric`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 200); exporter2.stopServer(() => { return done(); @@ -143,7 +143,7 @@ describe('PrometheusExporter', () => { exporter.startServer(() => { const url = `http://localhost:${port}/invalid`; - http.get(url, function (res: any) { + http.get(url, function(res: any) { assert.equal(res.statusCode, 404); exporter.shutdown(() => { return done(); @@ -301,7 +301,6 @@ describe('PrometheusExporter', () => { it('should add a description if missing', done => { const gauge = meter.createGauge('gauge') as GaugeMetric; - const handle = gauge.getHandle(['labelValue1']); handle.set(10); exporter.export([gauge.get()!], () => { @@ -324,6 +323,31 @@ describe('PrometheusExporter', () => { .on('error', errorHandler(done)); }); }); + + it('should sanitize names', done => { + const gauge = meter.createGauge('gauge.bad-name') as GaugeMetric; + const handle = gauge.getHandle(['labelValue1']); + handle.set(10); + exporter.export([gauge.get()!], () => { + http + .get('http://localhost:9464/metrics', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.deepEqual(lines, [ + '# HELP gauge_bad_name description missing', + '# TYPE gauge_bad_name gauge', + 'gauge_bad_name 10', + '', + ]); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); }); }); From c3e5e90e042385ba85398cabf12232a9941ba149 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 13:19:01 -0500 Subject: [PATCH 19/24] fix: typo Co-Authored-By: Mayur Kale --- packages/opentelemetry-exporter-prometheus/src/prometheus.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 6b6ddaf81e..27d26bba82 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -208,7 +208,7 @@ export class PrometheusExporter implements MetricExporter { /** * Remove characters that are invalid in prometheus metric names. * - * https://io/docs/concepts/data_model/#metric-names-and-labels + * https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels * * 1. Names must match `[a-zA-Z_:][a-zA-Z0-9_:]*` * From 9d9dd45047cd0a30debe2345cd1d0c2738ff1f5e Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 14:22:30 -0500 Subject: [PATCH 20/24] chore: make _server read only --- packages/opentelemetry-exporter-prometheus/src/prometheus.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 6b6ddaf81e..eb56a8bec6 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -41,7 +41,7 @@ export class PrometheusExporter implements MetricExporter { private readonly _logger: types.Logger; private readonly _port: number; private readonly _endpoint: string; - private _server: Server; + private readonly _server: Server; private readonly _prefix?: string; private readonly _invalidCharacterRegex = /[^a-z0-9_]/gi; From 1f824cfe5612a976e5bd0090c1c211b4aa8e4d26 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 14:35:23 -0500 Subject: [PATCH 21/24] test: add tests for prom export configuration --- .../test/prometheus.test.ts | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts index a328c34b5a..03476153d4 100644 --- a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts @@ -349,6 +349,111 @@ describe('PrometheusExporter', () => { }); }); }); + + describe('configuration', () => { + let meter: Meter; + let gauge: GaugeMetric; + let exporter: PrometheusExporter | undefined; + + beforeEach(() => { + meter = new Meter(); + gauge = meter.createGauge('gauge') as GaugeMetric; + gauge.getHandle(['labelValue1']).set(10); + }); + + afterEach(done => { + if (exporter) { + exporter.shutdown(done); + exporter = undefined; + } else { + done(); + } + }); + + it('should use a configured name prefix', done => { + exporter = new PrometheusExporter({ + prefix: 'test_prefix', + }); + + exporter.startServer(() => { + exporter!.export(meter.getMetrics(), () => { + http + .get('http://localhost:9464/metrics', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.deepEqual(lines, [ + '# HELP test_prefix_gauge description missing', + '# TYPE test_prefix_gauge gauge', + 'test_prefix_gauge 10', + '', + ]); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); + }); + + it('should use a configured port', done => { + exporter = new PrometheusExporter({ + port: 8080, + }); + + exporter.startServer(() => { + exporter!.export(meter.getMetrics(), () => { + http + .get('http://localhost:8080/metrics', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.deepEqual(lines, [ + '# HELP gauge description missing', + '# TYPE gauge gauge', + 'gauge 10', + '', + ]); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); + }); + + it('should use a configured endpoint', done => { + exporter = new PrometheusExporter({ + endpoint: '/test', + }); + + exporter.startServer(() => { + exporter!.export(meter.getMetrics(), () => { + http + .get('http://localhost:9464/test', res => { + res.on('data', chunk => { + const body = chunk.toString(); + const lines = body.split('\n'); + + assert.deepEqual(lines, [ + '# HELP gauge description missing', + '# TYPE gauge gauge', + 'gauge 10', + '', + ]); + + done(); + }); + }) + .on('error', errorHandler(done)); + }); + }); + }); + }); }); function errorHandler(done: Mocha.Done): (err: Error) => void { From ccd5ce434da0e4f2852b184e24067ca9babfc838 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Tue, 5 Nov 2019 14:59:57 -0500 Subject: [PATCH 22/24] style: make comments descriptive --- .../src/prometheus.ts | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index eb56a8bec6..548d069000 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -72,7 +72,8 @@ export class PrometheusExporter implements MetricExporter { } /** - * Save current metric state so that it can be pulled by the metrics backend. + * Saves the current values of all exported {@link ReadableMetric}s so that they can be pulled + * by the Prometheus backend. * * @todo reach into metrics to pull metric values on endpoint * In its current state, the exporter saves the current values of all metrics when export @@ -104,7 +105,9 @@ export class PrometheusExporter implements MetricExporter { } /** - * Shut down the export server + * Shuts down the export server and clears the registry + * + * @param cb called when server is stopped */ shutdown(cb?: () => void) { this._registry.clear(); @@ -112,7 +115,7 @@ export class PrometheusExporter implements MetricExporter { } /** - * Save the value of one metric to be exported + * Updates the value of a single metric in the registry * * @param readableMetric Metric value to be saved */ @@ -206,7 +209,8 @@ export class PrometheusExporter implements MetricExporter { } /** - * Remove characters that are invalid in prometheus metric names. + * Ensures metric names are valid Prometheus metric names by removing + * characters allowed by OpenTelemetry but disallowed by Prometheus. * * https://io/docs/concepts/data_model/#metric-names-and-labels * @@ -227,7 +231,7 @@ export class PrometheusExporter implements MetricExporter { } /** - * Stops the Prometheus exporter server + * Stops the Prometheus export server * @param callback A callback that will be executed once the server is stopped */ stopServer(callback?: () => void) { @@ -249,8 +253,9 @@ export class PrometheusExporter implements MetricExporter { } /** - * Starts the Prometheus exporter server and registers the request handler - * @param callback A callback that will be called once the server is ready + * Starts the Prometheus export server + * + * @param callback called once the server is ready */ startServer(callback?: () => void) { this._server.listen(this._port, () => { @@ -264,7 +269,11 @@ export class PrometheusExporter implements MetricExporter { } /** - * Route request based on incoming message url + * Request handler used by http library to respond to incoming requests + * for the current state of metrics by the Prometheus backend. + * + * @param request Incoming HTTP request to export server + * @param response HTTP response object used to respond to request */ private _requestHandler = ( request: IncomingMessage, @@ -278,7 +287,7 @@ export class PrometheusExporter implements MetricExporter { }; /** - * Respond to incoming message with current state of all metrics + * Responds to incoming message with current state of all metrics. */ private _exportMetrics = (response: ServerResponse) => { response.statusCode = 200; @@ -287,7 +296,7 @@ export class PrometheusExporter implements MetricExporter { }; /** - * Respond with 404 status code to all requests that do not match the configured endpoint + * Responds with 404 status code to all requests that do not match the configured endpoint. */ private _notFound = (response: ServerResponse) => { response.statusCode = 404; From 129023e8b790d3a4f8a9ae3ef5d7b1754316035f Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Fri, 8 Nov 2019 14:56:35 -0500 Subject: [PATCH 23/24] test: update tests for label sets --- .../test/prometheus.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts index 03476153d4..984a5feb4e 100644 --- a/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts +++ b/packages/opentelemetry-exporter-prometheus/test/prometheus.test.ts @@ -180,14 +180,14 @@ describe('PrometheusExporter', () => { labelKeys: ['key1'], }) as CounterMetric; - const handle = counter.getHandle(['labelValue1']); + const handle = counter.getHandle(meter.labels({ key1: 'labelValue1' })); handle.add(10); - exporter.export([counter.get()!], () => { + exporter.export(meter.getMetrics(), () => { // This is to test the special case where counters are destroyed // and recreated in the exporter in order to get around prom-client's // aggregation and use ours. handle.add(10); - exporter.export([counter.get()!], () => { + exporter.export(meter.getMetrics(), () => { http .get('http://localhost:9464/metrics', res => { res.on('data', chunk => { @@ -220,7 +220,7 @@ describe('PrometheusExporter', () => { labelKeys: ['key1'], }) as GaugeMetric; - const handle = gauge.getHandle(['labelValue1']); + const handle = gauge.getHandle(meter.labels({ key1: 'labelValue1' })); handle.set(10); exporter.export([gauge.get()!], () => { http @@ -254,8 +254,8 @@ describe('PrometheusExporter', () => { labelKeys: ['counterKey1'], }) as CounterMetric; - gauge.getHandle(['labelValue1']).set(10); - counter.getHandle(['labelValue1']).add(10); + gauge.getHandle(meter.labels({ key1: 'labelValue1' })).set(10); + counter.getHandle(meter.labels({ key1: 'labelValue1' })).add(10); exporter.export([gauge.get()!, counter.get()!], () => { http .get('http://localhost:9464/metrics', res => { @@ -301,7 +301,7 @@ describe('PrometheusExporter', () => { it('should add a description if missing', done => { const gauge = meter.createGauge('gauge') as GaugeMetric; - const handle = gauge.getHandle(['labelValue1']); + const handle = gauge.getHandle(meter.labels({ key1: 'labelValue1' })); handle.set(10); exporter.export([gauge.get()!], () => { http @@ -326,7 +326,7 @@ describe('PrometheusExporter', () => { it('should sanitize names', done => { const gauge = meter.createGauge('gauge.bad-name') as GaugeMetric; - const handle = gauge.getHandle(['labelValue1']); + const handle = gauge.getHandle(meter.labels({ key1: 'labelValue1' })); handle.set(10); exporter.export([gauge.get()!], () => { http @@ -358,7 +358,7 @@ describe('PrometheusExporter', () => { beforeEach(() => { meter = new Meter(); gauge = meter.createGauge('gauge') as GaugeMetric; - gauge.getHandle(['labelValue1']).set(10); + gauge.getHandle(meter.labels({ key1: 'labelValue1' })).set(10); }); afterEach(done => { From 20efbba6edcd82cfab6dbc2448a84c85c990a7a5 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 11 Nov 2019 15:18:03 -0500 Subject: [PATCH 24/24] fix: lint --- .../src/prometheus.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts index 4af6471295..69f4545909 100644 --- a/packages/opentelemetry-exporter-prometheus/src/prometheus.ts +++ b/packages/opentelemetry-exporter-prometheus/src/prometheus.ts @@ -131,15 +131,19 @@ export class PrometheusExporter implements MetricExporter { // ReadableMetric value is the current state, not the delta to be incremented by. // Currently, _registerMetric creates a new counter every time the value changes, // so the increment here behaves as a set value (increment from 0) - metric.inc(this._getLabelValues(labelKeys, ts.labelValues), ts.points[0] - .value as number); + metric.inc( + this._getLabelValues(labelKeys, ts.labelValues), + ts.points[0].value as number + ); } } if (metric instanceof Gauge) { for (const ts of readableMetric.timeseries) { - metric.set(this._getLabelValues(labelKeys, ts.labelValues), ts.points[0] - .value as number); + metric.set( + this._getLabelValues(labelKeys, ts.labelValues), + ts.points[0].value as number + ); } }