Fleshed out LLM Proxy - #63
Closed
Saaketh0 wants to merge 1 commit into
Closed
Conversation
Saaketh0
commented
Sep 2, 2026
Saaketh0
commented
Collaborator
- Added telemetry for bedrock
- Added proxy for ventis
- moved folder inside ventis
- every local controller on start would start this separate process
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Saaketh0
force-pushed
the
feature/llm-proxy-telemetry
branch
from
September 2, 2026 00:09
091f29d to
6ae6d83
Compare
…cture Implements automatic LLM telemetry capture with zero agent code changes. ## Architecture - Distributed proxy: One per container, auto-starts on port 8081 - boto3 hook: Injects X-Ventis-Future-Id header from thread-local context - Token extraction: Automatic parsing of Bedrock responses - Redis persistence: All 7 metrics written to future:<id> keys ## Key Components - ventis/llm_proxy/: Complete proxy package (app, hooks, providers) - ventis/controller/local_controller.py: Auto-starts proxy subprocess - ventis/controller/utils/process_supervisor.py: Subprocess management - ventis/stub_generator.py: Copies llm_proxy with full package structure ## Metrics Captured (Bedrock only) - model: Full model ID from request path - input_token_count, output_token_count, token_count - input_cache_tokens (cache reads) - input_cache_write_tokens (cache writes) - errors: HTTP status >= 400 ## Key Fixes - Package structure: llm_proxy copied as ventis/llm_proxy/ to preserve imports - Infinite loop prevention: Proxy's boto3 client uses explicit AWS endpoint - Hooks initialization: Import hooks inside proxy_request() to get configured instance - Flask header normalization: Handle X-Ventis-Future-Id (Title-Case) - Dependencies: Added flask and requests to BASE_AGENT_REQUIREMENTS ## Agent Changes Agents use standard boto3 - zero telemetry code needed: - examples/portfolio/agents/advisor_agent.py: Removed ventis.llm imports - examples/portfolio/agents/intent_agent.py: Removed ventis.llm imports - examples/text2sql/agents/vllm_agent.py: Removed ventis.llm imports ## Removed - ventis/llm/: Old bedrock wrapper (deprecated in favor of proxy) - Planning docs: Consolidated into llm_proxy/README.md ## Testing Verified end-to-end on EC2: - LLM calls succeed through proxy - Token extraction works (inputTokens, outputTokens, cache tokens) - Redis writes confirmed with all 7 fields - Environment: boto3 + AWS_ENDPOINT_URL_BEDROCK_RUNTIME auto-routing ## Scope Bedrock-only for now. OpenAI/Anthropic use different SDKs (not boto3), would need separate hooks in their HTTP clients. Achieves complete parity with old ventis/llm/bedrock.py telemetry.
Saaketh0
force-pushed
the
feature/llm-proxy-telemetry
branch
from
September 2, 2026 00:15
6ae6d83 to
3c43b67
Compare
Collaborator
Author
|
#73 Having all of this code, closing this PR |
Saaketh0
added a commit
that referenced
this pull request
Sep 9, 2026
* includes all the files when ventis build * fixed some bugs * ventis build: sweep project .py files into Docker build contexts generate_docker()/generate_workflow_docker() now recursively sweep every .py file under the project directory into the build context, preserving directory structure, so helper files that aren't declared as an agent entrypoint still make it into the image. Generated dirs (docker_container/, stubs/, grpc_stubs/) are excluded at the project root only, not at every depth. Stub files are placed at their agent's declared entrypoint path (mapped from global_controller.yaml) instead of a hardcoded guess, so a stub overwrites the exact real file it replaces. Guards against absolute and '..'-containing entrypoints, symlinked sources, and symlinked-destination escapes, with warnings on unsafe or unmapped stubs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix missing os import in metrics_agent.py Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * WIP: OTel exporter testing + portfolio merge-conflict fix (pre-pull checkpoint) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * [Feature] Pass env / secrets into agent containers Users had no way to get API keys (OpenAI, Anthropic, embedding models) into an agent container. Add a top-level `env_file` key to global_controller.yaml pointing at a local .env file, which reaches every container as `docker run --env-file`. - resolve_env_file validates the path before anything launches, so a missing .env fails at deploy time instead of deep inside a container. Relative paths resolve against the project root, matching entrypoint. - env_file_args is a context manager owning the local-vs-remote decision and the cleanup, so both runtimes share one code path. Local containers read the original file; remote containers get a copy that is deleted as soon as `docker run` returns, whether or not it succeeded. - GlobalController._push_file streams the file over ssh under `umask 077` rather than scp, so the copy is never briefly world-readable and the secret never lands in a command line. _run_cmd's ssh options moved to a shared _ssh_args. --env-file is appended after the explicit -e VENTIS_* flags; Docker gives those precedence regardless of order, so a stray VENTIS_* line in someone's .env cannot break agent wiring. Closes #50 * Harden the remote env file copy against a hostile /tmp Two holes in the remote staging path, both found reviewing the feature commit. `umask 077` only governs files the shell creates, and `>` follows symlinks -- so it did not actually guarantee a 0600 copy. The destination path is fully predictable (`/tmp/ventis-env-ventis-ec2-<agent>-<n>`), so a local user on the remote host could pre-create it world-readable, or point it at a file of their own, and collect the API keys. Remove whatever sits at the path before writing; `rm -f` unlinks a symlink rather than following it, so `cat >` then creates a fresh file under the umask. `_run_cmd` joins its argv with spaces and hands the result to a remote shell unquoted. `_push_file` quoted its path but the cleanup `rm` did not, so a container name containing a space split the `rm` into two arguments that matched nothing -- it exited 0 while the secrets file stayed on the host, and the returncode check logged nothing. Scrub the name down to [A-Za-z0-9_.-] in remote_env_path, which also closes the same gap in the `--env-file` argument and in any future use of that path. Still open, tracked separately: a push that dies mid-transfer can leave a copy behind, since the cleanup only covers the `docker run` that follows. On EC2 the instance is terminated on that path, which disposes of it. * WIP: OTel multi-destination fan-out (Railway+Langfuse+Grafana) + cleanup-race fix (pre-pull checkpoint) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Dedupe 'import os' from PR #51 merge (both sides added it independently) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix PR #51 regression: disable entrypoint-based stub relocation This project's agents/workflow import each other's stubs by flat module name, not by the exporting agent's own entrypoint path. Applying _stub_destination's entrypoint-mirroring broke both the Workflow (ModuleNotFoundError: intent_agent) and agent-to-agent calls (MetricsAgent -> price_agent) on live redeploy. Keeps PR #51's actual fix (project_dir sweep for unstubbed helper files) intact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Parallelize per-instance polling in _poll_controllers Metrics/telemetry latency scaled with instance count x per-instance round-trip time since every instance was polled sequentially, one blocking the next, with the following tick only starting after the whole pass finished. Extracted the per-instance body into _poll_one_instance (whole body wrapped in one top-level try/except, since ThreadPoolExecutor.map() re-raises on first exception when results are consumed) and run all instances concurrently via the same ThreadPoolExecutor pattern _trigger_cleanup already used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * rough draft * rough draft * Parallelize per-instance polling in _poll_controllers Metrics/telemetry latency scaled with instance count x per-instance round-trip time since every instance was polled sequentially, one blocking the next, with the following tick only starting after the whole pass finished. Extracted the per-instance body into _poll_one_instance (whole body wrapped in one top-level try/except, since ThreadPoolExecutor.map() re-raises on first exception when results are consumed) and run all instances concurrently via the same ThreadPoolExecutor pattern _trigger_cleanup already used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * cleaned up OTel Exporter * Align with feature/otel-exporter: use updated langfuse config example and remove _write_entrypoint_file - Fixed langfuse example to use generic env-var headers pattern - Removed _write_entrypoint_file (directory structure preservation via _sweep_py_files is cleaner) * Restore parallelized polling implementation Re-applied the parallel instance polling that was lost during conflict resolution. The _poll_controllers method now uses ThreadPoolExecutor to poll all instances concurrently via _poll_one_instance, preventing one slow instance's Redis/Postgres round-trip from blocking the entire poll tick. * added concurrent polling * Redis-backed otel destination reload: config change no longer needs full redeploy Moves otel.destinations from a one-shot VENTIS_OTEL_DESTINATIONS env var (frozen at exporter subprocess spawn) to a Redis key (otel:destinations), mirroring the existing routing-table live-reload pattern. GlobalController writes it at startup and again in reload_config() (SIGHUP); otel_exporter.py's existing 5s poll tick re-reads it each cycle and rebuilds its BatchSpanProcessors only when it changed. No signal-forwarding, no subprocess restart, no ProcessSupervisor.restart -- just a small ProcessSupervisor.is_registered() so reload_config knows whether the exporter is even running. Kept in scope: exporter start-gating at boot is unchanged (still skipped entirely if otel.destinations is absent at startup); destinations added after boot only take effect if the exporter was already running. * Simplify: assume otel_exporter's Redis is always localhost:6379 Drops the redis-connection-info env plumbing (VENTIS_REDIS_HOST/PORT/DB, GlobalController._otel_exporter_env) added in the previous commit -- otel_exporter and GlobalController always run on the same host, and RedisClient's own defaults already are localhost:6379/db0, so passing them through was dead flexibility for a case that doesn't exist yet. * Trim explanatory comments off simple/obvious functions Kept comments only where behavior is genuinely non-obvious (why the exporter polls Redis instead of restarting, why reload_config gates on is_registered, the invalid-update-keeps-old-processors fallback). Dropped comments/docstrings that just narrated 'this was added' on trivial pass-through code. * Ventis fixes extracted from the CLI packaging work Everything under ventis/ and tests/ that the canyonos CLI branch depends on, lifted off feature/config-reloading with no cli/ or examples/ changes. - Package layout: move deploy/future/ventis_context/bedrock/utils under ventis/controller/, add ventis/Dockerfile and ventis/README.md. - ventis/server.py: Flask control surface the CLI's container talks to (/deploy, /clean, /status), replacing the ad-hoc entrypoint. - ventis/cli.py: fold `build` into `deploy`, support the .car artifact layout (.car/app sources, .car/config declarations, .car/stubs), and resolve env_file against the project dir so it matches how the GlobalController resolves it at runtime. - GlobalController: persist a dashed-uuid project_id and publish the controller identity to Redis. - OTLP exporter: generate Future.id at 64 bits (secrets.token_hex(8)) so it is a valid OTel span_id without truncation, and cost lookups that fail (no pricing table on a local deploy) now cost at 0 instead of dropping the whole telemetry row. - stub_generator: a stub is written to exactly one location, the path of the entrypoint it replaces, rather than being duplicated at the flat basename as well. Flat is only the fallback for a stub with no entrypoint mapping or one whose mapping escapes the build context. Carries over the placement half of 692d17c from feature/all-the-files, which never reached this line; the entrypoint-adjacent YAML discovery from that commit is deliberately left out, since the .car layout already resolves declarations from .car/config. - stub_generator: fail loudly when an agent has no declaration or entrypoint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Point example workflow imports at the stub's entrypoint path Stub placement is now mirrored-only, so a workflow importing the flat basename no longer resolves -- the stub is written to agents/<name>.py and nothing is left at the context root. Switch the four example workflows to the nested form. Cherry-picked from 7f925ef on fix/remove-duplicate-stub. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Route local-provider agents over a dedicated docker network Cherry-picked from a0efb5a on bug/local-provider-routing, resolved against the .car/CLI packaging changes already on this branch. Agents used to be reached at host.docker.internal:<host_port>, which only works when whatever is doing the reaching shares the host's network namespace. They now join a `ventis-local` docker network and are addressed by container name at the fixed container port, so routing no longer depends on the caller's vantage point. - Local/_runtime.py: --network ventis-local instead of --add-host, VENTIS_AGENT_PORT is the container port, VENTIS_AGENT_HOST is the container name, routing_endpoint_for returns <runtime_id>:50051. - global_controller.py: creates the network alongside the local Redis container, and derives status/metrics Redis keys from instance_manager._routing_endpoint_for instead of the host string. Conflict resolution notes: - Kept this branch's MAX_PORT_ATTEMPTS retry loop and env_file support around the docker run, applying the new network/env flags inside it. - Kept _is_local_host in Local/_runtime.py; a0efb5a dropped it, but env_file_args (added later on the CLI line) still needs it. - The controller:<endpoint>:agent_id key written after launch, which postdates a0efb5a, now uses the container-name endpoint so it matches the keys global_controller reads back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add the canyonos CLI package, porting skill, and .car examples The CLI layer on top of the ventis fixes: `canyonos` wraps the Global Controller in a container and drives it over HTTP, so a project goes from source to a running workflow with a local dashboard in one command. - cli/: the canyonos package -- deploy (which folds in init, sync, build and launch, then auto-starts the dashboard once the workflow reports up), serve, stop, logs, quit, clean, config, integrate, new-app. - cli/canyonos/dashboard.compose.yml: api/db/web stack. The api port is published so the GC container can POST OTLP spans to /v1/traces, which is also what renames ventis' `project_id` attribute to the `canyon.project.id` the dashboard queries filter on. serve replaces the api container every run, since it reads the controller's Redis identity only at startup and would otherwise keep serving a stale project. - cli/canyonos/constants.py: resolve the config path per call rather than at import, preferring .car/config over the flat layout. - .claude/skills/porting-to-canyonos-core/: the porting skill `canyonos integrate` installs, plus its validator. - examples/: joke_writer converted to the .car layout, epigenomics added, and workflow imports pointed at each agent's entrypoint path to match single-location stub placement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * removed some useless code * removed more useless code * cli * cleanup * Integrate LLM proxy telemetry, replacing bedrock.py Re-apply the feature/llm-proxy-telemetry proxy onto the current CLI packaging architecture (rather than merging the stale branch): - Add ventis/llm_proxy/ package (imports remapped to current layout, with in-container flat fallbacks for redis_client/ventis_context) - Delete ventis/controller/bedrock.py; rewrite the 3 examples to call boto3 bedrock-runtime converse() directly (telemetry now via proxy) - Start the proxy per-container in LocalController and auto-inject the X-Ventis-Future-ID boto3 header - Inject AWS_ENDPOINT_URL_BEDROCK_RUNTIME in Local/EC2 runtimes - Copy llm_proxy into agent+workflow images via stub_generator; add flask/requests to agent image + host deps - Update FUTURE_SCHEMA.md provenance and instance-manager runtime tests * Fix boto3 header injection: use before-call params['headers'] not request before-call handlers receive the prepared-request params dict, not the request object (that only exists on before-send). Reading kwargs['request'] was always None, so X-Ventis-Future-ID was never attached and the proxy could not attribute token telemetry to the executing future. * docs: add cli/ARCHITECTURE.md and link it from cli/README.md Bring the CLI architecture doc (build/deploy/config flows and container lifecycle) over from cli/canyonos-cli, and drop the stale PyPi republish snippet from the README. * reviewed branch and code, made small changes --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Nick Huo <jiajun.h@canyoncode.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.