Skip to content

chore: prod to test backmerge - #419

Merged
mayankpande88 merged 46 commits into
testfrom
prod
May 5, 2026
Merged

chore: prod to test backmerge#419
mayankpande88 merged 46 commits into
testfrom
prod

Conversation

@mayankpande88

Copy link
Copy Markdown
Contributor

No description provided.

github-actions Bot and others added 30 commits January 27, 2026 14:42
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
Use HTTP interface (port 8123) instead of native TCP (port 9000) to
eliminate false-positive "deprecated interserver protocol" warnings
from ClickHouse logs.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
feat: add optional headless service for node-agent prometheus scraping
fix: switch OTel Collector ClickHouse exporter from native TCP to HTTP
loki-stack chart creates a Loki datasource ConfigMap with isDefault: true
by default. Combined with kube-prometheus-stack's Prometheus datasource
(also isDefault: true), Grafana fails during provisioning with:
"Only one datasource per organization can be marked as default"
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
fix: set loki datasource isDefault=false to prevent Grafana crash
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
…riggers

Rate limit defaults were hidden in Python trigger classes, making it
hard for customers to discover and override them. Now explicitly set
in values.yaml with inline comments.
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
fix: expose rate_limit params for builtin playbook triggers
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
Without --no-paginate, the JMESPath query runs per-page, returning
multiple image tags. The multi-line value causes InvalidImageTag errors.
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
fix: add --no-paginate to nudgebee-agent ECR query
…kup failure

When opentelemetry-collector.enabled is set to false, the ClickHouse
subchart is not deployed and its secret does not exist. The runner
template OR condition (otel-collector.enabled OR clickhouse_enabled)
still evaluated to true due to the old default, causing the runner pod
to fail on a missing ClickHouse secret.
mayankpande88 and others added 16 commits April 5, 2026 12:11
fix: default runner.clickhouse_enabled to false when otel-collector disabled
matchers is not part of the receiver schema in v1alpha1, causing helm
install/upgrade to fail with "field not declared in schema".
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
- Add helm template + kubeconform steps to both dev and prod workflows
  to validate rendered manifests against K8s and CRD schemas (strict mode)
- Enable validate-chart-schema in ct.yaml
- Add missing dependency repos and helm dependency build step
- Bump action versions (checkout@v4, setup-helm@v4, setup-python@v5,
  chart-testing-action@v2.7.0, Helm v3.14.0)
- Default create_nb_alert_config to false
fix: remove invalid matchers field from AlertmanagerConfig receiver
…ent control

Add 6 new flags for self-hosted/on-prem deployments and component toggling:
- `-w` relay WebSocket address
- `-c` collector endpoint URL
- `-i` custom image registry
- `-x` disable OpenCost
- `-t` disable OpenTelemetry Collector and ClickHouse
- `-g` disable Prometheus stack (skips auto-discovery/install)

Fixes nudgebee/nudgebee#10415
The default PrometheusRule shipped by this chart was generating
hundreds of firings per day on real clusters, with most carrying
no actionable signal. The four rules used absolute-count or sparse
expressions that fire on benign log lines and routine 4xx responses.

Re-tune each rule against signal characteristics rather than
arbitrary thresholds, validated against historical data on a dev
cluster (24-hour window):

  HighErrorCriticalLogs    250 -> 18 distinct containers (-93%)
  ApplicationAPIFailures   68  -> 19 distinct containers (-72%)
  KubePodStuckTerminating  73  -> 31 distinct pods       (-58%)
  KubeHpaMaxedOut          unchanged

Changes per rule:

* HighErrorCriticalLogs: was `increase(err logs)[5m] > 1` — fires
  on a single error message in 5 minutes. Replaced with deviation
  from baseline: current 5m rate must exceed 3x the same metric
  at `[1h] offset 1h`. The offset prevents an ongoing spike from
  inflating the baseline and silencing sustained incidents. An
  activity floor of 0.1 err/s and a `baseline > 0` guard avoid
  ratio blowups on idle and cold-start containers. The container_id
  exclusion regex now also drops log-collection infrastructure
  (fluent-bit, fluentd, loki, opensearch, elasticsearch, logstash,
  datadog, newrelic, otel, promtail) — these emit steady error-level
  log volume from transport/ingest failures and are not app signal.
  Severity demoted to `warning`: log-level alerting is structurally
  noisy (apps lie about levels) and almost always co-occurs with a
  higher-confidence signal.

* ApplicationAPIFailures: was `increase(4xx|5xx)[5m] > 1` — fires
  on a single request failure regardless of total volume. Replaced
  with a 5xx rate ratio: `5xx rate / total rate > 5%` plus an
  activity floor of 0.1 req/s. 4xx is removed entirely — those are
  client errors (bad request, unauthorised) and don't signal an
  application failure. `for` raised from 5m to 10m to ride over slow
  rolling updates.

* KubePodStuckTerminating: original expression
  `count(kube_pod_deletion_timestamp) * count(NodeLost==0) > 0`
  fires for any deleting pod, including pods in normal graceful
  termination. Replaced with an age-based expression
  `(time() - max_over_time(kube_pod_deletion_timestamp[20m])) > 1800`,
  i.e. pods stuck for 30 minutes — 60x the Kubernetes default
  terminationGracePeriodSeconds. `max_over_time(...[20m])` smooths
  across kube-state-metrics scrape gaps so the alert stays firing
  rather than flapping on every missed scrape (this was the main
  cause of the 537 state changes per 24h observed previously).
  The NodeLost suppression uses the cleaner `unless on(...) ... == 1`
  idiom. Severity demoted to `warning` — a single per-pod page for a
  stuck pod is rarely 3am-actionable and usually resolves with a
  manual force-delete.

* KubeHpaMaxedOut: unchanged behaviourally; only whitespace cleaned.

Container-id exclusion regexes for the log and HTTP rules are now
defined once at the top of the template via Helm template variables
(`$excludeLog`, `$excludeApi`) and reused, so additions stay in sync
across the duplicated denominator/numerator selectors.

Each new expression was tested directly against VictoriaMetrics
(which serves the same PromQL surface) to confirm the queries parse
and produce the expected reduction in firings.
…to otel pipeline

The shipped otel-collector pipeline was uniformly dropping 90% of all
traces via probabilistic_sampler regardless of status or latency. That
discards 90% of error and slow traces — the exact traces an SRE wants
to see — alongside the noise it was meant to remove. Replace with
tail_sampling: keep 100% of error traces, 100% of traces above 1s, and
sample 10% of healthy traffic.

Other improvements bundled with the sampling rewrite:

* memory_limiter is now defined AND threaded into all three pipelines
  as the first processor, so the collector back-pressures receivers
  under memory load instead of OOM-killing.

* filter/drop_health_check rewritten from 15 hardcoded path equality
  checks (covering /health, /healthz, /live, /ready, /metrics across
  three OTel attribute conventions) to three regex matches that also
  catch /livez, /readyz, /ping, /status, /api/health, /healthcheck,
  and namespaced variants.

* filter/drop_invalid_timestamps added to the traces pipeline. Drops
  spans with zero or inverted timestamps from broken upstream
  instrumentation.

Trade-off: tail_sampling holds spans for `decision_wait` (10s) before
deciding, costing ~50-100MB per replica with `num_traces: 50000`.
Constraint: tail sampling requires all spans of a trace to land on
the same collector instance. The chart's default deployment is a
single replica so works out of the box; customers scaling to >1
replica must add a consistent-hash load balancer (otel `loadbalancing`
exporter) in front, otherwise sampling decisions are made on partial
traces. Documented inline in values.yaml.
The numerator was grouping by (container_id, method, path, status) which
splits 500/502/503/etc into independent buckets. A service with 6% total
5xx but a 3% 500 + 3% 503 split would not fire because neither individual
status code crosses the 5% threshold. Drop status from the grouping so
the ratio measures aggregate 5xx rate per (container, method, path).

Reported by gemini-code-assist on PR #417.
…gs-prod

feat: add new flags to installation script for self-hosted and component control
fix: reduce monitoring noise — alert rules + otel trace sampling
The bare IsMatch() calls introduced in 0.0.123 fail to parse with the
filterprocessor's OTTL grammar, which requires conditions to be of the
form `<expr> <op> <value>`. The collector crash-loops with:

  processors::filter/drop_health_check: unable to parse OTTL statement:
  unexpected token "<EOF>" (expected <opcomparison> Value)

Append `== true` to each IsMatch() condition. Bumps chart to 0.0.124.
  Updated image tags to latest versions from ECR for prod branch.

  🤖 Generated with [Claude Code](https://claude.ai/code)

  Co-Authored-By: Claude <noreply@anthropic.com>
…-syntax

fix(otel): make filter/drop_health_check OTTL conditions parse

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Helm chart and installation script, introducing refined Prometheus alert rules, enhanced OpenTelemetry collector configurations with tail sampling and memory limiting, and new installation flags for self-hosted environments. Several critical issues were identified during the review: an invalid multi-line image tag in values.yaml that will break image pulls, a high-severity command injection vulnerability in installation.sh due to the use of eval with un-sanitized input, and a logic error in the ApplicationAPIFailures alert ratio calculation. Additionally, the reviewer pointed out a hardcoded architecture suffix in a sidecar image tag and redundant Prometheus scrape annotations on the node-agent service that could lead to duplicate metrics.

Comment thread charts/nudgebee-agent/values.yaml
Comment thread installation.sh
Comment thread charts/nudgebee-agent/templates/prometheus-alert-rule.yaml
Comment thread charts/nudgebee-agent/values.yaml
Comment thread charts/nudgebee-agent/templates/daemonset.yaml
@mayankpande88
mayankpande88 merged commit bd6fa85 into test May 5, 2026
2 checks 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.

3 participants