improve performance of registry.metrics()

[doochik]

I've rebased original PR #276 and achieve the same results

### Summary:

✓ registry ➭ getMetricsAsJSON#1 with 64 is 1.900% faster.
✓ registry ➭ getMetricsAsJSON#2 with 8 is 2.054% faster.
⚠ registry ➭ getMetricsAsJSON#2 with 4 and 2 with 2 is 7.658% acceptably slower.
✓ registry ➭ getMetricsAsJSON#2 with 2 and 2 with 4 is 8.235% faster.
✓ registry ➭ getMetricsAsJSON#6 with 2 is 4.261% faster.
✓ registry ➭ metrics#1 with 64 is 6.524% faster.
✓ registry ➭ metrics#2 with 8 is 35.91% faster.
✓ registry ➭ metrics#2 with 4 and 2 with 2 is 26.44% faster.
✓ registry ➭ metrics#2 with 2 and 2 with 4 is 31.02% faster.
✓ registry ➭ metrics#6 with 2 is 34.78% faster.
✓ histogram ➭ observe#1 with 64 is 10.78% faster.
✓ histogram ➭ observe#2 with 8 is 9.954% faster.
✓ histogram ➭ observe#2 with 4 and 2 with 2 is 18.51% faster.
✓ histogram ➭ observe#2 with 2 and 2 with 4 is 5.529% faster.
✓ histogram ➭ observe#6 with 2 is 4.469% faster.
⚠ summary ➭ observe#1 with 64 is 0.7088% acceptably slower.
✓ summary ➭ observe#2 with 8 is 0.8064% faster.
⚠ summary ➭ observe#2 with 4 and 2 with 2 is 0.8744% acceptably slower.
⚠ summary ➭ observe#2 with 2 and 2 with 4 is 3.878% acceptably slower.
✓ summary ➭ observe#6 with 2 is 2.453% faster.

[zbjornson]

Thanks for rebasing and for your patience! Much easier to review now.

My two main concerns are ⭐'ed.

[zbjornson]

⭐ This is going to make it impossible to implement #298/landing #368. As you can tell from the comments in those, I'm not certain what the fate of that issue is though.

[doochik]

Can you clarify what do you mean? Should I rewrite it or implement inside generated function?

[doochik]

@zbjornson can you take a look on this?

[zbjornson]

Thanks for the nudge, will review this PR this weekend.

On this particular comment: I'd (strongly) lean toward making the formatter work with dynamically declared labels. #298 has quite a bit of support so I suspect #368 will land. The main (probably minor) thing holding it up is #298 (comment).

[doochik]

This eliminate the main idea of formatter - build labels string once.
I need to think how to rewrite this.

[zbjornson]

⭐ It's valid to only set values for a subset of labels when a metric is updated. With this test:

it('should allow subsetse of labels', () => {
	instance = new Gauge({
		name: 'name_2',
		help: 'help',
		labelNames: ['code', 'success'],
	});
	instance.inc({ code: '200' }, 10);
	const str = globalRegistry.getSingleMetricAsString('name_2');
	console.log(str);
});

in this PR, you get

# HELP name_2 help
# TYPE name_2 gauge
name_2{code="200",success="undefined"} 10

whereas in master, you get

# HELP name_2 help
# TYPE name_2 gauge
name_2{code="200"} 10

[SimenB]

should add this as a test, no?

[doochik]

Yes, of course. I’ll fix it

[zbjornson]

The above test case passes, but now if the first label is undefined, there's a stray leading comma:

it.only('should allow combinations of labels', () => {
	instance = new Gauge({
		name: 'name_2',
		help: 'help',
		labelNames: ['code', 'success', 'ok'],
	});
	instance.inc({ code: '200' }, 10);
	instance.inc({ code: '200', ok: 'yes' }, 10);
	instance.inc({ ok: 'yes', success: 'false' }, 10);
	instance.inc({ success: 'false', ok: 'no' }, 10);
	const str = globalRegistry.getSingleMetricAsString('name_2');
	console.log(str);
});

prints

      # HELP name_2 help
      # TYPE name_2 gauge
      name_2{code="200"} 10
      name_2{code="200",ok="yes"} 10
      name_2{,success="false",ok="yes"} 10
      name_2{,success="false",ok="no"} 10

(notice last two lines)

Something like these should both be committed as test cases. Do you feel like adding them please?

---

Just checked, benchmarks still look significantly faster despite the new branch added in the last commits. 👍

[SimenB]

Any news here? 🙂

[SimenB]

Does this still provide improvements after #594?