feat(base-cluster/monitoring): allow upsizing tempo storage#1448
Conversation
WalkthroughThe changes introduce a configurable persistent storage size for the tracing ingester component within the monitoring stack. This is achieved by updating the Helm chart template to accept a persistence size parameter, extending the values schema to validate this new property, and setting a default value in the configuration file. No changes were made to logic, exported entities, or other configuration sections beyond the tracing ingester persistence settings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm Chart
participant Kubernetes
participant PersistentVolume
User->>Helm Chart: Set tracing.ingester.persistence.size in values.yaml
Helm Chart->>Kubernetes: Deploy Grafana Tempo with persistence config
Kubernetes->>PersistentVolume: Provision volume of specified size
Kubernetes->>Grafana Tempo Ingester: Attach persistent volume
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the ability to upsize Tempo storage by adding a persistence configuration.
- Added persistence configuration under the ingester section in the values.yaml file.
- Updated the Grafana Tempo template to include persistence with the specified size.
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| charts/base-cluster/values.yaml | Added a persistence block for ingester with a default size of 10Gi |
| charts/base-cluster/templates/monitoring/tracing/grafana-tempo.yaml | Included a persistence block referencing the persistence size value |
Files not reviewed (1)
- charts/base-cluster/values.schema.json: Language not supported
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/base-cluster/values.schema.json (1)
967-976: Consistency with other persistence schema definitions
Unlike existing persistence schemas formonitoring.prometheus,alertmanager, andloki, this newtracing.ingester.persistenceblock only exposes asizeproperty and does not allow configuringstorageClass. Consider adding astorageClass(orstorageClassName) property to this schema to align with the other persistence blocks and enable users to override the storage class for Tempo ingester volumes.charts/base-cluster/values.yaml (1)
341-342: Clarify persistence toggling and storage class overrides
Thetracing.ingester.persistenceblock defines only asize, whereas other charts (e.g., Grafana) also includeenabledandstorageClassNameflags. If you intend to keep persistence always on, document this behavior in README. Otherwise, you may want to introduce anenabledflag or allow specifying astorageClassNamefor greater flexibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/base-cluster/templates/monitoring/tracing/grafana-tempo.yaml(1 hunks)charts/base-cluster/values.schema.json(1 hunks)charts/base-cluster/values.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: generateDiffCommentBody
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
🔇 Additional comments (1)
charts/base-cluster/templates/monitoring/tracing/grafana-tempo.yaml (1)
25-26: Verify alignment with the Bitnami Grafana-Tempo chart’s expected values
The template injectspersistence.sizeunderingester, but does not setpersistence.enabledorpersistence.storageClass. Confirm that the upstream Bitnami chart’s default foringester.persistence.enabledistrue(so persistence is actually enabled) and that it does not require astorageClasskey. Also double-check the indentation to ensure the block nests correctly underingester.
93d00aa to
a9f5f2e
Compare
🤖 I have created a release *beep* *boop* --- ## [8.1.0](base-cluster-v8.0.0...base-cluster-v8.1.0) (2025-06-06) ### Features * **base-cluster/monitoring:** allow upsizing tempo storage ([#1448](#1448)) ([db1a742](db1a742)) * **base-cluster/monitoring:** also read secrets for datasources ([#1479](#1479)) ([83ba8bd](83ba8bd)) * **base-cluster/monitoring:** configure service graph for grafana ([#1422](#1422)) ([8d4bb4c](8d4bb4c)) * **base-cluster/monitoring:** set code challenge for grafana ([#1500](#1500)) ([aa803da](aa803da)) * **base-cluster/monitoring:** set code_challenge_method for oauth2-proxy ([#1496](#1496)) ([b252cd7](b252cd7)) ### Bug Fixes * **base-cluster:** this prevents the user from installing this under another name ([#1418](#1418)) ([f4807e8](f4807e8)) ### Miscellaneous Chores * **base-cluster/docs:** update flux helmrelease command to update CRDs ([#1421](#1421)) ([a8fd535](a8fd535)) * **base-cluster/monitoring:** remove unnecessary open-telemetry-collector dashboard ([#1449](#1449)) ([520e9e1](520e9e1)) * **base-cluster:** change descheduler syntax ([#1483](#1483)) ([907bdae](907bdae)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced monitoring with support for upsizing tempo storage. - Enabled reading secrets for datasources and configuring the service graph in Grafana. - Added options to set the code challenge and code_challenge_method for Grafana and oauth2-proxy. - Introduced an optional persistence configuration for tracing ingester storage size. - **Bug Fixes** - Resolved an issue preventing installation of the chart under unintended names. - **Chores** - Updated helmrelease command for CRD updates. - Removed an unnecessary dashboard and adjusted descheduler syntax. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit