feat: add signoz chart#202
Conversation
📝 WalkthroughHey... Sharon! WalkthroughNew SigNoz monitoring chart added to the repository with Helm configurations for mainline and in-cluster deployments. Includes chart definitions, values configurations, documentation, and repository updates. No functional code changes—purely infrastructural stratification. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Wow... 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@charts/monitoring/signoz/README.md`:
- Around line 1-18: Add a short "Prerequisites" section to the
charts/monitoring/signoz/README.md above the "Helm values" heading that
explicitly lists required preconditions: that a StorageClass named
local-path-retain exists (or instruct users to adjust values.yaml to their
default StorageClass), that DNS and TLS are configured for the ingress host
(mention Traefik + cert-manager as an example), and that a monitoring namespace
exists (or instruct to enable namespace creation in Helmfile); keep the wording
concise and match the style of the file so users see these requirements before
running helmfile apply.
In `@charts/monitoring/signoz/values.yaml`:
- Around line 1-11: The ingress configuration under frontend.ingress is missing
an explicit TLS stanza for the traefik websecure entrypoint; add a tls block
(e.g., tls: - hosts: [signoz.hobroker.me] secretName: <tls-secret-name>) so
Traefik uses the correct certificate for host signoz.hobroker.me instead of a
default cert, and document or reference cert-manager/Let's Encrypt if the secret
is managed automatically; ensure ingressClassName: traefik and the
traefik.ingress.kubernetes.io/router.entrypoints: websecure annotation remain
unchanged.
- Around line 13-21: The chart uses the custom StorageClass name
"local-path-retain" (referenced under clickhouse.persistence.storageClass and
alertmanager.persistence.storageClass) which is defined in
charts/system/local-path-retain.yaml and must exist before SigNoz PVCs are
created; update docs and/or chart metadata to require that the system chart is
deployed first by adding a clear README note and/or a pre-install hook or
dependency guidance in the SigNoz chart (or helmfile) explaining that
charts/system/local-path-retain.yaml must be applied before installing the
monitoring/signoz release so PVCs do not remain Pending.
- Around line 23-27: Remove the obsolete otelCollectorMetrics top-level key—it's
not used by chart v0.111.0—and either delete that block or move its intent into
the main otelCollector configuration by defining metrics under
otelCollector.config.service.pipelines.metrics; update references to
otelCollectorMetrics to instead configure pipelines within otelCollector (look
for otelCollector and otelCollector.config.service.pipelines.metrics in the diff
to locate where to apply the change).
In `@README.md`:
- Around line 56-62: The README.md is out of sync with the generated docs; run
the project's documentation generator to regenerate README.md (so the charts
table entries like grafana-backup, prometheus-operator, scraparr, signoz,
wakatime-exporter are updated), review the regenerated README.md for any
unintended changes, and commit the updated README.md so CI's docs check passes.
| # `signoz` | ||
|
|
||
| > SigNoz is an open-source observability platform that helps you monitor your applications and troubleshoot problems. | ||
|
|
||
| Source Code: https://github.com/SigNoz/signoz | ||
| Chart: https://github.com/SigNoz/charts | ||
|
|
||
| ## Installing/upgrading | ||
|
|
||
| ```shell | ||
| helmfile apply | ||
| ``` | ||
|
|
||
| ### Helm values | ||
|
|
||
| | chart | values.yaml | | ||
| | -------- | -------------------------------------------------------------------- | | ||
| | `signoz` | https://github.com/SigNoz/charts/blob/main/charts/signoz/values.yaml | |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hey... Sharon! Add a short prerequisites section for storage class, TLS, and namespace. This doc reads like a turnkey install, but the values require local-path-retain, DNS/TLS for the ingress host, and a monitoring namespace. Make those explicit to avoid install surprises.
Doc addition
## Installing/upgrading
```shell
helmfile apply+## Prerequisites
+- local-path-retain StorageClass exists (or update values to your default StorageClass).
+- DNS + TLS for the ingress host (Traefik + cert-manager, if used).
+- monitoring namespace exists (or enable namespace creation in Helmfile).
+
Helm values
</details>
I thought this was America!
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 GitHub Actions: Docs</summary>
[error] 1-1: README.md is not up to date. Please run `npm run generate` and commit the updated file.
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @charts/monitoring/signoz/README.md around lines 1 - 18, Add a short
"Prerequisites" section to the charts/monitoring/signoz/README.md above the
"Helm values" heading that explicitly lists required preconditions: that a
StorageClass named local-path-retain exists (or instruct users to adjust
values.yaml to their default StorageClass), that DNS and TLS are configured for
the ingress host (mention Traefik + cert-manager as an example), and that a
monitoring namespace exists (or instruct to enable namespace creation in
Helmfile); keep the wording concise and match the style of the file so users see
these requirements before running helmfile apply.
</details>
<!-- fingerprinting:phantom:poseidon:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
| frontend: | ||
| ingress: | ||
| enabled: true | ||
| ingressClassName: traefik | ||
| annotations: | ||
| traefik.ingress.kubernetes.io/router.entrypoints: websecure | ||
| hosts: | ||
| - host: signoz.hobroker.me | ||
| paths: | ||
| - path: / | ||
| pathType: ImplementationSpecific |
There was a problem hiding this comment.
Hey... Sharon! Add explicit TLS config for the websecure entrypoint. Without a tls block, Traefik may serve a default cert or fail hostname validation for websecure. Consider adding a TLS stanza or documenting cert-manager handling.
Proposed TLS stanza (placeholder secret)
frontend:
ingress:
enabled: true
ingressClassName: traefik
annotations:
traefik.ingress.kubernetes.io/router.entrypoints: websecure
+ tls:
+ - hosts:
+ - signoz.hobroker.me
+ secretName: signoz-tls
hosts:
- host: signoz.hobroker.me
paths:
- path: /
pathType: ImplementationSpecific📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| frontend: | |
| ingress: | |
| enabled: true | |
| ingressClassName: traefik | |
| annotations: | |
| traefik.ingress.kubernetes.io/router.entrypoints: websecure | |
| hosts: | |
| - host: signoz.hobroker.me | |
| paths: | |
| - path: / | |
| pathType: ImplementationSpecific | |
| frontend: | |
| ingress: | |
| enabled: true | |
| ingressClassName: traefik | |
| annotations: | |
| traefik.ingress.kubernetes.io/router.entrypoints: websecure | |
| tls: | |
| - hosts: | |
| - signoz.hobroker.me | |
| secretName: signoz-tls | |
| hosts: | |
| - host: signoz.hobroker.me | |
| paths: | |
| - path: / | |
| pathType: ImplementationSpecific |
🤖 Prompt for AI Agents
In `@charts/monitoring/signoz/values.yaml` around lines 1 - 11, The ingress
configuration under frontend.ingress is missing an explicit TLS stanza for the
traefik websecure entrypoint; add a tls block (e.g., tls: - hosts:
[signoz.hobroker.me] secretName: <tls-secret-name>) so Traefik uses the correct
certificate for host signoz.hobroker.me instead of a default cert, and document
or reference cert-manager/Let's Encrypt if the secret is managed automatically;
ensure ingressClassName: traefik and the
traefik.ingress.kubernetes.io/router.entrypoints: websecure annotation remain
unchanged.
| otelCollector: | ||
| enabled: true | ||
|
|
||
| otelCollectorMetrics: | ||
| enabled: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
helm show values signoz/signoz --version 0.111.0 2>&1 | rg -n 'otelCollector|otelCollectorMetrics'Repository: hobroker/selfhosted
Length of output: 45
🏁 Script executed:
helm repo add signoz https://charts.signoz.io 2>&1 && \
helm repo update && \
helm show values signoz/signoz --version 0.111.0 2>&1 | head -50 | rg -n 'otelCollector|otelCollectorMetrics'Repository: hobroker/selfhosted
Length of output: 107
🌐 Web query:
signoz chart v0.111.0 values.yaml otelCollector otelCollectorMetrics
💡 Result:
In the SigNoz Helm chart signoz-0.111.0, the values.yaml does not have an otelCollectorMetrics section/key—only otelCollector. [1] (raw.githubusercontent.com)
To configure the collector pipelines (including metrics), you now do it under:
otelCollector.config- specifically
otelCollector.config.service.pipelines.metrics(and other pipelines) [1] (raw.githubusercontent.com)
otelCollector is defined in charts/signoz/values.yaml for chart signoz-0.111.0, and includes otelCollector.config as the main collector configuration block. [1] (raw.githubusercontent.com)
Citations:
- 1: https://raw.githubusercontent.com/SigNoz/charts/signoz-0.111.0/charts/signoz/values.yaml
- 2: https://raw.githubusercontent.com/SigNoz/charts/signoz-0.111.0/charts/signoz/values.yaml
- 3: https://raw.githubusercontent.com/SigNoz/charts/signoz-0.111.0/charts/signoz/values.yaml
Remove otelCollectorMetrics—this key doesn't exist in chart v0.111.0 and will be silently ignored, creating a tectonic rift in your config. The otelCollector key is valid, but otelCollectorMetrics has no foundation. In v0.111.0, metrics pipelines are configured under otelCollector.config.service.pipelines.metrics. Delete the orphaned otelCollectorMetrics block or restructure metrics config to flow through the main collector magma chamber.
Wow...
🤖 Prompt for AI Agents
In `@charts/monitoring/signoz/values.yaml` around lines 23 - 27, Remove the
obsolete otelCollectorMetrics top-level key—it's not used by chart v0.111.0—and
either delete that block or move its intent into the main otelCollector
configuration by defining metrics under
otelCollector.config.service.pipelines.metrics; update references to
otelCollectorMetrics to instead configure pipelines within otelCollector (look
for otelCollector and otelCollector.config.service.pipelines.metrics in the diff
to locate where to apply the change).
hobroker
force-pushed
the
add-signoz-chart
branch
from
0a2161e to
7ea89f5
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (3)
charts/monitoring/signoz/values.yaml (1)
2-12:⚠️ Potential issue | 🟡 MinorHey... Sharon! Add explicit TLS under ingress for the
websecurepath.Right now the ingress relies on entrypoint annotation only; adding
tlsmakes cert binding deterministic forsignoz.hobroker.me. Wow...🌋 Suggested patch
signoz: ingress: enabled: true className: traefik annotations: traefik.ingress.kubernetes.io/router.entrypoints: websecure + tls: + - hosts: + - signoz.hobroker.me + secretName: signoz-tls hosts: - host: signoz.hobroker.me paths: - path: / pathType: ImplementationSpecific port: 8080🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/monitoring/signoz/values.yaml` around lines 2 - 12, Ingress configuration currently sets entrypoint via annotations but lacks an explicit TLS section, causing non-deterministic cert binding; update the ingress block (ingress.enabled, ingress.className, ingress.annotations, ingress.hosts) to include a tls entry that references the host "signoz.hobroker.me" and maps it to the appropriate secretName so the websecure entrypoint is bound to a certificate deterministically. Ensure the tls.hosts contains signoz.hobroker.me and that tls.secretName matches the certificate secret you'll use for Traefik.charts/monitoring/signoz/README.md (1)
8-15:⚠️ Potential issue | 🟠 MajorHey... Sharon! Add a short prerequisites section before install steps.
This reads turnkey, but it depends on
local-path-retain, DNS/TLS forsignoz.hobroker.me, and amonitoringnamespace. Call these out explicitly to avoid magma-level install surprises. Wow...🌋 Suggested doc patch
## Installing/upgrading ```shell helmfile apply+## Prerequisites
+-local-path-retainStorageClass exists (or changevalues.yaml).
+- DNS + TLS are configured forsignoz.hobroker.me.
+-monitoringnamespace exists (or enable namespace creation in Helmfile).
+Helm values
</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@charts/monitoring/signoz/README.mdaround lines 8 - 15, Add a short
"Prerequisites" section before the "Installing/upgrading" step in
charts/monitoring/signoz/README.md that calls out required preconditions:
mention thelocal-path-retainStorageClass (or that the user must update
values.yamlto use another SC), that DNS and TLS must be configured for
signoz.hobroker.me, and that themonitoringnamespace must exist (or advise
enabling namespace creation in the Helmfile/Helm values); keep it brief and
place it immediately above the "Helm values" heading so users see these
requirements before runninghelmfile apply.</details> </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> `112-178`: _⚠️ Potential issue_ | _🟠 Major_ **Hey... Sharon! Regenerate `README.md` from the docs generator before merge.** The auto-generated Apps section is still out of sync with docs generation, and CI is blocking on it. Run `npm run generate` and commit the updated README. Wow... <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 112 - 178, The README's auto-generated "Apps" section
between the markers and is out of sync;
regenerate the docs and update README by running the docs generator (npm run
generate) locally, verify the Apps table is updated, then commit and push the
regenerated README.md so CI can pass; target the README.md file and the section
delimited by the markers to ensure the generated content replaces the stale
block.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In@charts/monitoring/signoz/README.md:
- Around line 8-15: Add a short "Prerequisites" section before the
"Installing/upgrading" step in charts/monitoring/signoz/README.md that calls out
required preconditions: mention thelocal-path-retainStorageClass (or that
the user must updatevalues.yamlto use another SC), that DNS and TLS must be
configured forsignoz.hobroker.me, and that themonitoringnamespace must
exist (or advise enabling namespace creation in the Helmfile/Helm values); keep
it brief and place it immediately above the "Helm values" heading so users see
these requirements before runninghelmfile apply.In
@charts/monitoring/signoz/values.yaml:
- Around line 2-12: Ingress configuration currently sets entrypoint via
annotations but lacks an explicit TLS section, causing non-deterministic cert
binding; update the ingress block (ingress.enabled, ingress.className,
ingress.annotations, ingress.hosts) to include a tls entry that references the
host "signoz.hobroker.me" and maps it to the appropriate secretName so the
websecure entrypoint is bound to a certificate deterministically. Ensure the
tls.hosts contains signoz.hobroker.me and that tls.secretName matches the
certificate secret you'll use for Traefik.In
@README.md:
- Around line 112-178: The README's auto-generated "Apps" section between the
markers and is out of sync; regenerate the
docs and update README by running the docs generator (npm run generate) locally,
verify the Apps table is updated, then commit and push the regenerated README.md
so CI can pass; target the README.md file and the section delimited by the
markers to ensure the generated content replaces the stale block.</details> --- <details> <summary>ℹ️ Review info</summary> **Configuration used**: Organization UI **Review profile**: ASSERTIVE **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 0a2161ebe97919b8e501538dad3b9d28e0e5d7ac and 7ea89f596bbc661390e9383abd02f20d67558fd8. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `README.md` * `charts/monitoring/signoz/README.md` * `charts/monitoring/signoz/helmfile.yaml` * `charts/monitoring/signoz/k8s-infra-values.yaml` * `charts/monitoring/signoz/values.yaml` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
1 participant
Description
This PR adds a new Helm chart configuration for SigNoz, an open-source observability platform.
Changes
charts/monitoring/signoz/directory.helmfile.yamlwith the official SigNoz repository.values.yamlwith:signoz.hobroker.me).local-path-retain) for ClickHouse and Alertmanager persistence.README.mdwith installation instructions.Summary by CodeRabbit
Release Notes