-
Notifications
You must be signed in to change notification settings - Fork 615
Monitoring API: remove maximum retention days prometheus #2851
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 all commits
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 |
|---|---|---|
|
|
@@ -1340,7 +1340,7 @@ type PrometheusConfig struct { | |
| // +kubebuilder:validation:MinItems=1 | ||
| Resources []ContainerResource `json:"resources,omitempty"` | ||
| // retention configures how long Prometheus retains metrics data and how much storage it can use. | ||
| // When omitted, the platform chooses reasonable defaults (currently 15 days retention, no size limit). | ||
| // When omitted, the platform chooses reasonable defaults (currently 360 hours retention, no size limit). | ||
| // +optional | ||
| Retention Retention `json:"retention,omitempty,omitzero"` | ||
| // tolerations defines tolerations for the pods. | ||
|
|
@@ -2235,24 +2235,24 @@ type SecretKeySelector struct { | |
| // Retention configures how long Prometheus retains metrics data and how much storage it can use. | ||
| // +kubebuilder:validation:MinProperties=1 | ||
| type Retention struct { | ||
| // durationInDays specifies how many days Prometheus will retain metrics data. | ||
| // durationInDays specifies how many hours Prometheus will retain metrics data. | ||
| // The JSON field name is durationInDays for wire compatibility with persisted objects; | ||
| // the value unit is hours (not days). | ||
| // Prometheus automatically deletes data older than this duration. | ||
| // When omitted, this means no opinion and the platform is left to choose a reasonable default, which is subject to change over time. | ||
| // The default value is 15. | ||
| // Minimum value is 1 day. | ||
| // Maximum value is 365 days (1 year). | ||
| // The default value is 360 (equivalent to 15 days). | ||
| // Minimum value is 1 hour. | ||
| // +kubebuilder:validation:Minimum=1 | ||
| // +kubebuilder:validation:Maximum=365 | ||
| // +optional | ||
| DurationInDays int32 `json:"durationInDays,omitempty"` | ||
| DurationInHours int32 `json:"durationInDays,omitempty"` | ||
| // sizeInGiB specifies the maximum storage size in gibibytes (GiB) that Prometheus | ||
|
Contributor
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 gibibytes a small enough unit? Is there a reasonable use case to limit this to smaller than a gibibyte? |
||
| // can use for data blocks and the write-ahead log (WAL). | ||
| // When the limit is reached, Prometheus will delete oldest data first. | ||
| // When omitted, no size limit is enforced and Prometheus uses available PersistentVolume capacity. | ||
| // Minimum value is 1 GiB. | ||
| // Maximum value is 16384 GiB (16 TiB). | ||
| // Maximum value is 2147483647 GiB (the maximum representable int32 value). | ||
| // +kubebuilder:validation:Minimum=1 | ||
| // +kubebuilder:validation:Maximum=16384 | ||
| // +kubebuilder:validation:Maximum=2147483647 | ||
| // +optional | ||
| SizeInGiB int32 `json:"sizeInGiB,omitempty"` | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4130,19 +4130,19 @@ spec: | |
| retention: | ||
| description: |- | ||
| retention configures how long Prometheus retains metrics data and how much storage it can use. | ||
| When omitted, the platform chooses reasonable defaults (currently 15 days retention, no size limit). | ||
| When omitted, the platform chooses reasonable defaults (currently 360 hours retention, no size limit). | ||
| minProperties: 1 | ||
| properties: | ||
| durationInDays: | ||
| description: |- | ||
| durationInDays specifies how many days Prometheus will retain metrics data. | ||
| durationInDays specifies how many hours Prometheus will retain metrics data. | ||
| The JSON field name is durationInDays for wire compatibility with persisted objects; | ||
| the value unit is hours (not days). | ||
| Prometheus automatically deletes data older than this duration. | ||
| When omitted, this means no opinion and the platform is left to choose a reasonable default, which is subject to change over time. | ||
| The default value is 15. | ||
| Minimum value is 1 day. | ||
| Maximum value is 365 days (1 year). | ||
| The default value is 360 (equivalent to 15 days). | ||
| Minimum value is 1 hour. | ||
| format: int32 | ||
| maximum: 365 | ||
|
Contributor
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. Just a note, a maximum of 2,147,483,647 here is equivalent to ~58835 centuries. Seems like an unreasonably high number that would never be set. Without more context, I would think it to be reasonable to increase this to something like 3650 to account for ~10 years of retention. That seems pretty high to me for metrics, but 🤷
Contributor
Author
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. done |
||
| minimum: 1 | ||
| type: integer | ||
| sizeInGiB: | ||
|
|
@@ -4152,9 +4152,9 @@ spec: | |
| When the limit is reached, Prometheus will delete oldest data first. | ||
| When omitted, no size limit is enforced and Prometheus uses available PersistentVolume capacity. | ||
| Minimum value is 1 GiB. | ||
| Maximum value is 16384 GiB (16 TiB). | ||
| Maximum value is 2147483647 GiB (the maximum representable int32 value). | ||
| format: int32 | ||
| maximum: 16384 | ||
| maximum: 2147483647 | ||
| minimum: 1 | ||
| type: integer | ||
| type: object | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
You can't change the semantics of an existing field name either. The correct way to do this is to tombstone the
durationInDaysfield by commenting it out entirely (and adding a notice that the field has been tombstoned) and adding the newdurationInHoursfield to replace it.We aren't concerned about existing instances because this API will only ever be present in a TPNU cluster, but we are concerned about potential clients that may serialize/deserialize based on the existing field name which is why we tombstone here (to make sure we never introduce a field with the same name but different serialization to prevent breaking clients). The tombstone can be removed when the API version is increased.