-
Notifications
You must be signed in to change notification settings - Fork 185
feat: refactor observability configs for Compose and add for Local #351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: JaredforReal <[email protected]>
✅ Deploy Preview for vllm-semantic-router ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
👥 vLLM Semantic Team NotificationThe following members have been identified for the changed files in this PR and have been automatically assigned: 📁
|
Need further test, leave it as a draft. |
Signed-off-by: JaredforReal <[email protected]>
Signed-off-by: JaredforReal <[email protected]>
else | ||
log_info " - Check status: docker compose ps" | ||
log_info " - View logs: docker compose logs prometheus grafana -f" | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be possible to add open-webui in a followup PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, all of these will be consolidated in the Modern Dashboard. Working on it
PROJECT_ROOT="$(git -C "$SCRIPT_DIR" rev-parse --show-toplevel)" | ||
else | ||
# Fallback: three levels up from scripts -> repo root | ||
PROJECT_ROOT="$(cd "$SCRIPT_DIR/../../../" && pwd)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you validate if the PROJECT_ROOT contains the tools directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checked, and it's good
tools/make/common.mk
Outdated
@echo "" | ||
@echo " Observability targets:" | ||
@echo " run-observability - Start observability (alias for obs-local)" | ||
@echo " obs-local - Start observability in LOCAL mode" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: obs-> o11y (obs could be confused with something else)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@JaredforReal this is really helpful! Would you please record a demo and add the demo link to the docs too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors observability configurations to unify Docker Compose and Local deployment modes, consolidating Prometheus and Grafana configurations into a shared tools/observability/
directory.
- Consolidates observability configs from
config/
to unifiedtools/observability/
directory - Adds new local mode support with
docker-compose.obs.yml
for running router natively while observability runs in Docker - Introduces convenient
make
commands for easy observability stack management
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
tools/observability/ |
New unified config directory with Prometheus, Grafana configs, dashboard JSON, and management scripts |
docker-compose.yml |
Updated to use new observability configs and added persistent volumes |
docker-compose.obs.yml |
New compose file for local mode (router on host, observability in Docker) |
tools/make/observability.mk |
New makefile with observability management targets |
website/docs/tutorials/observability/observability.md |
Comprehensive rewrite to document the new unified approach |
config/prometheus.yaml , config/grafana/ |
Removed old config files replaced by unified structure |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: JaredforReal <[email protected]>
@JaredforReal this is cool! looking forward to open-webui integration and demo links |
Signed-off-by: JaredforReal <[email protected]>
I think it's ready to be merged now! Thanks for your time! @rootfs |
What type of PR is this?
feat: refactor observability configs for Compose and add for Local
What this PR does / why we need it:
config/
are no longer needed.docs/tutorials/observability/observability.md
.make
commands for easy access.