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

Dashboard variables #44

Merged
merged 2 commits into from
Jun 8, 2022
Merged

Conversation

TarasPriadka
Copy link
Contributor

@TarasPriadka TarasPriadka commented Jun 6, 2022

Add support for dashboard variables. Depends on #42

For example here,

df = px.DataFrame(table='http_events', start_time=__time_from)
df['cluster'] = '$clusters'

The $clusters will get replaced with the dashboard variable value.

@nserrino
Copy link

nserrino commented Jun 6, 2022

Is there a reason we use a different syntax here ($ vs __ prefix) than the time/interval special variables? I would recommend using the same approach for both

@nserrino
Copy link

nserrino commented Jun 6, 2022

Also, looks like the commit history might be including some other stuff

@TarasPriadka
Copy link
Contributor Author

Is there a reason we use a different syntax here ($ vs __ prefix) than the time/interval special variables? I would recommend using the same approach for both

This is actually replacing the variables directly in Grafana UI. Previously we handled the __time and other variables ad hoc, but grafana can do it natively from the UI. The benefit of doing that is that you can use any dashboard variable in your scripts, even user defined variables. So we can handle the common case like __start_time, __end_time, but also any other variable such as cluster or pod in the future.

The Grafana UI actually just replaces the $ variables before sending to backend, so we can remove that logic there completely.

Also, looks like the commit history might be including some other stuff

I will rebase on main in a moment.

@nserrino
Copy link

nserrino commented Jun 6, 2022

So you're saying that the $ replacement is done by Grafana, not manual logic on our side.

In that case, we might want to support $start_time, $interval, and $end_time in addition to __start_time, __interval, and __end_time, so that we can move toward a consistent experience with $ only. I think that would be better than the user having to know both ways, going forward, but we can still support the old way for backwards compatibility

We can therefore update our Grafana plugin to support both and update our example scripts to use $

Signed-off-by: Taras Priadka <tpriadka@pixielabs.ai>
@TarasPriadka TarasPriadka force-pushed the dashboard-variables branch from 06d4e1c to cea6c44 Compare June 6, 2022 20:52
@TarasPriadka
Copy link
Contributor Author

So you're saying that the $ replacement is done by Grafana, not manual logic on our side.

In that case, we might want to support $start_time, $interval, and $end_time in addition to __start_time, __interval, and __end_time, so that we can move toward a consistent experience with $ only. I think that would be better than the user having to know both ways, going forward, but we can still support the old way for backwards compatibility

We can therefore update our Grafana plugin to support both and update our example scripts to use $

The equivalent with Grafana global variables are $__interval, $__from and $__to. I think these should be familiar to users because Grafana mentions these variables directly in their documentation.

Signed-off-by: Taras Priadka <tpriadka@pixielabs.ai>
@aimichelle aimichelle merged commit 95c7451 into pixie-io:main Jun 8, 2022
@TarasPriadka TarasPriadka deleted the dashboard-variables branch June 10, 2022 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants