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

Add helper for batching DataQueries by time range #1178

Closed
wants to merge 1 commit into from

Conversation

iwysiu
Copy link
Contributor

@iwysiu iwysiu commented Dec 18, 2024

What this PR does / why we need it:
This is a helper function to separate a batch of queries into smaller batches by time range. This function can be used in datasource plugins that need to batch their queries before sending them to the datasources.

Which issue(s) this PR fixes:

Part of grafana/grafana#85495

Special notes for your reviewer:
We could also get the string values of the time range fields to use as the key, but that didn't seem necessary to me.

@iwysiu iwysiu requested review from gabor and aangelisc December 18, 2024 19:52
@iwysiu iwysiu force-pushed the add-time-batch-helper branch from 413d938 to 75f920d Compare December 18, 2024 19:57
@iwysiu iwysiu marked this pull request as ready for review December 18, 2024 22:30
@iwysiu iwysiu requested a review from a team as a code owner December 18, 2024 22:31
@iwysiu iwysiu requested review from wbrowne, andresmgot and xnyo and removed request for a team December 18, 2024 22:31
require.Equal(t, 2, len(result))
var FiveMinQueries = result[0]
var TenMinQueries = result[1]
if len(result[0]) == 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you explain this if? There is nothing variable in this function so not sure why it's needed

@@ -110,6 +110,21 @@ type DataQuery struct {
JSON json.RawMessage
}

func BatchDataQueriesByTimeRange(queries []DataQuery) [][]DataQuery {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add documentation about this function?

Also, I don't think backend is the right package for this utility, I would first add it in experimental or maybe gtime.

@iwysiu
Copy link
Contributor Author

iwysiu commented Dec 19, 2024

Talked with @gabor , decided to add this directly to cloudwatch for now and we can move it to the plugin-sdk if we find that it's a good solution for other datasources with this problem

@iwysiu iwysiu closed this Dec 19, 2024
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.

2 participants