Repository navigation
[monitoring] Add inlineScrapeConfig support to tenant vmagent - #2200
Andrei Kvapil (kvaps) merged 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds a conditional block in the VMAgent Helm template to render Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/extra/monitoring/values.yaml (1)
181-183: Also add this field to the structured values docs above.The example is useful, but
Vmagentis still documented as having onlyexternalLabelsandremoteWrite, so generated values docs will missadditionalScrapeConfigs. Please add an@fieldfor the new object and its expected shape.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extra/monitoring/values.yaml` around lines 181 - 183, The structured values docs for Vmagent are missing the new additionalScrapeConfigs field; update the docs section that defines Vmagent to include an `@field` entry for additionalScrapeConfigs (object | nullable) and document its expected shape with properties name (string) and key (string) so generated values docs include the example from values.yaml; locate the Vmagent documentation block in the structured values docs and add the `@field` line and a short description of the object and its properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/extra/monitoring/values.yaml`:
- Around line 181-183: The structured values docs for Vmagent are missing the
new additionalScrapeConfigs field; update the docs section that defines Vmagent
to include an `@field` entry for additionalScrapeConfigs (object | nullable) and
document its expected shape with properties name (string) and key (string) so
generated values docs include the example from values.yaml; locate the Vmagent
documentation block in the structured values docs and add the `@field` line and a
short description of the object and its properties.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30e5656d-a0a3-47db-b96a-40bb3ea6eef9
📒 Files selected for processing (3)
packages/extra/monitoring/values.yamlpackages/system/monitoring/templates/vm/vmagent.yamlpackages/system/monitoring/values.yaml
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the tenant VMAgent resource by integrating Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds support for additionalScrapeConfigs to the tenant VMAgent resource, allowing for custom scrape targets. The implementation in the Helm template is correct, and commented examples are added to the values.yaml files. My review focuses on a minor but important convention: ensuring files end with a newline character for better compatibility with POSIX tools. I've suggested adding a newline to both modified values.yaml files.
| - http://vminsert-longterm:8480/insert/0/prometheus | ||
| ## additionalScrapeConfigs: | ||
| ## name: my-secret | ||
| ## key: scrape-configs.yaml |
There was a problem hiding this comment.
Will check and fix if missing.
| - http://vminsert-longterm:8480/insert/0/prometheus | ||
| ## additionalScrapeConfigs: | ||
| ## name: my-secret | ||
| ## key: scrape-configs.yaml No newline at end of file |
There was a problem hiding this comment.
Will check and fix if missing.
There was a problem hiding this comment.
Fixed — added trailing newline to packages/system/monitoring/values.yaml. The other file (packages/extra/monitoring/values.yaml) already had one.
1375a5b to
b363bb3
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/extra/monitoring/values.yaml (1)
181-183: DocumentadditionalScrapeConfigsin theVmagenttypedef block too.This example makes the new value visible in the file, but the structured docs above still omit it, so generated values documentation will stay incomplete unless the
##FIELD.IO (@field)...annotations forVmagentare updated as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extra/monitoring/values.yaml` around lines 181 - 183, The values docs omit additionalScrapeConfigs because the Vmagent typedef block lacks a corresponding `@field` annotation; add a `@field additionalScrapeConfigs` entry to the Vmagent typedef (the typedef block that documents Vmagent fields) describing its structure (name: secret name, key: filename/key) and example usage so the generated documentation includes this new value; reference the Vmagent typedef and the additionalScrapeConfigs example in values.yaml when adding the annotation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/extra/monitoring/values.yaml`:
- Around line 181-183: The values docs omit additionalScrapeConfigs because the
Vmagent typedef block lacks a corresponding `@field` annotation; add a `@field
additionalScrapeConfigs` entry to the Vmagent typedef (the typedef block that
documents Vmagent fields) describing its structure (name: secret name, key:
filename/key) and example usage so the generated documentation includes this new
value; reference the Vmagent typedef and the additionalScrapeConfigs example in
values.yaml when adding the annotation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04d7eac5-b524-4e83-b3b1-f0470ad22195
📒 Files selected for processing (3)
packages/extra/monitoring/values.yamlpackages/system/monitoring/templates/vm/vmagent.yamlpackages/system/monitoring/values.yaml
b363bb3 to
8430e57
Compare
| - http://vminsert-shortterm:8480/insert/0/prometheus | ||
| - http://vminsert-longterm:8480/insert/0/prometheus | ||
| ## additionalScrapeConfigs: | ||
| ## name: my-secret |
There was a problem hiding this comment.
Standard tenant users have no access to read/create secrets or change anything expect resources under apps.cozystack.io group
There was a problem hiding this comment.
Good catch — switched from additionalScrapeConfigs (which references a Secret) to inlineScrapeConfig (inline YAML string). Since tenant users can only manage apps.cozystack.io resources and have no access to Secrets, inlineScrapeConfig is the right approach: the scrape config is passed directly through Helm values, no Secret needed.
Signed-off-by: Mattia Eleuteri <[email protected]> Signed-off-by: mattia-eleuteri <[email protected]>
8430e57 to
f5d8c89
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/extra/monitoring/values.yaml (1)
181-184: Add this option to the typedVmagentdocs too.The example helps, but
packages/extra/monitoring/values.yamlstill documentsVmagentwith onlyexternalLabelsandremoteWrite. Without a matching@fieldentry, the generated/reference values docs will stay incomplete for this new setting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/extra/monitoring/values.yaml` around lines 181 - 184, Add the new inlineScrapeConfig option to the typed Vmagent docs by adding a corresponding `@field` entry for inlineScrapeConfig in the Vmagent type documentation (the same place that documents externalLabels and remoteWrite) so the generated/reference values docs include this setting; update the Vmagent docblock to describe inlineScrapeConfig (expected YAML/array format and example) and ensure the field name matches inlineScrapeConfig used in packages/extra/monitoring/values.yaml.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/system/monitoring/values.yaml`:
- Around line 83-86: The values key and example currently reference
vmagent.inlineScrapeConfig but the PR, release notes, and intended API use
vmagent.additionalScrapeConfigs (Secret-backed name/key); update
packages/system/monitoring/values.yaml to replace the inlineScrapeConfig example
with a commented example for additionalScrapeConfigs showing the Secret name and
key, and modify the template in
packages/system/monitoring/templates/vm/vmagent.yaml to render
.Values.vmagent.additionalScrapeConfigs (and stop rendering
.Values.vmagent.inlineScrapeConfig), ensuring the values key, example text, and
template usage are consistent; remove or mark inlineScrapeConfig as deprecated
if present elsewhere.
---
Nitpick comments:
In `@packages/extra/monitoring/values.yaml`:
- Around line 181-184: Add the new inlineScrapeConfig option to the typed
Vmagent docs by adding a corresponding `@field` entry for inlineScrapeConfig in
the Vmagent type documentation (the same place that documents externalLabels and
remoteWrite) so the generated/reference values docs include this setting; update
the Vmagent docblock to describe inlineScrapeConfig (expected YAML/array format
and example) and ensure the field name matches inlineScrapeConfig used in
packages/extra/monitoring/values.yaml.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6cdea26f-9929-4f42-ba12-64ad37959833
📒 Files selected for processing (3)
packages/extra/monitoring/values.yamlpackages/system/monitoring/templates/vm/vmagent.yamlpackages/system/monitoring/values.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/system/monitoring/templates/vm/vmagent.yaml
| ## inlineScrapeConfig: | | ||
| ## - job_name: "custom" | ||
| ## static_configs: | ||
| ## - targets: ["my-service:9090"] |
There was a problem hiding this comment.
Align this values key with the feature the PR says it adds.
These comments document vmagent.inlineScrapeConfig, but the PR objective, release note, and usage example all describe a Secret-backed vmagent.additionalScrapeConfigs (name/key). The related template snippet in packages/system/monitoring/templates/vm/vmagent.yaml also renders inlineScrapeConfig, so the chart is exposing a different public API than the one requested. Please make the values key, example, and template consistent before merging.
Example if the intended API is still additionalScrapeConfigs
- ## inlineScrapeConfig: |
- ## - job_name: "custom"
- ## static_configs:
- ## - targets: ["my-service:9090"]
+ ## additionalScrapeConfigs:
+ ## name: my-secret
+ ## key: scrape-configs.yaml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/system/monitoring/values.yaml` around lines 83 - 86, The values key
and example currently reference vmagent.inlineScrapeConfig but the PR, release
notes, and intended API use vmagent.additionalScrapeConfigs (Secret-backed
name/key); update packages/system/monitoring/values.yaml to replace the
inlineScrapeConfig example with a commented example for additionalScrapeConfigs
showing the Secret name and key, and modify the template in
packages/system/monitoring/templates/vm/vmagent.yaml to render
.Values.vmagent.additionalScrapeConfigs (and stop rendering
.Values.vmagent.inlineScrapeConfig), ensuring the values key, example text, and
template usage are consistent; remove or mark inlineScrapeConfig as deprecated
if present elsewhere.
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
LGTM, thank you!
Summary
inlineScrapeConfigsupport to the tenantVMAgentresource inpackages/system/monitoring/templates/vm/vmagent.yamlpackages/system/monitoring/values.yamlandpackages/extra/monitoring/values.yamlProblem
The tenant VMAgent resource does not support custom scrape configurations. Using
additionalScrapeConfigs(which references a Kubernetes Secret) is not viable because tenant users have no access to create or read Secrets — they can only manage resources underapps.cozystack.io. Instead,inlineScrapeConfigaccepts inline YAML directly through Helm values, which is consistent with tenant permissions.Relates to #2194
Usage
Test plan
helm templatemonitoring withinlineScrapeConfigset — block renderedhelm templatemonitoring without it — noinlineScrapeConfigin outputSummary by CodeRabbit
New Features
Documentation