Discussion: Macros standardization

grafana / sqlds

A package that assists writing SQL-driven datasources

Apache License 2.0

17 stars 12 forks source link

Discussion: Macros standardization #43

Closed andresmgot closed 2 years ago

andresmgot commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

I guess

kminehart commented 3 years ago

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 commented 3 years ago

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!