OTel Logging (CAN-311) - #108
Merged
Merged
Conversation
…e Redis key Fold execution metrics (cpu/gpu, timings, LLM usage, failure details) into the future's own hash instead of a separate :metrics key, so a single snapshot travels between origin and executor in both request and completion callbacks. Also fixes the completion callback firing before final metrics (finished_at/cpu_resource/gpu_resource/agent/queue_time) were written. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolves conflicts between the future:{future_id}/:metrics consolidation
and origin/main's changes by keeping the consolidated-key implementation
(and layering it on top of main's telemetry_logging.py/session_logging.py
renames, stub_generator.py updates, etc.).
Also finishes a merge origin/main had left broken: portfolio/agents/
intent_agent.py had unresolved <<<<<<< HEAD markers (SyntaxError),
global_controller.yaml had leftover duplicate/incomplete agent entries,
and advisor_agent.py had dead code referencing unset self.model_id/
self.region. Resolved all three in favor of the direct
ventis.llm.bedrock.call_bedrock pattern (matching vllm_agent.py), dropping
the newly-added but unwired shared LLMAgent stub and its policy/config
references so Bedrock calls keep flowing through per-future telemetry.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
slimmed down useless function removed useless file removed some useless comments
…talling # Conflicts: # .gitignore # tests/test_session_logging.py # tests/test_telemetry_logging.py # ventis/controller/global_controller.py # ventis/controller/utils/telemetry_logging.py # ventis/deploy.py
…ixes This branch now carries only the standalone global-controller polling bug fixes (SIGHUP config/project-id reload, cross-node identity broadcasting, Docker/Redis container-reuse-on-restart, multi-node request:completed gathering) with no telemetry-polling changes at all. Health checking and telemetry writes are back to the original single synchronous loop, same as on future-fixing -- the stalling behavior that causes is expected here and is what change/global_polling (built on top of this branch) fixes. Removed: telemetry_poller.py, its GlobalController wiring (_shutdown_event/_lifecycle_lock/_run_thread, the health/telemetry loop split, telemetry_poller.start()/stop() in run()/stop()), the telemetry-persisted cleanup gate, and the Redis ack write in send_runtime_information -- along with the tests that only existed to cover that machinery.
Builds on top of bug/global-stalling's polling bug fixes (SIGHUP reload, identity broadcasting, container-reuse, multi-node cleanup gathering). TelemetryPoller now owns its own thread and timer instead of being driven by the controller's health-check loop each cycle, so a slow Postgres write can no longer stall health polling. GlobalController's health loop and telemetry are fully independent, coordinated only through a targets_provider callable and start()/stop(). Decoupling telemetry onto its own thread reopens a race with cleanup (a request's futures could be deleted before their telemetry is persisted), so cleanup's existing multi-node request:completed gather now also waits on a small per-node Redis ack flag written once a future's row commits. See docs/TELEMETRY_POLLING.md for the full rationale and how this branch relates to bug/global-stalling.
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>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…heckpoint) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> # Conflicts: # examples/portfolio/agents/advisor_agent.py
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
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.
…nup-race fix (pre-pull checkpoint) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
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>
…lders respectively
…erator path, and OTLP logs endpoint - EC2/_runtime.py: fix malformed docker-run arg (missing -e before CANYONOS_LOGS_ENABLED=...) that broke every EC2 deploy on this branch. - Local/_runtime.py: thread CANYONOS_LOGS_ENABLED into local-provider containers (was never wired, so `logs: true` did nothing locally). - local_controller.py: fix leftover ventis_context reference (should be canyonos_context) that raised NameError in _process_request; add guardrail comments clarifying logs_enabled only gates the rich `logs` entry, never the always-on error/failed/fan-out/relay path. - log_entry.py / log_handler.py: document the Path A (deterministic failure, always-on) vs Path B (ambient DEBUG/INFO, flag-gated) boundary so the split reads as intentional, not accidental. - stub_generator.py: fix wrong source path for log_entry.py in the workflow Docker-context generator (was silently skipped, would have crashed the Workflow container at import time). - otel_exporter.py: fix logs being shipped to the traces endpoint for HTTP destinations; derive /v1/logs from a /v1/traces endpoint (or use an explicit logs_endpoint override). gRPC destinations are unaffected (already correctly signal-routed via RPC method, not URL path). - FUTURE_SCHEMA.md / DESIGN.md: document the `logs` field, its two writers, and the new logs_endpoint destination option. - Tests: regression coverage for every fix above (test_error_propagation, test_instance_manager_runtime, test_runtime_ec2, test_stub_generator, test_otel_exporter_fanout). Verified end-to-end on a real `canyonos deploy` (portfolio example, all providers local): every future's `logs` field populates correctly with ordered INFO breadcrumbs and OTel-shaped failure entries.
Takes CAN-312's OTLP exporter package (canyonos_core/otlp_exporter/, per-signal traces_waiting/metrics_waiting schema, controller/utils/schema.py + otel_writer.py, otlp_exporter/otel_reader.py, sync exporter.export(), Redis otel:destinations) as the base for the entire exporter pipeline, while preserving CAN-311's logging work: - log_convert.py and the reconciled otlp_exporter/utils/otlp_utils.py (keeping span_id_from_future and the fuller docstring) now live under the new lowercase otlp_exporter/ package; nothing imports log_convert yet, as intended. - Non-exporter controller changes (deploy.py identity lookup, stub_generator's log_handler.py/log_entry.py copying, local_controller_frontend.py's error_message handling) are preserved/combined. - telemetry_logging.py, its test, and test_global_controller_reload.py are kept despite CAN-312 deleting them; global_controller.py still calls assign_project_id() so that integration keeps working (duplicating some of otel_writer.py's cost-annotation logic -- left as-is for now). - OTLP_Exporter/db.py is kept (moved to otlp_exporter/db.py) as dead code, unreferenced by the new pipeline. - test_otel_exporter_fanout.py, test_otel_exporter_fields.py, and test_stub_generator.py rebased onto CAN-312's versions, re-adding test_log_entry_is_copied_into_the_workflow_context. - test_otlp_utils.py's otlp_utils/log_convert pure-function tests ported to the new import paths; its ProcessSignalTests (old db.py/_process_signal architecture) removed as fully superseded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Rebuilds this branch directly on CAN-312-OTel-Metrics so the logs signal
follows its per-signal conventions and the legacy Postgres telemetry path
stays removed.
- logs_waiting table (log_id = {future_id}:{index}) in controller/utils/schema.py
- log_write_rows in controller/utils/otel_writer.py, called from send_telemetry
- log_mark_sent / log_mark_sent_many in otlp_exporter/otel_reader.py
- log_row_to_log_records in otlp_exporter/log_convert.py
- _log_build_exporter(s), _log_send_pending, _log_note_queue_state in
otlp_exporter/otel_exporter.py, delivering through the same synchronous
export path as traces and metrics; logs_waiting registered for prune and flush
- LOGS.md alongside TRACES.md/METRICS.md; README.md covers three signals
- tests/test_otel_logs.py for the signal; test_otlp_utils.py reduced to
pure-function tests
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Saaketh0
marked this pull request as ready for review
September 16, 2026 19:08
Collaborator
Author
|
Note: Most of the existing diff (1.7k lines) is taken up by tests and markdown files, the actual logging logic added is like 400 lines max. |
iidsample
approved these changes
Sep 22, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ce_id/span_id Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
userAugustos
added a commit
that referenced
this pull request
Sep 22, 2026
Conflicts:
- cli.py _load_config: both kept. The root .env import and ${VAR} expansion
run first, then #116's structural checks, unchanged, on the expanded config.
- global_controller.py _write_identity: main's payload. #104 dropped
database_url from it, which removes the line the `or {}` guard protected.
The config_env delegation merged cleanly.
- test_global_controller_identity.py: main's version. The empty-database
test covered that same line.
The gate in _run_build still runs before _load_config. #116 overlaps with it:
- #116's missing-sources block in _run_build is removed. The gate already
reports a missing entrypoint or workflow_file, one line per violation.
That block also required an `entrypoint` on `type: database` services, so
on main any manifest with a database fails to build.
- The schema now rejects everything #116's _load_config checks reject, so
none of those RuntimeErrors can escape as a traceback once the gate has
passed: ports must be 1-65535, a local workflow may not have replicas > 1,
and resources are positive numbers (cpu 0.5 now passes, gpu 0 fails).
test_build_fails_when_stub_cannot_be_generated now asserts the gate's
exit 1 instead of #116's RuntimeError.
- Provider case is left strict: `LOCAL` is still rejected at the gate even
though _load_config would normalize it.
The schema declares #108's new top-level `logs` flag as a boolean, default
true, the same default both runtimes read. The portfolio example set
`env_file: .env` twice. PyYAML silently kept the second copy; the
duplicate-key check rejects it, so the second copy is removed.
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
5 participants
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.
WIP, need to complete the README and verify everything is good.
Adds OTel Logging capabilities to CanyonOS, allowing for users to see complete system logs as well as better error messages.
This would allow users to gain more useful logs to know what is happening in the system, as well as helping us debug faster and easier.
This also fixes past issues about error messages not properly being included on the UI.