feat: capture system prompt, tools and reasoning in OTel; fail loud on setup - #174
Merged
Conversation
…n setup Three gaps kept a run's LLM behaviour out of Cloud Trace. 1. System prompt and tool definitions were never recorded. Only ADK's experimental GenAI semconv path emits gen_ai.system_instructions and gen_ai.tool_definitions; the stable path records what the model said but never what it was told or what tools it had. setup_otel() now opts in via OTEL_SEMCONV_STABILITY_OPT_IN, appending to the shared CSV rather than replacing it, and leaving an explicit "stable" alone. 2. Reasoning was never recorded because it was never produced. Gemini only returns a thought summary when include_thoughts is set, so there was nothing for OTel to capture. Both agents now pass thinking_config. Turning that on changes the shape of every response: thought summaries arrive as extra text parts before the answer. main_agent took the first non-empty text part and feedback_agent took parts[0].text, so both would have parsed a thought summary as the answer - failing every session, and burning all three feedback attempts into EVALUATION_FAILED. Both now skip parts via is_thought_part(), which tests `is True` rather than truthiness so an attribute-generating test double cannot mask the answer. 3. Setup failure was silent. setup_otel() caught every exception, warned, and let the run continue with no exporters. Worse, get_gcp_exporters() returns empty hooks (no exception) when it cannot resolve the GCP project, so that path threw nothing at all and dropped every span for the whole process. setup_otel() now raises TelemetrySetupError, verifies a real SDK TracerProvider was actually installed, returns the project ID, and registers an atexit flush so a crash does not lose the buffered tail. The CLI reports and exits 4, and prints the project traces land in - the wrong project looks identical to no telemetry at all. Principle IV is unaffected: it governs per-event emission, and TelemetryLogger.emit still falls back to stderr. Setup is a preflight check, like config validation, which already exits 1. 635 tests, 100% coverage, ruff clean.
… in tests CI caught two things the local run could not. 1. Ordering was wrong. The telemetry preflight ran before config validation, so `alphoryn run` against a config.json containing "not json" reported "Telemetry error: could not resolve Google credentials" and exited 4. Config validation is local, cheap and deterministic; the preflight needs network and credentials. Config now wins, and exit 1 is restored for the two contract tests that assert it. Startup steps renumbered accordingly (this also fixes a pre-existing duplicate "# 6"). 2. The suite depended on ambient credentials. Contract and integration tests drive the real `run` command and stub every network call, but setup_otel() was not one of them because it could not fail before. With ADC present it passed; in CI, 11 contract tests and then 9 integration tests failed on a DefaultCredentialsError. Both modules now carry an autouse fixture that stubs the preflight, so they test CLI surface rather than the machine. Verified both ways this time: `GOOGLE_APPLICATION_CREDENTIALS=/nonexistent` reproduces the CI failure locally, and the suite is green with and without it. New contract tests: exit 4 when telemetry is unavailable (and the scheduler is never reached), and config errors winning over telemetry errors. 637 tests, 100% coverage, ruff clean.
This was referenced Aug 11, 2026
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
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.
Why
OpenTelemetry was already wired up and exporting to Cloud Trace and Cloud Logging, but three things about a run's LLM behaviour never reached it: the system prompt, the tool definitions the model was given, and its reasoning. On top of that, a setup failure was silent, so a run could execute for hours and leave no trace at all.
What changed
1. System prompt and tool definitions are now recorded
Only ADK's experimental GenAI semantic conventions emit
gen_ai.system_instructionsandgen_ai.tool_definitions. The stable path records what the model said but never what it was told, or what tools it could have reached for - so a trace could not answer "why did it have that option".setup_otel()opts in viaOTEL_SEMCONV_STABILITY_OPT_IN. The variable is a CSV shared with other instrumentations, so the opt-in is appended rather than assigned, and an explicitstablefrom an operator is left alone.2. Reasoning is now recorded, because it is now produced
Gemini 2.5 reasons whether or not you ask, but only returns the thought summary when
include_thoughtsis set. Nothing was being dropped - there was nothing to drop. Both agents now passthinking_config(alphoryn/agents/thinking.py).This is the part that would have broken production. Thought summaries arrive as extra text parts before the answer, flagged
thought=True:main_agent.pytook the first non-empty text partfeedback_agent.pytookparts[0].textBoth would have started parsing a thought summary as the JSON answer. That fails every trading session, and for feedback it burns all three retries and files a perfectly good evaluation as
EVALUATION_FAILED- the exact failure mode FR-016a exists to prevent.Both now route through
is_thought_part(). It testsis Truerather than truthiness on purpose:getattr(MagicMock(), "thought", False)returns a truthy child, so a truthiness test would report every part as a thought and silently swallow the model's answer under test. That was not hypothetical - it happened while writing this, and 34 tests caught it.3. Setup failure is now loud (exit code 4)
setup_otel()caught every exception, logged a warning, and let the run continue with no exporters. Three ways a whole run went untraced:google.auth.default()raisesget_gcp_exporters()returns empty hooks and raises nothing, so not even a warning firedBatchSpanProcessorwas lostNow:
setup_otel()raisesTelemetrySetupError, verifies a real SDKTracerProviderwas actually installed (the global default is aProxyTracerProvider, which is what the silent path leaves behind), returns the resolved project ID, and registers anatexitflush.The CLI reports the error and exits 4, and on success prints
Telemetry -> GCP project '<id>'. Telemetry landing in the wrong project looks identical to telemetry landing nowhere, and this repo's secrets live inalphorynwhile gcloud's default on at least one dev box iswortcast.On constitution Principle IV: this does not contradict it. Principle IV governs per-event emission at run time, and
TelemetryLogger.emitstill falls back to stderr and never blocks. Setup is a preflight check - the same class of thing as config validation, which already exits 1.Regression tests that fail on
maintest_decide_skips_a_thought_part_and_parses_the_answertest_evaluate_skips_a_thought_part_and_parses_the_answertest_evaluate_treats_a_thought_only_response_as_no_answertest_setup_otel_raises_when_credentials_carry_no_project_idtest_setup_otel_raises_when_no_tracer_provider_was_installedtest_run_exits_4_when_telemetry_cannot_be_set_upVerification
pytest: 635 passed, 100% coverageruff check alphoryn/ tests/: cleanNot covered by this PR
No live run has exercised any of it - every test stubs the LLM. The real check is one
alphoryn runduring market hours, confirminggen_ai.system_instructionsand thought parts appear in Cloud Trace forprojects/alphoryn. That remains the standing top item onHANDOFF.md.Docs
TELEMETRY_ACCESS.md: new section on what is captured, the thought-part trap, and the exit-4 behaviourspecs/001-etf-paper-trading-agent/contracts/cli.md: exit code 4 added to the table