-
-
Notifications
You must be signed in to change notification settings - Fork 382
[Feature] Add Support for Historical Data Points #986
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,11 @@ func runDiscoveryJob( | |
| getMetricDataOutput := make([][]*cloudwatch.MetricDataResult, partition) | ||
| count := 0 | ||
|
|
||
| var addHistoricalMetrics bool | ||
| if job.AddHistoricalMetrics != nil { | ||
| addHistoricalMetrics = *job.AddHistoricalMetrics | ||
| } | ||
|
|
||
| for i := 0; i < metricDataLength; i += maxMetricCount { | ||
| go func(i, n int) { | ||
| defer wg.Done() | ||
|
|
@@ -76,7 +81,7 @@ func runDiscoveryJob( | |
| end = metricDataLength | ||
| } | ||
| input := getMetricDatas[i:end] | ||
| data := clientCloudwatch.GetMetricData(ctx, logger, input, svc.Namespace, length, job.Delay, job.RoundingPeriod) | ||
| data := clientCloudwatch.GetMetricData(ctx, logger, input, svc.Namespace, length, job.Delay, job.RoundingPeriod, addHistoricalMetrics) | ||
| if data != nil { | ||
| getMetricDataOutput[n] = data | ||
| } else { | ||
|
|
@@ -98,15 +103,34 @@ func runDiscoveryJob( | |
| if data == nil { | ||
| continue | ||
| } | ||
| previousIdx := -1 | ||
| previousID := "" | ||
| for _, metricDataResult := range data { | ||
| idx := findGetMetricDataByID(getMetricDatas, *metricDataResult.ID) | ||
| // TODO: This logic needs to be guarded by a feature flag! Also, remember to add compatibility in the client v2 | ||
| if idx == -1 { | ||
| logger.Warn("GetMetricData returned unknown metric ID", "metric_id", *metricDataResult.ID) | ||
| if addHistoricalMetrics { | ||
| // Use the previousIdx to make a copy | ||
| if previousIdx != -1 && previousID == *metricDataResult.ID { | ||
| // Create a new CloudwatchData object | ||
| newData := *getMetricDatas[previousIdx] | ||
| newData.GetMetricDataPoint = metricDataResult.Datapoint | ||
| newData.GetMetricDataTimestamps = metricDataResult.Timestamp | ||
|
|
||
| getMetricDatas = append(getMetricDatas, &newData) | ||
| } else { | ||
| logger.Warn("GetMetricData returned unknown metric ID", "metric_id", *metricDataResult.ID) | ||
| } | ||
| } else { | ||
| logger.Warn("GetMetricData returned unknown metric ID", "metric_id", *metricDataResult.ID) | ||
| } | ||
| continue | ||
| } | ||
| getMetricDatas[idx].GetMetricDataPoint = metricDataResult.Datapoint | ||
| getMetricDatas[idx].GetMetricDataTimestamps = metricDataResult.Timestamp | ||
| getMetricDatas[idx].MetricID = nil // mark as processed | ||
| previousIdx = idx | ||
| previousID = *metricDataResult.ID | ||
|
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -233,7 +233,9 @@ func EnsureLabelConsistencyAndRemoveDuplicates(metrics []*PrometheusMetric, obse | |
| } | ||
| } | ||
|
|
||
| metricKey := fmt.Sprintf("%s-%d", *metric.Name, prom_model.LabelsToSignature(metric.Labels)) | ||
| // Include the timestamp to avoid genuine duplicates!? At this point we have all the metrics to be exposed under the `/metrics` endpoint so | ||
| // we aren't able to tell if some of the metrics are present because the `addHistoricalMetrics` is set to `true`!? | ||
| metricKey := fmt.Sprintf("%s-%d-%d", *metric.Name, prom_model.LabelsToSignature(metric.Labels), metric.Timestamp.Unix()) | ||
|
Comment on lines
+253
to
+255
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it safe to include the timestamp to remove "real" duplicates? |
||
| if _, exists := metricKeys[metricKey]; !exists { | ||
| metricKeys[metricKey] = struct{}{} | ||
| output = append(output, metric) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new flag to allow this behaviour is only supported at the job level. I'll do some testing first but it might be a good idea to support it even at the metric level.