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

Discussion: Macros standardization #43

Closed
andresmgot opened this issue Sep 13, 2021 · 4 comments · Fixed by #45
Closed

Discussion: Macros standardization #43

andresmgot opened this issue Sep 13, 2021 · 4 comments · Fixed by #45

Comments

@andresmgot
Copy link
Collaborator

Hi, I am working on adding macros to a new datasource and it may makes sense to do some refactoring. As far as I can see, all SQL datasources can/should implement the following macros:

  • $__timeFilter. To filter based on the query interval
  • $__timeGroup. To group metrics
  • $__timeFrom. To filter based on the query interval starting point
  • $__timeTo. To filter based on the query interval ending point

The implementation (even the number of args) may differ between different data sources though. Does it makes sense to define those macros here and let the different datasources implement it?

Also, it's necessary a library to convert string durations (e.g. 10ms) to a time.Duration. Main grafana has a package for this: https://github.com/grafana/grafana/tree/main/pkg/components/gtime. Should we copy that package here so we can have a standard way of parsing durations (for the $__timeGroup macro)? (or to the grafana-plugin-sdk-go?)

WDYT @sunker @kminehart @yesoreyeram?

@sunker
Copy link
Collaborator

sunker commented Sep 14, 2021

I though the implementation of these macro would be the same for most of the data sources. Since that's not the case, I think the main benefit och defining them here would be to provide a baseline for what macros to use and what to call them for future sql plugin developers. I've noticed that what you describe as $__timeFrom macro above is called different things in different data source plugins, which is not good for our users.

So I think defining the interface for these macros in sqlds would make sense. Regarding gtime, @marefr would it make sense to move this into grafana-plugin-sdk-go?

@marefr
Copy link
Member

marefr commented Sep 14, 2021

Regarding gtime, @marefr would it make sense to move this into grafana-plugin-sdk-go?

I guess

@kminehart
Copy link
Collaborator

I think this makes sense. Should they be required or optional to implement?

would it make sense to move this into grafana-plugin-sdk-go

I think it would.

@andresmgot
Copy link
Collaborator Author

I though the implementation of these macro would be the same for most of the data sources. Since that's not the case, I think the main benefit och defining them here would be to provide a baseline for what macros to use and what to call them for future sql plugin developers. I've noticed that what you describe as $__timeFrom macro above is called different things in different data source plugins, which is not good for our users.

Yes, I do agree. Having a standard set of macros, which mean the same for the SQL data sources, would be beneficial.

Regarding gtime, @marefr would it make sense to move this into grafana-plugin-sdk-go?

I guess

I will send a PR for that.

I think this makes sense. Should they be required or optional to implement?

Probably optional to avoid breaking changes? (even though I'd highly encourage it for new data sources).

Thanks for the feedback!

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 a pull request may close this issue.

4 participants