feat: Add global OTEL extension, backend service, and release automation to Argo template

[yesterdaysrebel]

This PR enables the ArgoCD OTEL UI extension globally, deploys a purpose-built in-cluster backend service (otel-extension-api) that the extension calls, and adds release automation to publish the backend container image to GHCR.

Changes Made

argocd.yaml.tpl

• Enables ArgoCD extension proxy (server.enable.proxy.extension)
• Adds YAML-valid commented placeholders for OTEL extension config, RBAC policies, server extension list, and backend extraObjects

main.tf

• Adds otel_enabled, otel_extension_version, otel_backend_tag, and tempo_base_url input variables (OTEL opt-in defaults to false)
• Renders OTEL extension config, RBAC, server extensions, and backend Deployment/Service into the template when otel_enabled = true
• Adds output precondition guards for OTEL inputs with whitespace trimming
• Adds required_version = ">= 1.2.0" Terraform constraint

backend/

• src/server.js — Node.js/Express server proxying /prometheus/* → PROMETHEUS_BASE_URL and /tempo/* → TEMPO_BASE_URL, with a /healthz health check
• package.json — Express + http-proxy-middleware dependencies
• Dockerfile — Node 20 Alpine container image
• README.md — Documents all endpoints and environment variables

.github/workflows/release.yml

• Builds and pushes ghcr.io/glueops/argocd-otel-extension-api to GHCR on every published release using docker/build-push-action

README.md

• Documents OTEL Terraform inputs and manual install guidance for non-Terraform users
• Updates Terraform usage example with all required inputs and correct output reference

[Copilot]

Pull request overview

This pull request updates the Argo CD Helm values template and accompanying documentation to enable a global Argo CD UI extension (OTEL) and deploy an in-cluster backend service that the extension calls.

Changes:

• Enables Argo CD’s proxying for extension backends and adds extension configuration in configs.cm.
• Adds Argo CD RBAC rules to allow invoking the otel-extension.
• Deploys an otel-extension-api Deployment and Service via extraObjects, and documents the new placeholders in README.md.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
README.md	Documents new placeholders and explains that the OTEL extension is configured globally.
argocd.yaml.tpl	Enables the extension feature, configures the extension + RBAC, and adds Kubernetes resources for the extension backend API.

---

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[yesterdaysrebel]

@copilot ## Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

---

In README.md:

> @@ -17,6 +17,13 @@ wget -O argocd.yaml https://raw.githubusercontent.com/GlueOps/docs-argocd/main/a
     - Replace `placeholder_tenant_key` with your tenant/company key. Example: `antoniostacos`
     - Replace `placeholder_cluster_environment` with your cluster_environment name. Example: `nonprod`
     - The `placeholder_argocd_oidc_client_secret_from_dex` that you specify needs to be the same one you use in the `platform.yaml` for ArgoCD. If they do not match you will not be able to login.
+    - OTEL is tenant-overridable through the Terraform module inputs:
+      - `otel_enabled` enables or disables the global ArgoCD OTEL extension for the tenant. The default is `false`, so tenants must opt in explicitly.
+      - `otel_extension_version` sets the GitHub release tag used for the extension tarball.
+      - `otel_backend_tag` sets the OTEL backend API image tag.
+      - `tempo_base_url` sets the in-cluster Tempo endpoint. Leave it empty to disable traces while keeping metrics enabled.
+    - If you are installing from the downloaded `argocd.yaml.tpl` directly instead of using Terraform, replace `placeholder_otel_enabled` with `true` or `false` before running Helm. Leave the OTEL placeholder comments in place to keep OTEL disabled, or replace those placeholder comments with concrete OTEL config, RBAC, server extension, and backend object blocks if you want OTEL enabled without Terraform.

The manual-install note references the downloaded file as argocd.yaml.tpl, but the command above downloads it to argocd.yaml (wget -O argocd.yaml ...). This can confuse users about which file to edit/run Helm against.

---

In README.md:

> +  otel_enabled        = true
+  otel_extension_version = "v0.1.1"
+  otel_backend_tag    = "v0.1.1"
+  tempo_base_url      = "http://tempo.glueops-core-tempo.svc.cluster.local:3200"

The Terraform module example adds OTEL inputs, but it still omits required module inputs (argocd_app_version and gatekeeper_tag) and the output reference doesn’t match this module’s actual output (helm_values). As written, the example won’t apply successfully.

[Copilot]

@copilot ## Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
...

Addressed in 9b85769: the manual-install note now references the downloaded argocd.yaml, and the Terraform example now includes argocd_app_version/gatekeeper_tag and uses module.argocd_helm_values.helm_values.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[yesterdaysrebel]

@copilot ## Pull request overview
This PR switches the Argo CD otel-extension proxy backend from Grafana to a purpose-built in-cluster backend service (otel-extension-api), and adds build/release automation for publishing that backend image to GHCR.

Changes:

• Update extension backend URL(s) to point at otel-extension-api and add Kubernetes Deployment/Service via Helm extraObjects.
• Add a new Node/Express backend service that proxies Prometheus query and Tempo search endpoints.
• Extend the release workflow to build/push the backend container image to GHCR.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per fileFileDescriptionvalues/shared/otel-extension.yamlPoints extension backend to the new API and deploys it via extraObjects in argocd.values/clusters/venus.yamlCluster-specific backend URL and extraObjects deployment in glueops-core.scripts/verify-otel-extension.shUpdates verification logic to validate the new backend URL and service existence.helm-values.yamlUpdates example Helm values to deploy and route to the new backend service.deploy.shUpdates deployment script to configure Argo CD extension backend to the new API URL.backend/src/server.jsAdds the new Express server implementing Prometheus/Tempo proxy endpoints and health check.backend/README.mdDocuments endpoints and environment configuration for the backend service.backend/package.jsonDefines backend package metadata and Express dependency.backend/DockerfileAdds container build for the backend service..github/workflows/release.ymlAdds GHCR login and Docker build/push steps for backend image release..github/configs/labeler.ymlAdds labeler configuration.

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

---

In scripts/verify-otel-extension.sh:

>  NAMESPACE=${NAMESPACE:-argocd}
 CLUSTER_PROFILE=${CLUSTER_PROFILE:-earth}
-EXPECTED_GRAFANA_URL=${EXPECTED_GRAFANA_URL:-https://grafana.nonprod.earth.onglueops.rocks}
-EXPECTED_VAULT_URL=${EXPECTED_VAULT_URL:-https://vault.nonprod.earth.onglueops.rocks}
+EXPECTED_OTEL_BACKEND_URL=${EXPECTED_OTEL_BACKEND_URL:-http://otel-extension-api.argocd.svc.cluster.local:8000}
 KUBE_CONTEXT=${KUBE_CONTEXT:-}

EXPECTED_OTEL_BACKEND_URL defaults to the hard-coded argocd namespace, which will cause false failures when running the script with a different NAMESPACE (e.g., glueops-core). Consider deriving the default from $NAMESPACE.

---

In deploy.sh:

>  NAMESPACE=${NAMESPACE:-argocd}
-GRAFANA_BACKEND_URL=${GRAFANA_BACKEND_URL:-http://grafana.monitoring.svc.cluster.local:3000}
+OTEL_BACKEND_URL=${OTEL_BACKEND_URL:-http://otel-extension-api.argocd.svc.cluster.local:8000}
 

OTEL_BACKEND_URL defaults to a hard-coded argocd namespace, so deploy.sh will configure an incorrect backend when deploying Argo CD into a different namespace. Also, renaming the env var from GRAFANA_BACKEND_URL to OTEL_BACKEND_URL is a breaking change for existing automation; consider keeping backward compatibility by falling back to the old variable.

---

In backend/src/server.js:

> +const PORT = Number(process.env.PORT || 8000);
+const LOG_LEVEL = (process.env.LOG_LEVEL || 'INFO').toUpperCase();
+const REQUEST_TIMEOUT_MS = Number(process.env.REQUEST_TIMEOUT_MS || 8000);
+const PROMETHEUS_BASE_URL = (process.env.PROMETHEUS_BASE_URL || '').replace(/\/$/, '');

Casting env vars with Number(...) can yield NaN (e.g., if PORT="" or REQUEST_TIMEOUT_MS="abc"), which can make the server listen on an invalid port or cause the request timeout to immediately abort (since setTimeout treats NaN as 0). Use a finite-number check and fall back to defaults.

---

In backend/src/server.js:

> @@ -0,0 +1,125 @@
+const express = require('express');
+
+const app = express();

Express sets the X-Powered-By header by default, which can leak implementation details. Consider disabling it to reduce fingerprinting surface.

---

In backend/Dockerfile:

> +COPY package.json ./
+RUN npm install --omit=dev
+

This Dockerfile installs dependencies without a lockfile (npm install with only package.json), which makes builds non-reproducible and can unintentionally pull in new dependency versions. Consider adding package-lock.json (or another lockfile) and switching to npm ci --omit=dev.

---

In helm-values.yaml:

> +              env:
+                - name: PORT
+                  value: "8000"
+                - name: PROMETHEUS_BASE_URL
+                  value: "http://kps-prometheus.glueops-core-kube-prometheus-stack.svc.cluster.local:9090"
+                - name: TEMPO_BASE_URL
+                  value: ""
+

The Helm example values deployment omits LOG_LEVEL while the other provided values (e.g., values/shared/otel-extension.yaml) set it. If users rely on consistent defaults across environments, consider adding LOG_LEVEL here as well.

---

In values/shared/otel-extension.yaml:

> +extraObjects:
+  - apiVersion: apps/v1
+    kind: Deployment
+    metadata:
+      name: otel-extension-api
+      namespace: argocd
+      labels:
+        app.kubernetes.io/name: otel-extension-api
+    spec:

The extraObjects Deployment/Service manifest is duplicated across multiple values files (values/shared/otel-extension.yaml, values/clusters/venus.yaml, and helm-values.yaml). This can easily drift (e.g., probes/resources differ already in helm-values.yaml). Consider centralizing these objects (e.g., via a shared values file or Helm template) and only overriding the small env/namespace differences per cluster.

---

name: "AI Code Review"
description: "Structured review prompt"
argument-hint: "Diff or PR context to review"
agent: "agent"

If required inputs are missing or ambiguous, ask concise clarifying questions first and wait for answers before continuing.

Before asking the user, investigate the workspace and the linked PR/issue: read the changed files, the surrounding code, and any tests in the same suite. Only ask if the answer is genuinely undiscoverable.

Inputs:

• (GitHub PR link, issue link, or diff summary)

Review this as a senior engineer and production gatekeeper.
Priority: correctness, security, reliability, compatibility, test gaps.

For each finding provide:

• Severity
• Evidence (file + line or code snippet)
• Concrete bad input (the exact input shape/value you tested or reasoned through)
• Why it matters
• Suggested fix

Only report defects you can describe with a concrete bad input. If you're uncertain whether behavior is intentional, phrase it as a question to the author rather than a finding.

Mandatory mechanical step (do this BEFORE writing the checklist review)

Review the diff line by line and locally, not holistically. For every changed or added line, ignore your model of the author's intent and ask in isolation: "What does this exact expression do, for every input the runtime allows?"

For each expression introduced or modified — conditionals, type checks, default chains, guards, accessors, comparisons, coercions, regexes, format strings, error messages, log lines — enumerate the inputs the language/runtime can actually pass through it (including the ones the author probably didn't think about), evaluate the new behavior for each, and compare it against the old behavior in the same spot.

For every string input that is validated, normalized, substituted, concatenated into a URL/path/image tag, or rendered into output, explicitly test the sibling cases of: clean value, leading/trailing whitespace, empty string, and if realistic for that boundary, newline-containing input. If the code validates trimspace(x) / strings.TrimSpace(x) / .trim() / equivalent but later renders the original untrimmed value, that is a defect: the validation and the emitted output disagree.

For every format string, error message, or log line that interpolates a value (%s/%v/%d/{0}/{}/${x}/f"{x}"/template literals/etc.), independently verify that each interpolation slot actually contains what the surrounding text claims. If the text says "got unsupported type X" the slot must contain the type, not the value; if it says "for resource Y" the slot must contain the resource identifier, not a stringified object. Mismatched message-vs-slot is a category of bug that line-local truthiness review misses but a sentence-by-sentence read of the message catches.

Flag anything where:

• behavior changed without a test covering the changed input,
• the result is invalid for downstream consumers (broken output, wrong type, malformed message),
• a sibling input of the same logical category reaches a different / less precise branch,
• an input the language considers "edge" (whatever those are for this language) is silently coerced, swallowed, or routed to the wrong error path,
• a user-facing message describes one thing but interpolates another (e.g. claims to show the "type" but shows the "value"; claims to show the "field name" but shows a stringified struct).

When the change touches a template, scaffold, generated file, config example, or README install instructions, review each supported consumption path separately. Do not assume the Terraform/rendered/code-driven path covers the raw/manual path. If a template introduces new placeholders, flags, or comments, verify what happens for a user who downloads or copies that file directly and follows the documented manual install steps.

When proposing or accepting a new validation that rejects an input class, verify the rejected inputs would actually misbehave downstream before locking the rejection in. Run a render/build/test with at least one representative member of each rejected class and confirm the prior behavior was broken. Rejecting inputs that previously round-tripped cleanly is a regression dressed as a fix — and "non-string", "non-empty", "non-zero", "non-null" are blunt instruments that often over-reject. Distinguish between truly unsafe inputs (e.g. silently swallowed by a default chain, would crash a downstream parser, would emit malformed output) and inputs that merely look unsafe but are handled correctly by an existing coercion (toString, String(x), implicit cast, etc.). Only the former warrant a hard fail.

When a function, method, helper, template, macro, or any callable is introduced or modified, enumerate its declared parameters and verify each is type-validated at entry against every realistic shape the caller could supply — including misuse via CLI flags (--set, --var), environment variables, deserialized JSON/YAML, query strings, request bodies, and CLI args. Treat the parameter list as a public API even when the callable is "internal" or "private," because callers in the same repo can and do pass arbitrary shapes. Critically: a default(...) / null-coalesce / ?? fallback / or default_value on a parameter is not validation — it only handles the unset/nil case, not the wrong-type case. A wrong-type value slips straight through and crashes deep inside the function with a low-level type error instead of an actionable boundary error. If a helper expects a map/dict/object/struct, assert that at entry; if it expects a string, assert that at entry; do not rely on downstream field accesses to surface the type mismatch.

Do this pass first. Do not let an architectural mental model of "what the change is for" cause you to skip line-local edge cases. The point of this step is to catch the class of bug that line-by-line reviewers find and holistic reviewers miss.

Before the findings list, include a short Mechanical Pass Coverage section that names the sibling inputs you actually exercised for the changed boundaries (for example: enabled/disabled, empty/whitespace-padded/clean string, nil/wrong-type/correct-type, generated/manual path).

Common Miss Patterns

Before finalizing the review, explicitly check for these high-frequency misses:

• Validated trimmed value, rendered untrimmed value: code checks trimspace(x) / .trim() / equivalent for emptiness or validity, but later substitutes or emits the original x. Test with a whitespace-padded concrete input.
• Generated path works, manual path breaks: Terraform/rendered output or helper CLI path succeeds, but the raw template / copied example / README manual path still contains a new placeholder, sentinel, or required replacement that users were not told to update.
• One sibling input tested, adjacent siblings ignored: reviewer checked "" but not " value "; checked nil but not wrong-type; checked enabled branch but not disabled branch.
• Docs describe a supported behavior, but only one implementation path was verified: a README claim may be true for the backend or generated output while being false for the manual install path, or vice versa.

---

Production Readiness Checklist

Apply these principles to every changed file. Think adversarially — assume the author was too close to the code to see their own mistakes.

1. Correctness

• Does the code do what it claims? Trace every code path manually against the stated intent.
• Are all branches exhaustive? Chains without a final catch-all silently swallow unexpected inputs — always ask "what happens to values not covered?"
• Are defaults safe? A silent fallback to a wrong value is harder to debug than an explicit failure.
• Do documentation, comments, and error messages match the actual implemented behavior? Mismatched docs are a maintenance hazard.
• Cross-check the PR title, PR description, and any linked issue against the actual implementation. PR descriptions go stale just like code comments. If the description says the code does X but the code does Y (e.g. "renders untagged image" vs "fails with error"), that is a finding — either the description or the code is wrong.
• Doc/code parity: verify every behavioral assertion against the current implementation. For every doc comment, README claim, error message, or PR description sentence that asserts behavior ("required for X", "supports Y", "always Z", "only used when W"), open the referenced code path and confirm the assertion holds against the current code, not an earlier draft. Stale assertions in helper doc-comments are as misleading as stale READMEs.
• Treat docs and templates as separate public interfaces. If the repo supports both an automated path and a manual path (for example: Terraform module vs downloaded YAML template, generated config vs hand-edited config, CLI helper vs raw manifest), verify that each path remains usable after the change. A fix that is safe in the generated path can still break the manual path if placeholders, defaults, or required replacements changed.

2. Input Validation & Defensive Coding

• Is every external or user-supplied input validated before use, at the earliest safe point?
• Is every field access guarded against the wrong parent type? Accessing .field on a value that could be a string, null, or list causes low-level crashes instead of actionable errors.
• Are all realistic input types handled explicitly? Each case must be either handled or rejected with a clear error — no silent fallthrough.
• Are empty, null, zero, and missing-key values treated consistently and deliberately throughout?
• For string inputs, are whitespace-padded values treated consistently from validation through final output? If the code trims for validation, it should usually trim for storage/rendering as well unless preserving whitespace is explicitly intended and documented.
• When a validation gap is found in one place, scan the entire function and file for all other inputs of the same kind. A fix applied to one field (e.g. type-checking a flag value) is incomplete if sibling fields (e.g. registry, repository) have the same vulnerability.
• Treat negative type guards as a silent-coercion smell. When a branch is gated by a negative type check (not kindIs "map", != nil, typeof x !== 'object', !isinstance(x, dict), etc.), confirm that every value flowing into that branch is one specific expected type — not "everything else." Replace the negative guard with a positive one (kindIs "string", isinstance(x, str)) and fail fast on unexpected types instead of coercing them (e.g. via toString, String(x), str(x)).
• Map keys rendered as YAML scalars must be quoted. When iterating user-supplied maps and rendering the key into YAML output ({{ $key }}, key: {{ $name }}, name: {{ .Key }}), the key must be wrapped in quote (or equivalent) unless it is a statically known identifier. YAML 1.1 — what Kubernetes parses — interprets unquoted true/yes/on/off/123/1.2/null as bool/int/float/null, not strings, which silently breaks downstream CRD or schema validation (e.g. ESO secretKey, label/annotation keys, env var names). This is sibling-asymmetric to value validation: confirming that a map value is a string does not cover the map key. Apply the same rule to any user-supplied identifier rendered into a YAML scalar position (resource names, label values, annotation values), not just map keys.

3. Fail-Fast & Error Quality

• Does the code fail loudly on invalid state rather than silently producing wrong output?
• Do error messages include enough context for an on-call engineer with no prior knowledge to diagnose the issue? (field name, resource name, received value, expected value)
• Are errors surfaced at the right layer — early enough to prevent downstream corruption, not so late that context is lost?

4. Output & Side Effect Quality

• Is every output (rendered template, API response, file, log) free of debug artifacts, inline documentation, or extra content that should not be present in production?
• Are side effects idempotent where required?
• Does the change affect observability (logs, metrics, alerts) in a way that helps or hinders production diagnosis?

5. Backward Compatibility

• Does the change break any existing public interface, config schema, or behavioral contract?
• For changed defaults: is the old default preserved as opt-out, or are consumers silently migrated?
• For removed behavior: is there a migration path and is it documented?

6. Security

• Is any user-controlled input used in a way that could lead to injection, path traversal, privilege escalation, or information disclosure?
• Are secrets, credentials, or PII never logged, never rendered into output, never stored in plaintext?
• Does the change introduce new attack surface (new endpoints, new permissions, new external calls)?

7. Test Coverage

• Is there a test for every new code path, including every explicit error/fail branch?
• Are edge-case inputs tested: empty, null, wrong type, boundary values?
• Do tests assert on the specific behavior being changed, not just that the code runs?
• Would these tests catch a regression if the change were accidentally reverted?

8. Hygiene

• Do all modified files end with a trailing newline?
• Are there leftover debug statements, commented-out code, or unresolved TODOs introduced by this change?
• Is the diff minimal — no unrelated reformatting or refactors mixed in?

Anti-patterns (do not produce)

• Reviews that only restate the diff in prose without naming a finding.
• Findings without severity or evidence ("this looks risky" — say where and why).
• Padding the report with low-value style nits when correctness/security findings exist.
• Approving silently when the mechanical pass surfaces an unresolved truth-table row.
• Skipping the mechanical pass because the change "looks obviously correct."

[Copilot]

@copilot ## Pull request overview
This PR switches the Argo CD otel-extension proxy backend from Grafana to a purpose-built in-cluster backend service (otel-extension-api), and adds build/releas...

Implemented in the latest commits. Added:

• backend/src/server.js — Express server proxying /prometheus/* → PROMETHEUS_BASE_URL and /tempo/* → TEMPO_BASE_URL, with a /healthz health check. Proxy errors are logged internally but return a generic 502 to the client.
• backend/package.json — Express + http-proxy-middleware (v3.0.7, patched for the CRLF injection CVE) with "engines": {"node": ">=20"}.
• backend/Dockerfile — Node 20 Alpine image using npm ci --omit=dev.
• backend/README.md — Documents endpoints and all environment variables.
• .github/workflows/release.yml — Builds and pushes ghcr.io/glueops/argocd-otel-extension-api to GHCR on every published release.
• .gitignore — Excludes backend/node_modules/.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 11 changed files in this pull request and generated 8 comments.

Files not reviewed (1)

• backend/package-lock.json: Generated file