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

Forward grafana meta values to downstream API #1072

Merged
merged 8 commits into from
Jan 30, 2025

Conversation

yesoreyeram
Copy link
Collaborator

@yesoreyeram yesoreyeram commented Nov 28, 2024

This PR will allow datasource authors to pass grafana meta data such as user id, datasource uid to the underlying API as query parameters / headers.

Any value set by the user in query will be overidden by the values from the data source settings.

Fixes #1073
Fixes #867
Fixes #457

Design decisions & FAQs

  • Why don't this auto forward the headers? Ans: Not every API like the additional headers
  • Whey don't we respect grafana setting send_user_header? Answer: That setting is applicable for all the datasource instances. But in infinity, as said before not all the APIs will like additional headers
  • Why not just forward as headers with default keys? Ans: Some APIs need to forward these values as part of their query string and some APIs need these information in header with different key name. So this PR is flexible in such a way that data source admin can configure the way how they want to pass this information to the downstream api.
  • Why don't this PR interpolate these variables in the URL path? Ans: As the URL / path is set in the panel's query editor any user with editor role may leverage this. But we want this to be availble only at the backend for admins to handle these information safely.

How to test this

image

Before changes

  • git checkout main && git pull && yarn && yarn build && mage -v

  • docker compose up

  • Open http://localhost:3000

  • add a new infinity datasource instance

  • in datasource custom http header add a new key X-Grafana-User with value ${__user.login}

  • in datasource URL query parameter add a new key X-Grafana-Datasource-UID with value ${__ds.uid}

  • Add https://httpbin.org to the allowed hosts

  • Save and test the datasource

  • Create a query in explore with URL and backend parser option

  • Add https://httpbin.org/get as URL and run the query. Inspect headers and args in the result

image image

After changes

  • git checkout grafana-headers-forwarding && git pull && yarn && yarn build && mage -v

  • docker compose up

  • Open http://localhost:3000

  • add a new infinity datasource instance

  • in datasource custom http header add a new key X-Grafana-User with value ${__user.login}

  • in datasource URL query parameter add a new key X-Grafana-Datasource-UID with value ${__ds.uid}

  • Add https://httpbin.org to the allowed hosts

  • Save and test the datasource

  • Create a query in explore with URL and backend parser option

  • Add https://httpbin.org/get as URL and run the query. Inspect headers and args in the result

image image

also in query inspector

image

@yesoreyeram yesoreyeram force-pushed the grafana-headers-forwarding branch from 58a8f7a to fb1799b Compare November 28, 2024 13:41
@yesoreyeram yesoreyeram changed the title Support passing X-Grafana-User and X-Grafana-Datasource-UID as header… Forward grafana meta values as headers to downstream API Nov 28, 2024
@yesoreyeram yesoreyeram changed the title Forward grafana meta values as headers to downstream API Forward grafana meta values as headers/query parameters to downstream API Nov 28, 2024
@yesoreyeram yesoreyeram marked this pull request as ready for review November 28, 2024 16:10
@yesoreyeram yesoreyeram requested a review from a team as a code owner November 28, 2024 16:10
@yesoreyeram yesoreyeram changed the base branch from main to dev-3.0.0 December 1, 2024 23:22
@yesoreyeram yesoreyeram changed the title Forward grafana meta values as headers/query parameters to downstream API Forward grafana meta values as headers/query params to downstream API Dec 1, 2024
@yesoreyeram yesoreyeram changed the title Forward grafana meta values as headers/query params to downstream API Forward grafana meta values to downstream API Dec 1, 2024
@yesoreyeram yesoreyeram marked this pull request as draft December 1, 2024 23:28
@yesoreyeram yesoreyeram added this to the Version 3.0 milestone Dec 9, 2024
@yesoreyeram yesoreyeram self-assigned this Dec 9, 2024
Comment on lines +134 to +143
if pCtx.DataSourceInstanceSettings != nil {
value = strings.ReplaceAll(value, "${__ds.uid}", pCtx.DataSourceInstanceSettings.UID)
value = strings.ReplaceAll(value, "${__ds.name}", pCtx.DataSourceInstanceSettings.Name)
value = strings.ReplaceAll(value, "${__ds.id}", fmt.Sprintf("%d", pCtx.DataSourceInstanceSettings.ID))
}
if pCtx.User != nil {
value = strings.ReplaceAll(value, "${__user.login}", pCtx.User.Login)
value = strings.ReplaceAll(value, "${__user.email}", pCtx.User.Email)
value = strings.ReplaceAll(value, "${__user.name}", pCtx.User.Name)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we throw error in such nil cases to assist more to the users? or it would cause more inconvenience? ( If this used in the context of cron/anonymous jobs such as alert, public dashboard... then potentially the queries will silently fail there without much context but may succeed in the dashboards / explore page )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think throwing an error make sense. I was thinking about logging an error here but it may be better to throw and surface the error to the end user. Thinking out loud that would mean that the query would fail and not run because of this. right? So a user would have to create a separate ds with headers and without headers which is not ideal. How should we let the user know that these headers won't work in alert?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea. it is bit tricky. I prefer to go with current approach now and then later iterations, we can figure out if we can send warnings to the frontend in such cases.

@yesoreyeram
Copy link
Collaborator Author

yesoreyeram commented Jan 15, 2025

#1106 (reply in thread)

@mahendrapaipuri - if u have more thoughts around this, suggest to respond here in the thread so that it will be useful for reviewers

@yesoreyeram yesoreyeram marked this pull request as ready for review January 16, 2025 01:36
Copy link
Member

@zoltanbedi zoltanbedi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried it out and works as expected.

@yesoreyeram yesoreyeram merged commit ac34ec0 into dev-3.0.0 Jan 30, 2025
2 checks passed
@yesoreyeram yesoreyeram deleted the grafana-headers-forwarding branch January 30, 2025 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use global variable in Authentication
3 participants