Cloud Monitoring: Reduce request size when listing labels

[mtanda]

What this PR does / why we need it:
Cloud Monitoring amplify response size from v8.3.
It cause memory shortage and crash in some environment.

By splitting meta data and relocate it to each frames, the response size could be reduced.
I tested this change locally, and confirm that response size is significantly reduced. (63.06MB -> 3.60MB)

Which issue(s) this PR fixes:
Fixes #44190

Special notes for your reviewer:

[CLAassistant]

All committers have signed the CLA.

[andresmgot]

Let me know if I understood this properly:

Before, all labels were added to all frames, which caused big response sizes. Now only the labels related to a frame are added and in the fronted we evaluate all the frames (rather than the first) to look for all the different labels.

The approach looks good to me, we should add test to ensure that we are enforcing this behavior. Letting @sunker to also review this in case there is something I am missing.

[andresmgot]

you would need to avoid pushing labels that are already there:

[mtanda]

That's good point. I didn't check multiple Group bys.
I fixed it in 1b4ed42 .

[sunker]

Thanks for opening this PR @mtanda! Also think the approach looks good. Just fix the comments that we've added. Would also be great with a test that verifies the merging of labels in the datasource.ts file.

[sunker]

No need to import map from lodash - just use dataQueryResponse?.data.map((f) => f.meta.custom.labels)

[mtanda]

Thanks for advice, I tend to use lodash...
I fixed it in 5cf1cdf .

[sunker]

You just want to push the key, or am I missing something?
acc[key].push(labels[key])

[mtanda]

It is wrong code.
And I notice that response format is not appropriate.

The value type of label map should be string, not string list.
I fixed the response format in fc7bb7c .

And, fixed frontend in 1b4ed42 .

[mtanda]

Let me know if I understood this properly:

Before, all labels were added to all frames, which caused big response sizes. Now only the labels related to a frame are added and in the fronted we evaluate all the frames (rather than the first) to look for all the different labels.

Yes, I do that. I think it is only way to reduce response size with plugin sdk.

[andresmgot]

LGTM, just one minor comment to simplify the frontend code.

[andresmgot]

A slightly simpler solution:

Suggested change

	const labels = dataQueryResponse?.data
	.map((f) => f.meta?.custom?.labels)
	.filter((p) => !!p)
	.reduce((acc, labels) => {
	for (let key in labels) {
	if (!acc[key]) {
	acc[key] = new Set<string>();
	}
	acc[key].add(labels[key]);
	}
	return acc;
	}, {});
	return Object.fromEntries(
	Object.entries(labels).map((l: any) => {
	l[1] = Array.from(l[1]);
	return l;
	})
	);
	const labels = dataQueryResponse?.data
	.map((f) => f.meta?.custom?.labels)
	.filter((p) => !!p)
	.reduce((acc, labels) => {
	for (let key in labels) {
	if (!acc[key]) {
	acc[key] = [];
	}
	if (labels[key] && !acc[key].includes(labels[key])) {
	acc[key].push(labels[key]);
	}
	}
	return acc;
	}, {});
	return labels;

[andresmgot]

(I also noticed that some labels were empty, that code removes those)

[mtanda]

Thanks for advice!
Yes, using includes() is simpler.
But, I don’t want to use O(n) function in loop.
In some env, I guess the number of labels is too big. (I may worry too much)
Using Set is intended.

[mtanda]

In our env, Cloud Monitoring record more than 1000 pod_name labels.

I think Set is faster than includes() if list is long.
https://www.measurethat.net/Benchmarks/Show/16868/1/includes-vs-has-20220126

[andresmgot]

I am not sure if the performance benefit goes away with the later loop but anyway it's fine

[andresmgot]

Thanks for the patch!

[mtanda]

Thanks for merge!