Skip to content

Comments

Fix collectorDataSource retention env var conditions#336

Merged
mittal-ishaan merged 2 commits intoopencost:mainfrom
dag-andersen:fix/collector-datasource-retention-env-vars
Feb 20, 2026
Merged

Fix collectorDataSource retention env var conditions#336
mittal-ishaan merged 2 commits intoopencost:mainfrom
dag-andersen:fix/collector-datasource-retention-env-vars

Conversation

@dag-andersen
Copy link
Contributor

@dag-andersen dag-andersen commented Feb 17, 2026

Summary

  • Fix incorrect condition checks for retention10m, retention1h, and retention1d in the deployment template
  • The conditions were checking for non-existent field names (retentionResolution10m, retentionResolution1h, retentionResolution1d)
  • This caused the COLLECTOR_RESOLUTION_*_RETENTION environment variables to never be set, even when values were configured

Problem

The values.yaml defines:

collectorDataSource:
  retention10m: 36
  retention1h: 49
  retention1d: 15

But the template was checking for:

{{- if .retentionResolution10m }}
{{- if .retentionResolution1h }}
{{- if .retentionResolution1d }}

Instead of:

{{- if .retention10m }}
{{- if .retention1h }}
{{- if .retention1d }}

Alternative fix

Alternatively, the field names in values.yaml could be changed to match the template conditions:

collectorDataSource:
  retentionResolution10m: 36
  retentionResolution1h: 49
  retentionResolution1d: 15

I went with fixing the template since the values.yaml field names (retention10m, etc.) are more concise and consistent with the existing naming style in the file. But happy to change if you prefer the other approach.

Impact

Users configuring collectorDataSource.retention* values would not see any effect, as the environment variables were never set. The workaround was to use extraEnv to set the variables directly.

The condition checks for retention10m, retention1h, and retention1d were
incorrectly checking for non-existent field names (retentionResolution10m,
retentionResolution1h, retentionResolution1d), causing the env vars to never
be set even when the values were configured in values.yaml.
@dag-andersen dag-andersen marked this pull request as ready for review February 17, 2026 12:27
Copilot AI review requested due to automatic review settings February 17, 2026 12:27
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in the Helm chart template where environment variables for collector data source retention settings were never being set due to incorrect field name references in the conditional checks. The template was checking for non-existent fields (retentionResolution10m, retentionResolution1h, retentionResolution1d) instead of the actual field names defined in values.yaml (retention10m, retention1h, retention1d).

Changes:

  • Fixed three conditional checks in the deployment template to reference the correct field names that match the values.yaml schema

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mittal-ishaan
Copy link
Member

Thank you @dag-andersen ,
can you bump the patch version of the helm chart too?

@dag-andersen
Copy link
Contributor Author

@mittal-ishaan done 🚀

@mittal-ishaan mittal-ishaan merged commit 01c308d into opencost:main Feb 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants