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.

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

consolidating collect functions for multiple metrics #538

Open
InsOpDe opened this issue Jan 31, 2023 · 4 comments
Open

consolidating collect functions for multiple metrics #538

InsOpDe opened this issue Jan 31, 2023 · 4 comments

Comments

@InsOpDe
Copy link

InsOpDe commented Jan 31, 2023

I have the following example:

const employeeMetric = new promClient.Gauge({
			async collect(): Promise<void> {
				const employees = await that.getAllEmployees()
				this.set({}, employees.length)
			},
			help: "Employees",
			labelNames: ["service"],
			name: `employees_all`
		})
const employeeOvertimeHoursMetric = new promClient.Gauge({
			async collect(): Promise<void> {
				const employees = await that.getAllEmployees()
				const overTimeHours = employees.reduce((acc, curr) => acc + curr.overTimeHours, 0)
				this.set({}, overTimeHours)
			},
			help: "Employees overtime hours",
			labelNames: ["service"],
			name: `employees_overtime_hours`
		})

Now getAllEmployees is a very expensive query, which takes some time to retrieve. And that's rather bad if we have to call this method for each metric.

I could however add to the second collect-function the following snippet:

employeeMetric.set({}, employees.length)

and leave out the collect-function of the first metric. But now we have a running condition - which collect function is called first, and is it fast enough? should I rather add a setTimeout(Promise.resolve(),500) into the first collect-function so it doesnt resolve before the value is set in the second collect-function?

now I have more metrics I want to collect for the hypothetical employees

what is the best way to solve this?

@zbjornson
Copy link
Collaborator

I think we'd need to add some registry-level hook/event for this that tells you "collect is about to be called," but that has some risk too if there are concurrent collections for some reason.

Could you do something like this as a workaround?

let employees;
let nCollected = 0;

// in each metric
collect() {
  if (!employees)
    employees = await getAllEmployees();
  this.set({}, /* value */);
  nCollected++;
  if (nCollected === 2) {
    employees = null;
    nCollected = 0;
  }
}

@InsOpDe
Copy link
Author

InsOpDe commented Mar 19, 2023

thats indeed a workaround - thanks.

(Id be happier if a better solution exists - maybe something where a collection can hook onto another collection)


however I will just cache getAllEmployees with memoize

@jseparovic
Copy link

jseparovic commented May 21, 2023

Would be nice to be able to create multiple metrics at the same time using a common collect function.

Something like:

new Gauges(
    {
        metrics: [
            {    
                name: "mode_normal",
                help: 'Total number of hosts in Normal Mode',
            },
            {
                name: "mode_low_power",
                help: 'Total number of hosts in Low Power Mode',
            },
            {
                name: "mode_sleep",
                help: 'Total number of hosts in Sleep Mode',
            }
        ],
        async collect() {
            const modes = await getHostMode();
            this.set([
                {
                    name: 'mode_normal',
                    value: modes.Normal
                },
                {
                    name: 'mode_low_power',
                    value: modes.LowPower
                },
                {
                    name: 'mode_sleep',
                    value: modes.Sleep
                },
            ]);
        }
});

Then when collect runs, the set function could update all metrics at once.

@jseparovic
Copy link

Would be nice to be able to create multiple metrics at the same time using a common collect function.

Something like:

new Gauges(
    {
        metrics: [
            {    
                name: "mode_normal",
                help: 'Total number of hosts in Normal Mode',
            },
            {
                name: "mode_low_power",
                help: 'Total number of hosts in Low Power Mode',
            },
            {
                name: "mode_sleep",
                help: 'Total number of hosts in Sleep Mode',
            }
        ],
        async collect() {
            const modes = await getHostMode();
            this.set([
                {
                    name: 'mode_normal',
                    value: modes.Normal
                },
                {
                    name: 'mode_low_power',
                    value: modes.LowPower
                },
                {
                    name: 'mode_sleep',
                    value: modes.Sleep
                },
            ]);
        }
});

Then when collect runs, the set function could update all metrics at once.

Just realized I should be using labels for this instead, not multiple metrics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants