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

Dashboards: Data source template variable options now specify a current value using uid. #69259

Merged

Conversation

darrenjaneczek
Copy link
Contributor

@darrenjaneczek darrenjaneczek commented May 30, 2023

What is this feature?

The goal was to make it possible for a dashboard to specify a provisioned data source as the "current" option for a template variable. The only way to currently achieve this is to use the name of the datasource, but for our use case this would vary between grafana cloud instances.

The solution makes use of the value part of the {text, value} structure under a template variable's options.current. The value field was being completely ignored and was a duplicate of text: the name of data source. This PR now takes the value into consideration when resolving the initial template variable value, and for data source template variables, the value now corresponds to the uid. It will be reflected in new dashboard JSON accordingly. Old dashboard JSON which uses the data source name will still function as before.

Why do we need this feature?

When provisioning dashboards and datasources with specific uids, we might want a dashboard to initially select that specific datasource first, instead of defaulting to alphabetical order and choosing the first on the list. This should also make it possible to set a selected value for query-based template variables as well.

Who is this feature for?

Specifically, cloud customers accessing the Cardinality management dashboards with multiple metrics datasources defined. We want the dashboards to choose the provisioned grafanacloud-prom to be selected first by default when opening the dashboard, instead of the datasource with the earliest name in alphabetical sort. Since the grafanacloud-prom datasource has a inconstant name: grafanacloud-${instanceName}-prom, it cannot be specified by setting the current value in dashboard JSON. This PR allows specifying that uid through the variable's current.value field.

Special notes for your reviewer:

  • There didn't seem to be any place where the documentation needed to know about the specifics of this issue.
  • Previously, when a user changes their variable, the current fields of text, and value would both be set to the data source name. Now text gets the name, and value gets the uid. This will be reflected in the JSON. Old dashboards which use the name for both will still be compatible.

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

Release notice breaking change

The data source template variable type has changed the way it represents its options. The text field still represents the data source name, but the value has been changed to the uid of the data source. This allows dashboards to declare the currently selected option by uid, however it changes how a datasource template variable value will be rendered by default. If the name of the data source is expected, the variable syntax will have to be changed to specify the text format.

For example, given a data source variable (datasourceVariable), the following string:

${datasourceVariable}<br/>
Name: ${datasourceVariable:text}<br/>
UID: ${datasourceVariable:raw}

was previously interpolated as:

grafanacloud-k8smonitoring-prom
Name: grafanacloud-k8smonitoring-prom
UID: grafanacloud-k8smonitoring-prom

After these changes, it's interpolated as:

d7bbe725-9e48-4af8-a0cb-6cb255d873a3
Name: grafanacloud-k8smonitoring-prom
UID: d7bbe725-9e48-4af8-a0cb-6cb255d873a3

Any dashboards that are relying on the data source name being returned by ${datasourceVariable} will have to update all their usages to ${datasourceVariable:text} in order to get the previous behavior.

Affected use cases:

  • Using ${datasourceVariable} to display the data source name in text panel or in the panel title.
  • Using ${datasourceVariable} to use the data source name as part of the query content.

Unaffected use cases:

  • Using the ${datasourceVariable} to choose which data source to use for a query (through its data source picker) will not be affected since it can use both the name and the uid

@darrenjaneczek darrenjaneczek marked this pull request as ready for review May 30, 2023 15:45
@darrenjaneczek darrenjaneczek requested review from a team as code owners May 30, 2023 15:45
@darrenjaneczek darrenjaneczek requested review from dprokop, kaydelaney and leventebalogh and removed request for a team May 30, 2023 15:45
@darrenjaneczek darrenjaneczek added this to the 9.5.x milestone May 30, 2023
@darrenjaneczek darrenjaneczek requested review from a team as code owners May 30, 2023 17:49
@darrenjaneczek darrenjaneczek requested review from tskarhed, JoaoSilvaGrafana, mildwonkey, suntala and yangkb09 and removed request for a team May 30, 2023 17:49
@darrenjaneczek darrenjaneczek changed the title Template variables: allow data source option current value to be set by uid (using value field) Dashboards: allow data source template variables specify current option by datasource uid May 30, 2023
@darrenjaneczek darrenjaneczek changed the title Dashboards: allow data source template variables specify current option by datasource uid Dashboards: Data source template variable options now specify a current value using a uid May 30, 2023
@darrenjaneczek darrenjaneczek changed the title Dashboards: Data source template variable options now specify a current value using a uid [v9.5.x] Dashboards: Data source template variable options now specify a current value using a uid May 30, 2023
@darrenjaneczek darrenjaneczek changed the title [v9.5.x] Dashboards: Data source template variable options now specify a current value using a uid Dashboards: Data source template variable options now specify a current value using a UID May 30, 2023
@darrenjaneczek darrenjaneczek changed the title Dashboards: Data source template variable options now specify a current value using a UID Dashboards: Data source template variable options now specify a current value using uid. May 30, 2023
@dprokop
Copy link
Member

dprokop commented May 31, 2023

@darrenjaneczek I see the point of this change, and I do think this is a good direction. Tho, I'm afraid this is a breaking change from the POV of the variable interpolation. Given a data source variable (dsVar), the following string:

${dsVar}<br/>
Name: ${dsVar:text}<br/>
UID: ${dsVar:raw}

is interpolated as:

grafanacloud-k8smonitoring-prom
Name: grafanacloud-k8smonitoring-prom
UID: grafanacloud-k8smonitoring-prom

After your changes, it's interpolated as:

d7bbe725-9e48-4af8-a0cb-6cb255d873a3
Name: grafanacloud-k8smonitoring-prom
UID: d7bbe725-9e48-4af8-a0cb-6cb255d873a3

Think this should be ok, but requires a breaking change notice and an update for everyone who's relying in their dashboards on the data source name being returned by ${dsVar}. Now, they will have to update all their usages to ${dsVar:text} in order to get previous behavior.

I'm ok accepting this change but would like to hear thoughts from @axelavargas and @torkelo before proceeding with it.

@dprokop dprokop requested a review from torkelo May 31, 2023 08:43
@torkelo
Copy link
Member

torkelo commented May 31, 2023

@dprokop

I think this change makes sense as well, it could be a breaking change for some edge cases where data source variable is not used for it's normal use cases (as value in the datasource / datasource.uid prop).

So we need to add a breaking change release notes notice (add breaking change label to this PR, and a section in the PR description with title "Release notice breaking change" , see #66466 for example.

@dprokop
Copy link
Member

dprokop commented May 31, 2023

@darrenjaneczek one more comment. I see the milestone you set is 9.5.x. Given this is a breaking change I think we need to release it in 10.1.

@ivanortegaalba ivanortegaalba self-requested a review May 31, 2023 13:25
@darrenjaneczek darrenjaneczek modified the milestones: 9.5.x, 10.0.0 May 31, 2023
@darrenjaneczek
Copy link
Contributor Author

one more comment. I see the milestone you set is 9.5.x. Given this is a breaking change I think we need to release it in 10.1.

@dprokop
Is it too late for 10.0.0?

@darrenjaneczek
Copy link
Contributor Author

darrenjaneczek commented May 31, 2023

Now, they will have to update all their usages to ${dsVar:text} in order to get previous behavior.

Thanks @dprokop. I wasn't aware of that formation. I've added to the test to include this alternative variable form: 6513cb3 and corrected further in b74d51e.

@darrenjaneczek darrenjaneczek added the breaking change Relevant for changelog generation label May 31, 2023
@darrenjaneczek darrenjaneczek modified the milestones: 10.0.0, 10.0.x May 31, 2023
@ivanortegaalba
Copy link
Contributor

one more comment. I see the milestone you set is 9.5.x. Given this is a breaking change I think we need to release it in 10.1.

@dprokop Is it too late for 10.0.0?

Yes, it is, the hard freeze for G10 started last week 😓

I like this change, it goes in the right direction. I'd love to have second eyes on this one, but it looks good.

About breaking change notes, it creates the necessity for the customer to do something about this change, but it is not clear what use cases are affected by this change.
Could we please mention the use cases that are affected? The most common ones I'm aware of are:

  • Use ${dsVariable} as part of a markdown panel
  • Use ${dsVariable} as part of the query content. Use the variable selecting it in the DS picker is not affected.

@darrenjaneczek darrenjaneczek modified the milestones: 10.0.x, 10.1.x May 31, 2023
@darrenjaneczek
Copy link
Contributor Author

Sad that I missed the G10 boat 🚢.

  • Updated milestone to 10.1.x
  • Updated "breaking changes" content.

@dprokop dprokop merged commit d61af3a into main Jun 12, 2023
19 checks passed
@dprokop dprokop deleted the darrenjaneczek/initial-datasource-variable-select-by-uid branch June 12, 2023 08:05
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
deresolution20 added a commit that referenced this pull request Sep 26, 2023
Please see this PR for the breaking change to raw.  #69259
Eve832 pushed a commit that referenced this pull request Oct 10, 2023
* Update index.md

Please see this PR for the breaking change to raw.  #69259

* Update docs/sources/dashboards/variables/variable-syntax/index.md

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

* Fixed linting error

---------

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants