feat: add track_timestamps_staleness limit to CRDs #6105

dongjiang1989 · 2023-11-17T16:12:32Z

Description

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
If it fixes a bug or resolves a feature request, be sure to link to that issue.

ref: prometheus/prometheus#13060

add track_timestamps_staleness limit to CRDs

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

CHANGE (fix or feature that would cause existing functionality to not work as expected)
FEATURE (non-breaking change which adds functionality)
BUGFIX (non-breaking change which fixes an issue)
ENHANCEMENT (non-breaking change which improves existing functionality)
NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Changelog entry

Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.

add track_timestamps_staleness limit to CRDs

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

nicolastakashi · 2023-11-17T21:45:19Z

pkg/apis/monitoring/v1/prometheus_types.go

+	// When true, Prometheus ignores the timestamps for all the targets created
+	// from service and pod monitors.
+	// Otherwise the TrackTimestampsStaleness field of the service or pod monitor applies.
+	OverrideTrackTimestampsStaleness bool `json:"overrideTrackTimestampsStaleness,omitempty"`


Quick question.
Do we see any use cases where Prometheus admin wants to override the decision made by the scrape config?

Thanks for your review.
When OverrideTrackTimestampsStaleness is set to "true" for Job target, it will be inserted in the TSDB when the target is down.

I'm not sure that it's something that the Prometheus admin cares about (there's an bit of resource overhead for tracking the staleness but it's probably negligible). In the first implementation, I'd focus only on the service/pod monitor part.

@nicolastakashi @simonpasquier Is it necessary to retain the OverrideTrackTimestampsStaleness global configuration?🤔

I would keep it simple and drop it for now.

From my understanding we can remove it for now and focus only on svc/pod monitor as @simonpasquier mentioned.

Thanks @simonpasquier @nicolastakashi , PTAL
Drop the OverrideTrackTimestampsStaleness global configuration.

nicolastakashi · 2023-11-17T21:45:44Z

Cool!
Thanks for handling that.

simonpasquier · 2023-11-20T15:59:08Z

pkg/apis/monitoring/v1/prometheus_types.go

+	// When true, Prometheus ignores the timestamps for all the targets created
+	// from service and pod monitors.
+	// Otherwise the TrackTimestampsStaleness field of the service or pod monitor applies.
+	OverrideTrackTimestampsStaleness bool `json:"overrideTrackTimestampsStaleness,omitempty"`


I'm not sure that it's something that the Prometheus admin cares about (there's an bit of resource overhead for tracking the staleness but it's probably negligible). In the first implementation, I'd focus only on the service/pod monitor part.

pkg/apis/monitoring/v1/podmonitor_types.go

pkg/apis/monitoring/v1/types.go

pkg/apis/monitoring/v1alpha1/scrapeconfig_types.go

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

jsonnet/prometheus-operator/prometheusagents-crd.json

jsonnet/prometheus-operator/prometheuses-crd.json

pkg/apis/monitoring/v1/podmonitor_types.go

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

pkg/apis/monitoring/v1/prometheus_types.go

pkg/apis/monitoring/v1/podmonitor_types.go

pkg/apis/monitoring/v1/types.go

pkg/apis/monitoring/v1/podmonitor_types.go

pkg/prometheus/promcfg.go

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

dongjiang1989 · 2023-11-22T03:15:03Z

@simonpasquier addressed comments, please review :)

jan--f

Just a few nits for consistency in the doc comments. Otherwise this looks good!

jan--f · 2023-11-22T10:30:25Z

jsonnet/prometheus-operator/servicemonitors-crd.json

@@ -626,6 +626,10 @@
                            }
                          },
                          "type": "object"
+                        },
+                        "trackTimestampsStaleness": {
+                          "description": "`trackTimestampsStaleness` defines whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. \n It requires Prometheus >= v2.48.0.",


Suggested change

"description": "`trackTimestampsStaleness` defines whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. \n It requires Prometheus >= v2.48.0.",

"description": "`trackTimestampsStaleness` defines whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. Has no effect if `honorTimestamps` is false. It requires Prometheus >= v2.48.0.",

jan--f · 2023-11-22T10:30:48Z

jsonnet/prometheus-operator/scrapeconfigs-crd.json

@@ -1550,6 +1550,10 @@
                      }
                    },
                    "type": "object"
+                  },
+                  "trackTimestampsStaleness": {
+                    "description": "TrackTimestampsStaleness whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. Has no effect if `honorTimestamps` is false. It requires Prometheus >= v2.48.0.",


Suggested change

"description": "TrackTimestampsStaleness whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. Has no effect if `honorTimestamps` is false. It requires Prometheus >= v2.48.0.",

"description": "`trackTimestampsStaleness` defines whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. Has no effect if `honorTimestamps` is false. It requires Prometheus >= v2.48.0.",

jan--f · 2023-11-22T10:31:17Z

jsonnet/prometheus-operator/podmonitors-crd.json

@@ -651,6 +651,10 @@
                            }
                          },
                          "type": "object"
+                        },
+                        "trackTimestampsStaleness": {
+                          "description": "TrackTimestampsStaleness whether Prometheus tracks staleness of the metrics that have an explicit timestamps present in scraped data. Has no effect if `honorTimestamps` is false. \n It requires Prometheus >= v2.48.0.",


Suggested change

"description": "TrackTimestampsStaleness whether Prometheus tracks staleness of the metrics that have an explicit timestamps present in scraped data. Has no effect if `honorTimestamps` is false. \n It requires Prometheus >= v2.48.0.",

"description": "`trackTimestampsStaleness` defines whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. Has no effect if `honorTimestamps` is false. It requires Prometheus >= v2.48.0.",

jan--f · 2023-11-22T10:31:46Z

pkg/apis/monitoring/v1/podmonitor_types.go

@@ -207,6 +207,15 @@ type PodMetricsEndpoint struct {
 	// +optional
 	HonorTimestamps *bool `json:"honorTimestamps,omitempty"`

+	// TrackTimestampsStaleness whether Prometheus tracks staleness of
+	// the metrics that have an explicit timestamps present in scraped data.


Suggested change

// the metrics that have an explicit timestamps present in scraped data.

// the metrics that have an explicit timestamp present in scraped data.

jan--f · 2023-11-22T10:32:09Z

pkg/apis/monitoring/v1/podmonitor_types.go

@@ -207,6 +207,15 @@ type PodMetricsEndpoint struct {
 	// +optional
 	HonorTimestamps *bool `json:"honorTimestamps,omitempty"`

+	// TrackTimestampsStaleness whether Prometheus tracks staleness of


Suggested change

// TrackTimestampsStaleness whether Prometheus tracks staleness of

// `trackTimestampsStaleness` defines whether Prometheus tracks staleness of

jan--f · 2023-11-22T10:32:41Z

pkg/apis/monitoring/v1/types.go

@@ -438,6 +438,14 @@ type Endpoint struct {
 	// +optional
 	HonorTimestamps *bool `json:"honorTimestamps,omitempty"`

+	// `trackTimestampsStaleness` defines whether Prometheus tracks staleness of
+	// the metrics that have an explicit timestamp present in scraped data.


Suggested change

// the metrics that have an explicit timestamp present in scraped data.

// the metrics that have an explicit timestamp present in scraped data.

// Has no effect if `honorTimestamps` is false.

jan--f · 2023-11-22T10:33:04Z

pkg/apis/monitoring/v1alpha1/scrapeconfig_types.go

@@ -135,6 +135,13 @@ type ScrapeConfigSpec struct {
 	// HonorTimestamps controls whether Prometheus respects the timestamps present in scraped data.
 	// +optional
 	HonorTimestamps *bool `json:"honorTimestamps,omitempty"`
+	// TrackTimestampsStaleness whether Prometheus tracks staleness of


Suggested change

// TrackTimestampsStaleness whether Prometheus tracks staleness of

// `trackTimestampsStaleness` whether Prometheus tracks staleness of

simonpasquier

Thanks!

simonpasquier · 2023-11-22T13:44:44Z

I'm not dismissing the latest comments from @jan--f but they can be addressed in a follow-up PR :)

dongjiang1989 added 2 commits November 17, 2023 23:07

add trackTimestampsStaleness

4151498

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

update add track_timestamps_staleness

0b1c5b0

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

dongjiang1989 requested a review from a team as a code owner November 17, 2023 16:12

pull-request-size bot added the size/L label Nov 17, 2023

dongjiang1989 added 3 commits November 18, 2023 00:23

fix controller-gen version bug

d7cb396

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

Merge branch 'main' into add-track_timestamps_staleness

c388b55

Update podmonitor_types.go

0664831

nicolastakashi reviewed Nov 17, 2023

View reviewed changes

simonpasquier reviewed Nov 20, 2023

View reviewed changes

dongjiang1989 and others added 4 commits November 21, 2023 10:02

Update pkg/apis/monitoring/v1alpha1/scrapeconfig_types.go

ed827ba

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

Update pkg/apis/monitoring/v1/types.go

5c2df15

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

update make generate

2c8582b

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

update api.md

610d2ca

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

jan--f reviewed Nov 21, 2023

View reviewed changes

jsonnet/prometheus-operator/prometheusagents-crd.json Outdated Show resolved Hide resolved

jan--f reviewed Nov 21, 2023

View reviewed changes

jsonnet/prometheus-operator/prometheuses-crd.json Outdated Show resolved Hide resolved

jan--f reviewed Nov 21, 2023

View reviewed changes

pkg/apis/monitoring/v1/podmonitor_types.go Show resolved Hide resolved

drop overrideTrackTimestampsStaleness global config

1c19805

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

simonpasquier reviewed Nov 21, 2023

View reviewed changes

dongjiang1989 and others added 6 commits November 21, 2023 23:14

chore: update documentation

eceb2c3

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

Update pkg/apis/monitoring/v1/types.go

8c45df5

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

Update pkg/prometheus/promcfg.go

6fef4e2

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

Update pkg/prometheus/promcfg.go

9624376

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

Update pkg/apis/monitoring/v1/podmonitor_types.go

e80ad74

Co-authored-by: Simon Pasquier <spasquie@redhat.com>

chore: update documentation by code review

8b64115

Signed-off-by: dongjiang1989 <dongjiang1989@126.com>

nicolastakashi approved these changes Nov 22, 2023

View reviewed changes

jan--f reviewed Nov 22, 2023

View reviewed changes

simonpasquier approved these changes Nov 22, 2023

View reviewed changes

simonpasquier merged commit 712a2c1 into prometheus-operator:main Nov 22, 2023
17 checks passed

dongjiang1989 deleted the add-track_timestamps_staleness branch November 23, 2023 03:32

dongjiang1989 mentioned this pull request Nov 27, 2023

chore: update PodMonitor/ServiceMonitor documentation #6120

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add track_timestamps_staleness limit to CRDs #6105

feat: add track_timestamps_staleness limit to CRDs #6105

dongjiang1989 commented Nov 17, 2023 •

edited

nicolastakashi Nov 17, 2023

dongjiang1989 Nov 20, 2023 •

edited

simonpasquier Nov 20, 2023

dongjiang1989 Nov 21, 2023

simonpasquier Nov 21, 2023

nicolastakashi Nov 21, 2023

dongjiang1989 Nov 21, 2023

nicolastakashi commented Nov 17, 2023

simonpasquier Nov 20, 2023

dongjiang1989 commented Nov 22, 2023

jan--f left a comment

jan--f Nov 22, 2023

jan--f Nov 22, 2023

jan--f Nov 22, 2023

jan--f Nov 22, 2023

jan--f Nov 22, 2023

jan--f Nov 22, 2023

jan--f Nov 22, 2023

simonpasquier left a comment

simonpasquier commented Nov 22, 2023

	"description": "`trackTimestampsStaleness` defines whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. \n It requires Prometheus >= v2.48.0.",
	"description": "`trackTimestampsStaleness` defines whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. Has no effect if `honorTimestamps` is false. It requires Prometheus >= v2.48.0.",

	"description": "TrackTimestampsStaleness whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. Has no effect if `honorTimestamps` is false. It requires Prometheus >= v2.48.0.",
	"description": "`trackTimestampsStaleness` defines whether Prometheus tracks staleness of the metrics that have an explicit timestamp present in scraped data. Has no effect if `honorTimestamps` is false. It requires Prometheus >= v2.48.0.",

	// the metrics that have an explicit timestamps present in scraped data.
	// the metrics that have an explicit timestamp present in scraped data.

	// TrackTimestampsStaleness whether Prometheus tracks staleness of
	// `trackTimestampsStaleness` defines whether Prometheus tracks staleness of

	// the metrics that have an explicit timestamp present in scraped data.
	// the metrics that have an explicit timestamp present in scraped data.
	// Has no effect if `honorTimestamps` is false.

	// TrackTimestampsStaleness whether Prometheus tracks staleness of
	// `trackTimestampsStaleness` whether Prometheus tracks staleness of

feat: add track_timestamps_staleness limit to CRDs #6105

feat: add track_timestamps_staleness limit to CRDs #6105

Conversation

dongjiang1989 commented Nov 17, 2023 • edited

Description

Type of change

Changelog entry

Choose a reason for hiding this comment

dongjiang1989 Nov 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolastakashi commented Nov 17, 2023

Choose a reason for hiding this comment

dongjiang1989 commented Nov 22, 2023

jan--f left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonpasquier left a comment

Choose a reason for hiding this comment

simonpasquier commented Nov 22, 2023

dongjiang1989 commented Nov 17, 2023 •

edited

dongjiang1989 Nov 20, 2023 •

edited