Tag RADIS telemetry per service in observability overlay#219
Conversation
Restructure the overlay so service.namespace and deployment.environment live
once in a shared env anchor; each service merges that anchor and adds only
its own SERVICE_COMPONENT value (consumed by adit_radis_shared/telemetry.py).
service.name comes from OTEL_SERVICE_NAME=${STACK_NAME}, dynamically resolved
by the existing CLI helper.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ChangesObservability Configuration Modularization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Code Review
This pull request refactors the observability configuration in docker-compose.override.yml.example by introducing separate anchors for network and environment settings and adding comprehensive documentation. Feedback suggests improving maintainability by leveraging Docker Compose variable interpolation for the environment setting instead of manual sed commands. Additionally, it is recommended to use the ${VAR:-} syntax for STACK_NAME and ENVIRONMENT to avoid 'variable not set' warnings and ensure consistency across configurations.
| # 2. If this is a staging or development deployment, edit the | ||
| # `deployment.environment=production` value inside `OTEL_RESOURCE_ATTRIBUTES` | ||
| # below to `staging` or `development`: | ||
| # sed -i 's/deployment.environment=production/deployment.environment=staging/' \ | ||
| # docker-compose.override.yml |
There was a problem hiding this comment.
Instead of recommending a manual sed workflow to update the environment tag, it's more idiomatic and maintainable to leverage Docker Compose's variable interpolation. Since the ENVIRONMENT variable is already defined in the project's example.env, we can use it directly in the configuration below.
# 2. If this is a staging or development deployment, set the `ENVIRONMENT`
# variable in your `.env` file to `staging` or `development`.
# The `deployment.environment` attribute below will automatically
# pick it up.
There was a problem hiding this comment.
There was a problem hiding this comment.
The linked discussion clarifies that the sed approach is intended as a one-time setup step for the user after copying the example file, rather than a runtime requirement. Given this context, the current implementation is acceptable as it provides a clear, explicit instruction for the initial configuration of the environment-specific deployment tag.
| OTEL_SERVICE_NAME: ${STACK_NAME} | ||
| OTEL_RESOURCE_ATTRIBUTES: "service.namespace=openradx,deployment.environment=production" |
There was a problem hiding this comment.
For consistency with docker-compose.base.yml and to avoid "variable not set" warnings when running docker compose outside of the CLI wrapper, use the ${VAR:-} syntax for STACK_NAME. Additionally, use the ENVIRONMENT variable for the deployment environment attribute as suggested in the header to simplify per-stack configuration.
OTEL_SERVICE_NAME: ${STACK_NAME:-}
OTEL_RESOURCE_ATTRIBUTES: "service.namespace=openradx,deployment.environment=${ENVIRONMENT:-production}"
There was a problem hiding this comment.
There was a problem hiding this comment.
Thank you for the link. I have reviewed the context provided in the linked discussion. Given that STACK_NAME is guaranteed to be injected by the CLI wrapper as described in the file header, the current implementation is acceptable. I will retract the suggestion regarding the :- syntax for STACK_NAME.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose.override.yml.example`:
- Around line 16-17: The sed -i example is not portable to BSD/mac sed; update
the example around the commented line containing "sed -i
's/deployment.environment=production/deployment.environment=staging/'" to show a
portable alternative: document both the GNU form (sed -i 's/.../.../') and the
BSD/mac form that requires a zero-length backup suffix (sed -i '' 's/.../.../'),
or replace with a portable one-liner (e.g., using perl -pi -e) and mention which
variant to use on macOS vs Linux so users on both platforms can run the command
without errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 270b8046-32f6-4b34-8382-08c2cd617285
📒 Files selected for processing (1)
docker-compose.override.yml.example
| # sed -i 's/deployment.environment=production/deployment.environment=staging/' \ | ||
| # docker-compose.override.yml |
There was a problem hiding this comment.
Make the sed -i example portable across GNU/BSD sed.
The current command works on GNU sed but fails on macOS/BSD sed without a backup suffix argument. Consider documenting both forms (or use a portable alternative) to avoid setup friction.
Suggested doc tweak
-# sed -i 's/deployment.environment=production/deployment.environment=staging/' \
-# docker-compose.override.yml
+# # GNU sed (Linux):
+# sed -i 's/deployment.environment=production/deployment.environment=staging/' \
+# docker-compose.override.yml
+# # BSD sed (macOS):
+# sed -i '' 's/deployment.environment=production/deployment.environment=staging/' \
+# docker-compose.override.yml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.override.yml.example` around lines 16 - 17, The sed -i example
is not portable to BSD/mac sed; update the example around the commented line
containing "sed -i
's/deployment.environment=production/deployment.environment=staging/'" to show a
portable alternative: document both the GNU form (sed -i 's/.../.../') and the
BSD/mac form that requires a zero-length backup suffix (sed -i '' 's/.../.../'),
or replace with a portable one-liner (e.g., using perl -pi -e) and mention which
variant to use on macOS vs Linux so users on both platforms can run the command
without errors.
Swarm interpolates {{.Task.Slot}} to the replica ordinal so telemetry.py
in adit-radis-shared can suffix service.component with `-N` (e.g.
default_worker-1, -2, -3). Outside Swarm the literal template is passed
through and the shared helper drops it, leaving dev signals clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Why
OpenObserve telemetry was tagged with a single `service.name` per stack, collapsing all containers under one identity. With this overlay every signal carries:
This makes telemetry filterable per stack, per component, and per deployment environment.
Dependency
`service.component` is set via a custom `SERVICE_COMPONENT` env var that the `adit-radis-shared` telemetry helper reads — see openradx/adit-radis-shared#186. `pyproject.toml` will need updating from tag `0.21.0` to that branch (or a follow-up tag) before this PR's effect is visible end-to-end.
Sibling PR
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit