Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Collectors can now be async #304

Merged
merged 2 commits into from
Jan 29, 2021
Merged

Collectors can now be async #304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
266e8ad
Collectors can now be async
Half-Shot Jan 25, 2021
51846f4
Reinstate refresh as a public method
Half-Shot Jan 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/304.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fetch metrics asynchronously
6 changes: 3 additions & 3 deletions src/bridge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1404,9 +1404,9 @@ export class Bridge {
* }
* })
*/
public registerBridgeGauges(counterFunc: () => BridgeGaugesCounts) {
this.getPrometheusMetrics().registerBridgeGauges(() => {
const counts = counterFunc();
public registerBridgeGauges(counterFunc: () => Promise<BridgeGaugesCounts>|BridgeGaugesCounts) {
this.getPrometheusMetrics().registerBridgeGauges(async () => {
const counts = await counterFunc();
if (counts.matrixGhosts !== undefined) {
counts.matrixGhosts = Object.keys(this.intents.size).length;
}
Expand Down
32 changes: 21 additions & 11 deletions src/components/prometheusmetrics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import { AgeCounters } from "./agecounters";
import JsSdk from "matrix-js-sdk";
import { Request, Response } from "express";
import { Bridge } from "..";
type CollectorFunction = () => void;
import Logger from "./logging";
type CollectorFunction = () => Promise<void>|void;

export interface BridgeGaugesCounts {
matrixRoomConfigs?: number;
Expand Down Expand Up @@ -100,6 +101,9 @@ interface GagueOpts {
*
* @constructor
*/

const log = Logger.get('PrometheusMetrics');

export class PrometheusMetrics {
public static AgeCounters = AgeCounters;
private timers: {[name: string]: PromClient.Histogram<string>} = {};
Expand Down Expand Up @@ -173,13 +177,25 @@ export class PrometheusMetrics {
});
}

/**
* Fetch metrics from all configured collectors
*/
public async refresh () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does refresh need a lock (or wrapper returning an ongoing promise) so it cannot get run more than once at a time?

Copy link
Contributor Author

@Half-Shot Half-Shot Jan 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're okay in this case. The caller should manage that, whether that be a bridge or the application requesting the /metrics endpoint. I don't think a race will cause any serious problems as you're just asking it to refresh metrics , which should be RO functions.

try {
await Promise.all(this.collectors.map((f) => f()));
}
catch (ex) {
log.warn(`Failed to refresh metrics:`, ex);
}
}

/**
* Registers some exported metrics that expose counts of various kinds of
* objects within the bridge.
* @param {BridgeGaugesCallback} counterFunc A function that when invoked
* returns the current counts of various items in the bridge.
*/
public registerBridgeGauges (counterFunc: () => BridgeGaugesCounts) {
public async registerBridgeGauges (counterFunc: () => Promise<BridgeGaugesCounts>|BridgeGaugesCounts) {
const matrixRoomsGauge = this.addGauge({
name: "matrix_configured_rooms",
help: "Current count of configured rooms by matrix room ID",
Expand Down Expand Up @@ -220,8 +236,8 @@ export class PrometheusMetrics {
labels: ["age"],
});

this.addCollector(function () {
const counts = counterFunc();
this.addCollector(async () => {
const counts = await counterFunc();

if (counts.matrixRoomConfigs) {matrixRoomsGauge.set(counts.matrixRoomConfigs);}

Expand All @@ -239,10 +255,6 @@ export class PrometheusMetrics {
});
}

public refresh () {
this.collectors.forEach((f) => f());
}

/**
* Adds a new collector function. These collector functions are run whenever
* the /metrics page is about to be generated, allowing code to update values
Expand Down Expand Up @@ -381,17 +393,15 @@ export class PrometheusMetrics {
// For now, leave this unauthenticated.
checkToken: false,
handler: async (_req: Request, res: Response) => {
this.refresh();

try {
await this.refresh();
const exposition = await this.register.metrics();

res.set("Content-Type", "text/plain");
res.send(exposition);
}
catch (e) {
res.status(500);

res.set("Content-Type", "text/plain");
res.send(e.toString());
}
Expand Down