Skip to content

feat(connectivity): provision OpenTelemetry monitoring on the delegated resource - #500

Merged
Dav-14 merged 2 commits into
feat/connectivity-modulefrom
feat/connectivity-monitoring
Jul 31, 2026
Merged

feat(connectivity): provision OpenTelemetry monitoring on the delegated resource#500
Dav-14 merged 2 commits into
feat/connectivity-modulefrom
feat/connectivity-monitoring

Conversation

@Dav-14

@Dav-14 Dav-14 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Wire OpenTelemetry observability into the Connectivity module, mirroring the Ledger v3 pattern.

  • Resolve the stack's OTEL configuration via settings.GetOpenTelemetryConfiguration(ctx, stack.Name, "connectivity"). This is collector-aware: it points the signals at otel-collector.<stack>:4318 when the per-stack collector Service exists, otherwise it honours the opentelemetry.* Settings.
  • Embed the resolved config inline in the delegated resource's spec.monitoring (same shape as Ledger v3's Cluster.spec.monitoring). The connectivity operator's MonitoringConfig is a struct embedded in the CRD spec — there is no separate Monitoring object to create or reference. The connectivity operator turns spec.monitoring into OTEL_* env vars on the workload.
  • Each present signal is enabled with the otlp exporter; traces are batched, metrics carry runtime instrumentation — matching the Ledger v3 mapping.

Idempotency

The whole spec.monitoring block is rebuilt inside the existing CreateOrUpdate mutate closure on every reconcile, and pruned (RemoveNestedField) when telemetry is disabled — so repeated reconciles converge to the same object.

Test plan

  • go test ./internal/resources/connectivities/... green
  • New unit tests: inline signal mapping, sorted resource attributes, idempotency (byte-for-byte identical spec on re-apply), omission of unconfigured signals, and pruning when disabled
  • New helpers at 100% coverage (applyConnectivityMonitoring, connectivityMonitoringSpec)
  • just pre-commit (tidy, lint, generate, manifests, helm, docs, settings-catalog)

Notes

  • Base branch is feat/connectivity-module — this stacks the observability wiring on top of the connectivity module work.
  • End-to-end OTEL_* env-var emission is owned by the connectivity operator repo and covered there.

@Dav-14
Dav-14 requested a review from a team as a code owner July 30, 2026 13:12
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 79e2cf13-1074-47c0-8308-37e86e34587a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…ed resource

Resolve the stack's OpenTelemetry configuration with
settings.GetOpenTelemetryConfiguration (collector-aware: points at
otel-collector.<stack>:4318 when the per-stack collector exists, else honours
the opentelemetry.* Settings) and embed it inline in the delegated
Connectivity's spec.monitoring, mirroring the Ledger v3 Cluster pattern. The
connectivity operator turns spec.monitoring into OTEL_* env vars on the
workload; there is no separate Monitoring object to reference.

The whole spec.monitoring block is rebuilt on every reconcile and pruned when
telemetry is disabled, so the reconcile stays idempotent.

Not-tested: end-to-end env-var emission by the connectivity operator (covered by
the connectivity repo); unit tests assert the inline spec mapping and idempotency.
@NumaryBot

NumaryBot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

✅ Approve — automated review

The OpenTelemetry monitoring wiring for the delegated Connectivity resource looks correct. The previously identified major issue — the pod-name=$(POD_NAME) attribute being forwarded verbatim without a downward-API environment variable defined — has been addressed in this revision by filtering out unresolved placeholder attributes before they reach the delegated workload's spec. No new discrete correctness issues were raised by the independent review. The change is ready to merge.

No findings.

@NumaryBot NumaryBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NumaryBot posted 1 new inline finding.

Summary: #500 (comment)

Comment thread internal/resources/connectivities/init.go Outdated
@Dav-14
Dav-14 force-pushed the feat/connectivity-monitoring branch from 1b46489 to 937367d Compare July 30, 2026 13:14

@NumaryBot NumaryBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NumaryBot posted 1 new inline finding.

Summary: #500 (comment)

Comment thread internal/resources/connectivities/init.go Outdated
…d monitoring

GetOpenTelemetryConfiguration injects pod-name=$(POD_NAME), which only
resolves when a downward-API POD_NAME env var is defined ahead of
OTEL_RESOURCE_ATTRIBUTES. The connectivity operator emits
OTEL_RESOURCE_ATTRIBUTES verbatim from spec.monitoring.attributes and
defines no such env var, so the placeholder surfaced literally in the
delegated workload's telemetry.

Strip attributes whose value carries an unresolvable $(...) placeholder
before forwarding, keeping literal resource attributes (stack, custom).
Omit the attributes field entirely when nothing resolvable remains.

@NumaryBot NumaryBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NumaryBot review complete: no remaining inline findings.

Resolved 1 stale NumaryBot review thread (0 fixed, 1 outdated).

Summary: #500 (comment)

@Dav-14
Dav-14 merged commit bd8285c into feat/connectivity-module Jul 31, 2026
12 checks passed
@Dav-14
Dav-14 deleted the feat/connectivity-monitoring branch July 31, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants