go-graphite / carbonapi

Implementation of graphite API (graphite-web) in golang
Other
309 stars 140 forks source link

Fix hitcount function handling of intervals and alignToInterval #759

Closed carrieedwards closed 1 year ago

carrieedwards commented 1 year ago

This PR is a rewrite of the hitcount function in order to match Graphite web's behavior. There are two major issues that were discovered:

1) When the interval parameter was set to a value smaller than the fetched data's step between data points, it results were incorrectly shifted left, such that all of the hit counts were concentrated towards the left buckets, and there were empty buckets on the right. For example:

hitcount(metric1, "6m") []*types.MetricData{{"metric1", 0, 1}: {types.MakeMetricData("metric1", []float64{2, 4, 6}, 600, 100)},} // 10m step

The result will be []float64{1200, 2400, 3600, 0, 0, 0}. The hit values are all concentrated on the left half of the buckets.

2) Upon looking at Graphite web's implementation of hitcount, when the alignToInterval parameter is set to true, the start time is adjusted and the data is re-fetched. In CarbonAPI, the start time was adjusted, but data was not re-fetched.

Major changes in this PR:

carrieedwards commented 1 year ago

Hi @Civil, when you get the chance could you take a look at this PR? It's a pretty significant refactor. We have tested it pretty thoroughly on our end, including against Graphite-web.