go-graphite / carbonapi

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

Fix multiple issues with moving* functions #782

Closed carrieedwards closed 1 year ago

carrieedwards commented 1 year ago

The moving* functions have several bugs that are causing different results than what the same query in Graphite-web would produce. These include:

  1. Moving* functions are intended to support two different types for the windowSize parameter: either a string containing an interval such as “1min” or “1h”, or an integer which represents a fixed number of points to be considered for the window. When a n integer windowSize is specified in the query, the windowSize is not taken into consideration, causing extra NaN values to be included in the results.
  2. The start time of the fetch request is in the parser.Metrics() function, but only if a string interval was supplied for the windowSize parameter; it ignores the case of an integer interval being specified, which corresponds to a specific number of points. In the case of a specific number of points being used for the window, the step time of the data has to be factored in, and this is not possible to know before the data is fetched. Therefore, the moving* functions must re-fetch data to accommodate this
  3. Graphite-web ignores the first point of the re-fetched data. This issue in Graphite-web addresses why. This is not handled in the CarbonAPI implementation.
  4. MovingMedian is mapped to a consolidation function, so a query including the movingMedian function fails
  5. If a window contains only NaN values, the window's sum can become negative, and when Mean() is called on the window, the negative sum being divided by w.Len() (with a value of 0, since this function returns the result of subtracting the number of NaN values from the length of the window's data) results in -Inf being returned. Graphite-web checks if there are any non-null values, and if there are none, a value of None is returned for that window (see here for implementation).

See documentation for the moving* functions and the implementation in Graphite-web.