feat(telemetry): integrate the new FOE telemetry sidecar by mo-getter · Pull Request #566 · voxel51/fiftyone-teams-app-deploy

mo-getter · 2026-05-19T14:24:24Z

Rationale

We are launching new telemetry features in 2.19.0. This requires changes to our chart and compose files.

Review Priority

high
medium
low

Changes

Helm
- Adds a sidecar container to the following pods:
  - fiftyone-app
  - teams-api
  - plugins
  - do deployments
  - do jobs (as a native sidecar)
- Adds a role and rolebinding for telemetry
- Adds get pods/log permission to api role
- Adds an optional redis deployment and service, with a further optional chart-managed pvc, or a further optional existing PVC by name
Compose
- drops support for FIFTYONE_DELEGATED_OPERATOR_WORKER_REPLICAS config option, replacing it with COMPOSE_PROFILES="do-3"
- adds sidecar containers to same services as Helm
- adds a tmpfs volume for the telemetry socket
adds support for the new telemetry-sidecar image

Checklist

This PR maintains parity between Docker Compose and Helm

Testing

unit
integration
manual e2e tests:
- local
- release/compose
- release/k8s

coderabbitai · 2026-05-19T14:24:39Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

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

Run ID: 4ca7afa6-1b44-43a5-b9b8-bab9f5226294

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

Walkthrough

This PR adds telemetry sidecars and a bundled Redis backend to FiftyOne Enterprise v2.19+, enabling metrics and log collection for delegated operators and core services. Documentation, environment templates, Helm template helpers, and test coverage are added across Docker Compose and Kubernetes platforms.

Changes

Telemetry Sidecars and Infrastructure

Layer / File(s)	Summary
Telemetry deployment and configuration documentation `docker/README.md`, `docker/docs/configuring-telemetry.md`, `docker/docs/upgrading.md`, `docs/orchestrators/configuring-kubernetes-orchestrator.md`, `helm/docs/upgrading.md`	Documents telemetry sidecar architecture, Redis backend, default behavior, resource impact, host/cluster requirements (process namespace sharing, elevated capabilities), external Redis configuration, and opt-out procedures with implications for delegated-operator log viewing.
Docker environment templates and deployment configuration `docker/internal-auth/env.template`, `docker/legacy-auth/env.template`, `helm/fiftyone-teams-app/templates/NOTES.txt`	Adds telemetry configuration sections to Docker Compose env templates with optional variables for Redis URL, image, maxmemory, namespace labeling, and target name matching. NOTES.txt warns of Redis persistence risks in ephemeral deployments.
Helm telemetry template helpers and infrastructure `helm/fiftyone-teams-app/templates/_telemetry.tpl`	New template file defining 11 reusable Helm named templates: Redis naming/URL resolution (bundled vs. external), label/selector helpers, RBAC role and rolebinding subjects, shared sidecar environment variables, and two sidecar container variants (standard and native-sidecar with restartPolicy and readiness probe).
Wire telemetry into existing Helm templates `helm/fiftyone-teams-app/templates/_helpers.tpl`, `helm/fiftyone-teams-app/templates/_do_targets.tpl`	Updates helper templates to inject telemetry Redis URL and socket environment variables into API, app, plugins, teams-app, and delegated-operator deployment/service env lists when telemetry is enabled.
Helm chart values documentation updates `helm/fiftyone-teams-app/README.md`	Documents explicit podSecurityContext defaults (UID/GID 1000, runAsNonRoot) for apiSettings, appSettings, pluginsSettings, and delegated operator templates. Adds telemetry values section covering enabled flag, Redis container/pod security, external URL, persistence options, and sidecar configuration.
Helm unit tests for telemetry Redis, RBAC, and sidecars `tests/unit/helm/telemetry-redis_test.go`, `tests/unit/helm/telemetry-rolebinding_test.go`, `tests/unit/helm/telemetry-sidecar_test.go`, `tests/unit/helm/yaml_helpers.go`, `tests/unit/helm/common_test.go`	New comprehensive test suites: telemetry-redis validates Deployment/Service/PVC rendering with bundled/external URL wiring and persistence; telemetry-rolebinding verifies Role and RoleBinding RBAC; telemetry-sidecar checks shareProcessNamespace and security contexts. Helper file provides YAML splitting and disableTelemetry utilities.
Update existing Helm deployment tests for telemetry compatibility `tests/unit/helm/api-deployment_test.go`, `tests/unit/helm/app-deployment_test.go`, `tests/unit/helm/plugins-deployment_test.go`, `tests/unit/helm/teams-app-deployment_test.go`, `tests/unit/helm/delegated-operator-instance-deployment_test.go`, `tests/unit/helm/delegated-operator-job-configmap_test.go`, `tests/unit/helm/api-role_test.go`	Systematically wraps test values with disableTelemetry() to disable telemetry during standard test assertions. Updates pod security context expectations to reflect UID/GID 1000 defaults. Adds TestTelemetrySocketInjection and updated TestRoleRules validations.
Docker Compose tests for telemetry service verification `tests/unit/compose/docker-compose-internal-auth_test.go`, `tests/unit/compose/docker-compose-legacy-auth_test.go`	Expands test matrices to verify telemetry services (telemetry-redis and per-service sidecars) are discovered and properly configured. Adds delegated-operations GPU compose file support. Validates telemetry environment variables and volumes across service configurations.
Test fixtures and utility script updates `tests/fixtures/docker/integration_internal_auth.env`, `tests/fixtures/docker/integration_legacy_auth.env`, `utils/bump-fixtures-helm.sh`, `utils/validate-docker-pulls.sh`	Bumps test fixture versions to v2.19.0rc12 / v2.19.0-rc.11. Updates bump-fixtures-helm.sh to set telemetry sidecar image tags and validate-docker-pulls.sh to include redis:7-alpine and voxel51/telemetry-sidecar in expected images.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hops with glee, the telemetry flows,
Redis and sidecars, where data goes,
Metrics and logs in the cloud so bright,
Tests verify all is properly wired tight!
v2.19 brings observability's light! ✨📊

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and concisely describes the main change: integrating the new FOE telemetry sidecar feature across the deployment infrastructure.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description covers all required sections: rationale explains the telemetry launch, changes are well-detailed across Helm and Compose, and testing approach is described.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch alan/telemetry-including-redis

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…v2.19.0-rc.5 api:v2.19.0rc6 app:v2.19.0rc6 cas:v2.19.0-rc.5

Add an opt-in escape hatch for operators running managed Redis (ElastiCache, MemoryStore, etc.) or an existing shared Redis. When `telemetry.redis.external.url` is set, the chart skips the bundled Redis `Deployment`/`Service`/`PersistentVolumeClaim` and wires both consumer workloads and auto-injected sidecars at the external URL. Threads through a new `telemetry.redis.url` helper that returns either the external URL or the in-cluster `redis://<release>-telemetry-redis` URL, replacing the inline `printf` previously used in two places. Tests in tests/unit/helm/telemetry-redis_test.go cover: bundled Redis NOT rendered when external.url set; api deployment env var points at the external URL on every container (workload + sidecar); the default release-scoped URL still wires correctly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Replace the single `telemetry.redis.storage` knob with a `telemetry.redis.persistence` block: - `persistence.enabled` (default `true`) — when `false`, skip the PVC and run Redis on an `emptyDir`. State is lost on pod restart; intended for dev/minikube clusters without a dynamic PV provisioner. - `persistence.size` — replaces `storage`. Same default (`1Gi`). - `persistence.storageClass` — passed through to the PVC. Leave unset to use the cluster's default `StorageClass`. Tests added: PVC has the expected `storageClassName` when set; `persistence.enabled=false` skips the PVC and the Deployment switches the `redis-data` volume to `emptyDir`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

The delegated-operator workload containers and DO Job templates were still building `FIFTYONE_TELEMETRY_REDIS_URL` inline via `printf "redis://%s:6379" (include "telemetry.redis.name" .ctx)`, bypassing the `telemetry.redis.url` helper added in the external-Redis commit. As a result, setting `telemetry.redis.external.url` correctly pointed api/app/cas/plugins/teams-app at the external URL but left the DO workloads pointing at the in-cluster Service. Both sites now go through `telemetry.redis.url .ctx`. Regression tests added to tests/unit/helm/telemetry-redis_test.go cover both the DO deployment workload container and the DO Job ConfigMap. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…v2.19.0-rc.15 api:v2.19.0rc16 app:v2.19.0rc16 cas:v2.19.0-rc.15

…v2.19.0-rc.17 api:v2.19.0rc18 app:v2.19.0rc18 cas:v2.19.0-rc.17

…ume-ownership' into release/v2.19.0-volume-ownership

fix: add driver opts for telemetry-socket

kevin-dimichel

Review part 1:

There is still one code reference to FIFTYONE_DELEGATED_OPERATOR_WORKER_REPLICAS in
https://github.com/voxel51/fiftyone-teams-app-deploy/blob/main/tests/fixtures/docker/.env#L12C25-L12C31. Let's remove that.

Other than ^^ I tried to make suggestions that can be batch added. Formatting should be ok. I edited in my local IDE and copied it suggestion block.

kevin-dimichel

Review part 2:

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Kacey <kacey@voxel51.com>

kevin-dimichel

Review part 3:

let's scrap the triplicated/duplicated parts and refer back to the source docs.

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Alan Smith <209585+mo-getter@users.noreply.github.com>

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com>

kevin-dimichel

Review part 4:

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Alan Smith <209585+mo-getter@users.noreply.github.com>

kevin-dimichel

Review Part 5:

mo-getter force-pushed the alan/telemetry-including-redis branch 2 times, most recently from 05e0b27 to 8e12a91 Compare May 19, 2026 20:01

mo-getter changed the title ~~feat(helm): add telemetry Redis backend gated on telemetry.enabled~~ feat(helm): add redis to chart for telemetry May 19, 2026

mo-getter force-pushed the alan/telemetry-including-redis branch from 8e12a91 to 82aa5ac Compare May 19, 2026 21:27

kaixi-wang and others added 25 commits May 21, 2026 14:13

feat: enable extraContainers

0ca266b

fix: share process namespace

2151e2e

fix: add logs access

a46b8f0

chore: update tests

93359dc

fix: rm unread rbac file

4c2865e

feat: add nativeSidecarContainers

adb3b9e

fix: add missing files

9578958

chore: update compose files

e9e710a

chore: fix tests

ee0ba72

chore: fix tests

1ca27a4

chore: trigger ci

30282b1

fix: auto-inject sidecar settings

ea4c386

fix: fix auto inject sidecar vars and update test

d58c01c

chore: attempt to fix test

d532ec4

feat: default true for telemetry and add templates

fe89df4

feat: enable sidecar by default in docker deployments

8348c4b

chore: temp fix for tests

b290329

feat: default enable telemetric with all default settings generated

229c9eb

test: temporarily hardcode the sidecar

5d410c4

fix: use latest sidecar version 0.1.62

a8c0357

fix: runAsNonRoot override in sidecar

047041c

ci(fixtures): automated bump for integration test fixtures teams-app:…

c4013e9

…v2.19.0-rc.5 api:v2.19.0rc6 app:v2.19.0rc6 cas:v2.19.0-rc.5

This was referenced May 27, 2026

fix(telemetry): set kernel user to match workload user for logs #572

Closed

feat: add support for extraContainers and nativeSidecarContainers #553

Closed

voxelbuildbot and others added 8 commits May 27, 2026 05:45

ci(fixtures): automated bump for integration test fixtures teams-app:…

54a13c5

…v2.19.0-rc.15 api:v2.19.0rc16 app:v2.19.0rc16 cas:v2.19.0-rc.15

fix: use fiftyone debug log msg instead of sanic motd

49ffb73

fix: use connecting str

95dd6a7

fix: add driver-ops and docs warning

8abcc14

ci(fixtures): automated bump for integration test fixtures teams-app:…

94b5263

…v2.19.0-rc.17 api:v2.19.0rc18 app:v2.19.0rc18 cas:v2.19.0-rc.17

chore: rm comments about upgrade issues and rm pythonbuffered

b56fe9f

Merge remote-tracking branch 'refs/remotes/origin/release/v2.19.0-vol…

b9462ad

…ume-ownership' into release/v2.19.0-volume-ownership

Merge pull request #576 from voxel51/kacey/v2.19.0-volume-ownership

82453e8

fix: add driver opts for telemetry-socket

kaixi-wang mentioned this pull request May 27, 2026

fix executor sidecar #574

Closed

4 tasks

fix(tests): flaking on log entries again

a0e7ece

kevin-dimichel reviewed May 28, 2026

View reviewed changes

kaixi-wang and others added 2 commits May 27, 2026 17:45

Update docker/docs/configuring-delegated-operators.md

79d57ab

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Kacey <kacey@voxel51.com>

Update docker/docs/configuring-gpu-workloads.md

a27178f

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Kacey <kacey@voxel51.com>

kevin-dimichel reviewed May 28, 2026

View reviewed changes

mo-getter and others added 4 commits May 27, 2026 20:57

Apply suggestions from code review

f435db5

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Alan Smith <209585+mo-getter@users.noreply.github.com>

Apply suggestions from code review

170cddd

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Alan Smith <209585+mo-getter@users.noreply.github.com>

fix: apply suggestions from code review

d2f7f32

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com>

fix: apply suggestion from code review

ed9cd75

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com>

kevin-dimichel reviewed May 28, 2026

View reviewed changes

mo-getter and others added 2 commits May 27, 2026 21:35

Apply suggestions from code review

e08cff5

Co-authored-by: Kevin DiMichel <56850465+kevin-dimichel@users.noreply.github.com> Signed-off-by: Alan Smith <209585+mo-getter@users.noreply.github.com>

style: fix pre-commit lints

d25ee44

kevin-dimichel reviewed May 28, 2026

View reviewed changes

Comment thread tests/integration/compose/docker-compose-legacy-auth_test.go

kevin-dimichel approved these changes May 28, 2026

View reviewed changes

mo-getter merged commit 3feb555 into release/v2.19.0 May 28, 2026
12 checks passed

mo-getter deleted the alan/telemetry-including-redis branch May 28, 2026 02:12

coderabbitai Bot mentioned this pull request May 28, 2026

Release/v2.19.0 #577

Merged

4 tasks

coderabbitai Bot mentioned this pull request Jun 27, 2026

feat(sidecar): support extraVolumes #588

Merged

4 tasks

Uh oh!

Conversation

mo-getter commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Changes

Testing

Uh oh!

coderabbitai Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Review ran into problems

Uh oh!

kevin-dimichel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevin-dimichel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevin-dimichel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevin-dimichel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevin-dimichel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mo-getter commented May 19, 2026 •

edited

Loading

coderabbitai Bot commented May 19, 2026 •

edited

Loading

kevin-dimichel left a comment •

edited

Loading