clusterfuzz icon indicating copy to clipboard operation
clusterfuzz copied to clipboard

Ensure correct ordering of 'time_series' list.

Open aditya-wazir opened this issue 1 year ago • 2 comments

Maintain separate 'gauge_series' for gauge metrics. Combined gauge series with the main 'time_series' list only before the flush operation ensuring that all metric are in order based on corresponding 'start_time' in 'time_series' list

aditya-wazir avatar Jun 25 '24 16:06 aditya-wazir

/gcbrun

svasudevprasad avatar Jun 25 '24 17:06 svasudevprasad

/gcbrun

svasudevprasad avatar Jun 26 '24 02:06 svasudevprasad

/gcbrun

svasudevprasad avatar Jul 02 '24 05:07 svasudevprasad

I went looking for the docs for creating time series. Here's what they say about the array of time series objects [1]:

Adds at most one data point to each of several time series. The new data point must be more recent than any other point in its time series. Each TimeSeries value must fully specify a unique time series by supplying all label values for the metric and the monitored resource.

As for time series objects [2]:

points[] [...] When creating a time series, this field must contain exactly one point

The first one suggests to me that we only need to ensure that points are sorted within each time series. Sorting the list globally should also work, but this still begs the question: why are gauge series the problem we're trying to solve?

[1] https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.timeSeries/create
[2] https://cloud.google.com/monitoring/api/ref_v3/rest/v3/TimeSeries

letitz avatar Jul 09 '24 08:07 letitz

I went looking for the docs for creating time series. Here's what they say about the array of time series objects [1]:

Adds at most one data point to each of several time series. The new data point must be more recent than any other point in its time series. Each TimeSeries value must fully specify a unique time series by supplying all label values for the metric and the monitored resource.

As for time series objects [2]:

points[] [...] When creating a time series, this field must contain exactly one point

The first one suggests to me that we only need to ensure that points are sorted within each time series. Sorting the list globally should also work, but this still begs the question: why are gauge series the problem we're trying to solve?

[1] https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.timeSeries/create [2] https://cloud.google.com/monitoring/api/ref_v3/rest/v3/TimeSeries

While it's true that points within a time series must be sorted, the requirement for creating time series objects is that the timestamps across all series must be in chronological order. The challenge with gauge metrics is that as gauge metric's start_time is assigned the current timestamp('start_time = end_time', where end_time = time.time()) within the _FlusherThread.run() function, they can introduce out-of-order timestamps when combined with other metric types, which conflicts with the chronological order requirement for creating time series objects. To ensure correct time series creation, we're implementing a sorting based solution. This approach ensures that the timestamps are in ascending order, meeting the API's requirements.

aditya-wazir avatar Jul 12 '24 12:07 aditya-wazir

/gcbrun

marktefftech avatar Jul 18 '24 15:07 marktefftech

/gcbrun

jonathanmetzman avatar Jul 30 '24 18:07 jonathanmetzman