Loki: Display error with label filter conflicts

[gwdawson]

What is this feature?

• Part 1: display a visual error/warning if two or more labels are conflicting.
• Part 2: display a visual error/warning if two or more label filter operations are conflicting

Why do we need this feature?
...

Who is this feature for?
...

Which issue(s) does this PR fix?:
Fixes #59548

Special notes for your reviewer
test queries should be constructed in the builder mode.

Testing these conflict errors
construct the following query in the builder mode.
{job="tns/app", job!="tns/app"} | level = 'error' | level != 'error'

[github-actions]

Backend code coverage report for PR #63109
No changes

[github-actions]

Frontend code coverage report for PR #63109

Plugin	Main	PR	Difference
loki	83.49%	83.51%	.02%

[ivanahuckova]

@gwdawson Could you also do {job="tns/app", job!="tns/app"}?

[ivanahuckova]

Exciting improvement! 🎉 🎈

We already have a UI pattern for showings errors related to label selectors. Could we use this pattern also in this case and just show which label is conflicting and highlight only that one with error message.

Also left couple of other feedback.

[matyax]

Looks great! This is a partial review, will share some more feedback shortly.

[matyax]

There are some suggestions for your consideration. If you don't plan to implement them, I'd appreciate if you can share the reason with me.

[matyax]

An alternative approach to consider.

Suggested change

	export function isConflictingFilter(
	operation: QueryBuilderOperation,
	queryOperations: QueryBuilderOperation[]
	): boolean {
	let conflictingFilter = false;
	const labelFilters = queryOperations.filter((op) => {
	return op.id === LokiOperationId.LabelFilter && op.params !== operation.params;
	});
	// check if the new filter conflicts with any other label filter
	labelFilters.forEach((filter) => {
	if (operation.params[0] !== filter.params[0] || operation.params[2] !== filter.params[2]) {
	return;
	}
	if (
	(String(operation.params[1]).startsWith('!') && !String(filter.params[1]).startsWith('!')) ||
	(String(filter.params[1]).startsWith('!') && !String(operation.params[1]).startsWith('!'))
	) {
	conflictingFilter = true;
	}
	});
	return conflictingFilter;
	}
	export function isConflictingFilter(
	operation: QueryBuilderOperation,
	queryOperations: QueryBuilderOperation[]
	): boolean {
	const operationIsNegative = operation.params[1].toString().startsWith('!');
	// Discard non label filters and different params
	const candidates = queryOperations.filter(queryOperation => (queryOperation.id === LokiOperationId.LabelFilter && queryOperation.params[0] === operation.params[0] && queryOperation.params[2] === operation.params[2]));
	const conflict = candidates.find((candidate) => {
	if (operationIsNegative && candidate.params[1].toString().startsWith('!') === false) {
	return true;
	}
	if (operationIsNegative === false && candidate.params[1].toString().startsWith('!')) {
	return true;
	}
	return false;
	});
	return conflict !== undefined;
	}

A limitation of the current implementation is that it will continue iterating over all the label filters, even if one has already been found in line 180. I would suggest to change the forEach to a find or findIndex.

[svennergr]

I guess some would also be possible.

[matyax]

Definitely. And it looks like it's potentially faster https://www.measurethat.net/Benchmarks/Show/4337/0/array-find-vs-some

[gwdawson]

@matyax cool website, based off the fact some is faster, ill update to use that.

[matyax]

It took longer than I would like to admit to understand why this wasn't being validated.

[matyax]

👏 👏 great work! Really appreciate that you were on board with the latest iterations.