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

Fix hitcount function handling of intervals and alignToInterval #132

Merged
merged 2 commits into from
Apr 14, 2023

Conversation

carrieedwards
Copy link
Collaborator

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.

  1. 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:

  • The hitcount function was completely rewritten to match the behavior of Graphite web. This helps address the issue with intervals smaller than the data's step, as well as fixes discrepancies in results that were occurring with CarbonAPI tests that were tested in Graphite web.
  • The Metrics() function in pkg/parser.go was updated to pass in the from/to of the request, and code was added to determine the correct start time for the data being fetched for the hitcount function when alignToInterval is set to true. The new start time is determined based on the request's from value and the interval specified. This is intended to prevent needing to re-fetch the data as is done in Graphite web.

@ywwg
Copy link

ywwg commented Apr 13, 2023

this is the same as go-graphite#759

@carrieedwards carrieedwards force-pushed the cedwards/fix-hitcount branch 2 times, most recently from e68ecb2 to 89b4940 Compare April 13, 2023 19:56
@grafanabot
Copy link

Go coverage report:

Click to expand.
File %
github.com/go-graphite/carbonapi/cmd/carbonapi/http/capability.go 0.0%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/enrichcontext.go 6.7%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/expand_handler.go 28.1%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/find_handlers.go 33.9%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/functions_handler.go 0.0%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/helper.go 26.4%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/info_handlers.go 53.8%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/init.go 85.7%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/lbcheck_handler.go 0.0%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/metrics.go 55.8%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/render_handler.go 45.2%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/statsd.go 9.1%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/tags_handler.go 0.0%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/usage_handler.go 0.0%
github.com/go-graphite/carbonapi/cmd/carbonapi/http/version_handler.go 0.0%
github.com/go-graphite/carbonapi/date/date.go 78.9%
github.com/go-graphite/carbonapi/expr/consolidations/consolidations.go 45.6%
github.com/go-graphite/carbonapi/expr/expr.go 19.8%
github.com/go-graphite/carbonapi/expr/functions/absolute/function.go 80.0%
github.com/go-graphite/carbonapi/expr/functions/aggregate/function.go 82.2%
github.com/go-graphite/carbonapi/expr/functions/aggregateLine/function.go 81.0%
github.com/go-graphite/carbonapi/expr/functions/aggregateSeriesLists/function.go 72.2%
github.com/go-graphite/carbonapi/expr/functions/aggregateWithWildcards/function.go 86.3%
github.com/go-graphite/carbonapi/expr/functions/alias/function.go 80.8%
github.com/go-graphite/carbonapi/expr/functions/aliasByBase64/function.go 88.6%
github.com/go-graphite/carbonapi/expr/functions/aliasByMetric/function.go 92.9%
github.com/go-graphite/carbonapi/expr/functions/aliasByNode/function.go 81.0%
github.com/go-graphite/carbonapi/expr/functions/aliasByRedis/function.go 72.1%
github.com/go-graphite/carbonapi/expr/functions/aliasSub/function.go 75.8%
github.com/go-graphite/carbonapi/expr/functions/asPercent/function.go 77.5%
github.com/go-graphite/carbonapi/expr/functions/averageOutsidePercentile/function.go 86.7%
github.com/go-graphite/carbonapi/expr/functions/below/function.go 89.5%
github.com/go-graphite/carbonapi/expr/functions/cactiStyle/function.go 88.7%
github.com/go-graphite/carbonapi/expr/functions/changed/function.go 92.3%
github.com/go-graphite/carbonapi/expr/functions/compressPeriodicGaps/function.go 96.9%
github.com/go-graphite/carbonapi/expr/functions/consolidateBy/function.go 83.3%
github.com/go-graphite/carbonapi/expr/functions/constantLine/function.go 88.2%
github.com/go-graphite/carbonapi/expr/functions/cumulative/function.go 89.5%
github.com/go-graphite/carbonapi/expr/functions/delay/function.go 89.2%
github.com/go-graphite/carbonapi/expr/functions/derivative/function.go 93.8%
github.com/go-graphite/carbonapi/expr/functions/divideSeries/function.go 88.9%
github.com/go-graphite/carbonapi/expr/functions/ewma/function.go 87.5%
github.com/go-graphite/carbonapi/expr/functions/exclude/function.go 79.2%
github.com/go-graphite/carbonapi/expr/functions/exp/function.go 90.9%
github.com/go-graphite/carbonapi/expr/functions/exponentialMovingAverage/function.go 81.8%
github.com/go-graphite/carbonapi/expr/functions/fallbackSeries/function.go 88.2%
github.com/go-graphite/carbonapi/expr/functions/filter/function.go 83.0%
github.com/go-graphite/carbonapi/expr/functions/grep/function.go 79.2%
github.com/go-graphite/carbonapi/expr/functions/groupByNode/function.go 86.2%
github.com/go-graphite/carbonapi/expr/functions/groupByTags/function.go 88.9%
github.com/go-graphite/carbonapi/expr/functions/heatMap/function.go 86.4%
github.com/go-graphite/carbonapi/expr/functions/heatMap/helpers.go 84.6%
github.com/go-graphite/carbonapi/expr/functions/highestLowest/function.go 87.1%
github.com/go-graphite/carbonapi/expr/functions/hitcount/function.go 92.3%
github.com/go-graphite/carbonapi/expr/functions/holtWintersConfidenceBands/function.go 86.2%
github.com/go-graphite/carbonapi/expr/functions/identity/function.go 89.5%
github.com/go-graphite/carbonapi/expr/functions/integral/function.go 94.4%
github.com/go-graphite/carbonapi/expr/functions/integralByInterval/function.go 83.7%
github.com/go-graphite/carbonapi/expr/functions/integralWithReset/function.go 86.1%
github.com/go-graphite/carbonapi/expr/functions/interpolate/function.go 92.1%
github.com/go-graphite/carbonapi/expr/functions/invert/function.go 93.3%
github.com/go-graphite/carbonapi/expr/functions/isNotNull/function.go 93.8%
github.com/go-graphite/carbonapi/expr/functions/join/function.go 89.5%
github.com/go-graphite/carbonapi/expr/functions/keepLastValue/function.go 92.9%
github.com/go-graphite/carbonapi/expr/functions/legendValue/function.go 87.5%
github.com/go-graphite/carbonapi/expr/functions/limit/function.go 78.9%
github.com/go-graphite/carbonapi/expr/functions/linearRegression/function.go 80.0%
github.com/go-graphite/carbonapi/expr/functions/logarithm/function.go 90.0%
github.com/go-graphite/carbonapi/expr/functions/logit/function.go 91.3%
github.com/go-graphite/carbonapi/expr/functions/lowPass/function.go 89.3%
github.com/go-graphite/carbonapi/expr/functions/mapSeries/function.go 85.2%
github.com/go-graphite/carbonapi/expr/functions/minMax/function.go 86.7%
github.com/go-graphite/carbonapi/expr/functions/mostDeviant/function.go 86.5%
github.com/go-graphite/carbonapi/expr/functions/moving/function.go 83.0%
github.com/go-graphite/carbonapi/expr/functions/movingMedian/function.go 85.1%
github.com/go-graphite/carbonapi/expr/functions/nPercentile/function.go 87.5%
github.com/go-graphite/carbonapi/expr/functions/nonNegativeDerivative/function.go 90.8%
github.com/go-graphite/carbonapi/expr/functions/offset/function.go 85.2%
github.com/go-graphite/carbonapi/expr/functions/offsetToZero/function.go 94.1%
github.com/go-graphite/carbonapi/expr/functions/pearson/function.go 86.1%
github.com/go-graphite/carbonapi/expr/functions/pearsonClosest/function.go 75.0%
github.com/go-graphite/carbonapi/expr/functions/perSecond/function.go 90.8%
github.com/go-graphite/carbonapi/expr/functions/percentileOfSeries/function.go 76.0%
github.com/go-graphite/carbonapi/expr/functions/polyfit/function.go 87.3%
github.com/go-graphite/carbonapi/expr/functions/pow/function.go 86.2%
github.com/go-graphite/carbonapi/expr/functions/powSeries/function.go 94.9%
github.com/go-graphite/carbonapi/expr/functions/randomWalk/function.go 85.7%
github.com/go-graphite/carbonapi/expr/functions/rangeOfSeries/function.go 94.4%
github.com/go-graphite/carbonapi/expr/functions/removeBelowSeries/function.go 92.3%
github.com/go-graphite/carbonapi/expr/functions/removeBetweenPercentile/function.go 87.5%
github.com/go-graphite/carbonapi/expr/functions/removeEmptySeries/function.go 90.9%
github.com/go-graphite/carbonapi/expr/functions/round/function.go 89.7%
github.com/go-graphite/carbonapi/expr/functions/scale/function.go 86.1%
github.com/go-graphite/carbonapi/expr/functions/scaleToSeconds/function.go 85.7%
github.com/go-graphite/carbonapi/expr/functions/seriesList/function.go 66.1%
github.com/go-graphite/carbonapi/expr/functions/setXFilesFactor/function.go 82.6%
github.com/go-graphite/carbonapi/expr/functions/sigmoid/function.go 91.3%
github.com/go-graphite/carbonapi/expr/functions/sinFunction/function.go 86.2%
github.com/go-graphite/carbonapi/expr/functions/slo/function.go 84.0%
github.com/go-graphite/carbonapi/expr/functions/slo/helpers.go 72.0%
github.com/go-graphite/carbonapi/expr/functions/smartSummarize/function.go 89.0%
github.com/go-graphite/carbonapi/expr/functions/sortBy/function.go 86.5%
github.com/go-graphite/carbonapi/expr/functions/sortByName/function.go 85.2%
github.com/go-graphite/carbonapi/expr/functions/squareRoot/function.go 90.5%
github.com/go-graphite/carbonapi/expr/functions/stdev/function.go 85.7%
github.com/go-graphite/carbonapi/expr/functions/substr/function.go 85.4%
github.com/go-graphite/carbonapi/expr/functions/summarize/function.go 90.5%
github.com/go-graphite/carbonapi/expr/functions/timeFunction/function.go 83.3%
github.com/go-graphite/carbonapi/expr/functions/timeShift/function.go 76.9%
github.com/go-graphite/carbonapi/expr/functions/timeShiftByMetric/function.go 89.7%
github.com/go-graphite/carbonapi/expr/functions/timeShiftByMetric/misc.go 93.8%
github.com/go-graphite/carbonapi/expr/functions/timeSlice/function.go 86.5%
github.com/go-graphite/carbonapi/expr/functions/timeStack/function.go 86.1%
github.com/go-graphite/carbonapi/expr/functions/toLowerCase/function.go 88.9%
github.com/go-graphite/carbonapi/expr/functions/toUpperCase/function.go 88.9%
github.com/go-graphite/carbonapi/expr/functions/transformNull/function.go 88.1%
github.com/go-graphite/carbonapi/expr/functions/tukey/function.go 80.8%
github.com/go-graphite/carbonapi/expr/functions/unique/function.go 88.9%
github.com/go-graphite/carbonapi/expr/functions/weightedAverage/function.go 84.6%
github.com/go-graphite/carbonapi/expr/helper/align.go 57.8%
github.com/go-graphite/carbonapi/expr/helper/helper.go 9.2%
github.com/go-graphite/carbonapi/expr/helper/metric/extract.go 72.7%
github.com/go-graphite/carbonapi/expr/helper/sort.go 0.0%
github.com/go-graphite/carbonapi/expr/rewrite/aboveSeries/function.go 79.3%
github.com/go-graphite/carbonapi/expr/rewrite/applyByNode/function.go 77.1%
github.com/go-graphite/carbonapi/expr/sort.go 96.1%
github.com/go-graphite/carbonapi/expr/tags/helper.go 93.6%
github.com/go-graphite/carbonapi/expr/types/extract.go 87.5%
github.com/go-graphite/carbonapi/expr/types/list.go 0.0%
github.com/go-graphite/carbonapi/expr/types/metricheap.go 0.0%
github.com/go-graphite/carbonapi/expr/types/types.go 41.2%
github.com/go-graphite/carbonapi/expr/types/windowed.go 0.0%
github.com/go-graphite/carbonapi/pkg/parser/define.go 76.9%
github.com/go-graphite/carbonapi/pkg/parser/interface.go 0.0%
github.com/go-graphite/carbonapi/pkg/parser/internal.go 32.8%
github.com/go-graphite/carbonapi/pkg/parser/interval.go 85.4%
github.com/go-graphite/carbonapi/pkg/parser/parser.go 54.4%
github.com/go-graphite/carbonapi/util/pidfile/pidfile.go 100.0%
github.com/go-graphite/carbonapi/zipper/broadcast/broadcast_group.go 38.8%
github.com/go-graphite/carbonapi/zipper/helper/http_client.go 0.0%
github.com/go-graphite/carbonapi/zipper/helper/requests.go 26.3%
github.com/go-graphite/carbonapi/zipper/protocols/graphite/msgpack/type_gen.go 57.2%
github.com/go-graphite/carbonapi/zipper/protocols/irondb/irondb_group.go 2.5%
github.com/go-graphite/carbonapi/zipper/protocols/irondb/irondb_helpers.go 81.1%
github.com/go-graphite/carbonapi/zipper/protocols/prometheus/helpers/helpers.go 17.1%
github.com/go-graphite/carbonapi/zipper/protocols/victoriametrics/feature_set.go 19.7%
github.com/go-graphite/carbonapi/zipper/protocols/victoriametrics/fetch.go 0.0%
github.com/go-graphite/carbonapi/zipper/protocols/victoriametrics/find.go 0.0%
github.com/go-graphite/carbonapi/zipper/protocols/victoriametrics/tags.go 0.0%
github.com/go-graphite/carbonapi/zipper/protocols/victoriametrics/victoriametrics_group.go 7.8%
github.com/go-graphite/carbonapi/zipper/types/backend.go 0.0%
github.com/go-graphite/carbonapi/zipper/types/errors.go 0.0%
github.com/go-graphite/carbonapi/zipper/types/lbmethod.go 0.0%
github.com/go-graphite/carbonapi/zipper/types/requests.go 0.0%
github.com/go-graphite/carbonapi/zipper/types/response.go 6.4%
github.com/go-graphite/carbonapi/zipper/types/stats.go 0.0%
github.com/go-graphite/carbonapi/zipper/zipper.go 0.0%
github.com/go-graphite/carbonapi/zipper/zipper_pb3.go 0.0%
total 57.0%

Go lint report:

No issues found. 😎

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.

hitcount() produces incorrect results when the interval parameter is smaller than the fetched data's step
3 participants