DataLinks: Fix bug where links which used built in variables could be hidden

[aocenas]

Fixes: #70809
Relates to #64328

At some point we added check to see if variables get interpolated in the link, mainly to not show link which would end up being broken because of missing values, this check though didn't consider built in variables.

This adds a static list of variables which we just ignore.

This is not 100% perfect, variables can be added later (for example in query time) and this needs to be synchronized if some variable gets added but should be good enough for now.

You can test it for example with Tempo service map, which when clicking on node should show multiple metrics links to Prometheus but all of them use $__rate_interval variable.

[github-actions]

Backend code coverage report for PR #71372
No changes

[github-actions]

Frontend code coverage report for PR #71372

Plugin	Main	PR	Difference
explore	86.72%	86.72%	0%

[gelicia]

You can test it for example with Tempo service map

Are there instructions on how to get this set up with devenv tempo?

[aocenas]

@gelicia
You can reproduce by:

• make devenv sources=tempo
• Setup prometheus data source based on the one in the tempo devenv block
• Setup tempo data source based on the one in the tempo devenv block and use previous prometheus DS for service map config
• Query service map with tempo in explore
• Click on a node

But also I assume you can just add $__rate_interval to some correlation that targets Prometheus and that should have similar effect.

[gelicia]

Ah okay, so after taking a look at this and chatting with Andrej, the issue is interpolation can happen in two places (thank you for breaking it down for me!)

1. when creating the link, there we interpolate variables with data from the source
2. interpolation when query is executed, for example, prometheus does this here

When building correlations, we added in logic under the assumption we would never want to show a query with an uninterpolated variable in it, so we hide all queries that still have variable values in them after that first interpolation. However, queries can pass through with an uninterpolated variable, and then be interpolated later by the datasource code. Those datasources have a set list of expected variables they'd interpolate, so this PR adds them so the correlations code doesn't stop them from showing.

I'm super torn about this, I agreed having the uninterpolated variables displaying didn't look right, but having to maintain this list and having exceptions is also not great. If we do go this route we should have the list of exceptions in the correlations documentation.

[ifrost]

Good spot!

As it's a regression, it should be okay for a short-term quick fix but it seems we need a better solution. $__rate_interval is not a built-in variable, I think it's Prometheus-specific, it won't scale work long term. A few options we could consider to fix it:

• Add conditions to data links config (correlations would set requiresAllVariables but I can imagine this could be expanded in the future with more conditions). We kind of tried that in the pass but it was hard to implement just for Explore. Data sources would not set such conditions (but can do it optionally)
• Add escaping mechanism to templating so a variable can actually be included inside the query (something like Variables: Allow escaping $ #71424 ?)
• Use DataSourceAPI.interpolateVariablesInQueries to interpolate the link (though I think Prometheus is not actually using it there)

Two more things about this PR:

• It's a regression, so we should add a test for it
• Do we need to backport it?

[aocenas]

Only new test, others were just refactored to remove duplicated lines

[aocenas]

@ifrost

• added test
• not sure about the backports, considering this is broken since 9.5 and nobody noticed I would say this isn't that critical atm

In general, I agree this should be a temporary fix but a proper one seems like it will be more complex so just wanted to get this out quickly while still in somewhat reasonable shape.

[gelicia]

Opened a ticket to add correlation documentation around this exception (#71511), and a ticket to develop a more robust solution for this scenario (#71510)