Skip to content

feat(trace): add agent observability with Monocle - #4024

Merged
WillemJiang merged 17 commits into
bytedance:mainfrom
imohammedansari:monocle-instrumentation
Jul 14, 2026
Merged

feat(trace): add agent observability with Monocle#4024
WillemJiang merged 17 commits into
bytedance:mainfrom
imohammedansari:monocle-instrumentation

Conversation

@imohammedansari

@imohammedansari imohammedansari commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds Monocle observability to the agent. Monocle is an OpenTelemetry-based tracer for LLM applications. With it enabled, each run is recorded as a structured trace: the agent and graph invocations, tool calls, LLM inferences, token usage, and timings. The change is additive, off by default, and does not alter application logic.

What this adds

  • MONOCLE_TRACING env flag (default off), configured like the LangSmith/Langfuse flags via MonocleTracingConfig. MONOCLE_EXPORTERS picks the exporters (default file), validated against the known names; okahu requires OKAHU_API_KEY.
  • Setup runs once from the Gateway lifespan (app/gateway/app.py), never at import time, and cannot break startup. It warns when a non-file exporter is about to send trace data off the box.
  • monocle_apptrace is an optional extra (deerflow-harness[monocle], re-exposed as deer-flow[monocle]), so a default install pulls nothing new; it is pinned in the dev group for the tests.
  • Tests in backend/tests/test_monocle_tracing.py: default-off, toggle-on, exporter validation, the off-box warning, idempotent double-invoke, verified Langfuse coexistence, the Gateway-lifespan wiring, and a regression that importing deerflow.agents never starts tracing.

The setup call auto-instruments the frameworks already in use (LangGraph and the LLM clients), so there is no per-tool or per-call wiring to maintain. Traces are written to .monocle/ (git-ignored) by default. Only Gateway-served runs are traced; the embedded client and TUI are not (documented in AGENTS.md).

What you get

Each run produces a trace of all the agents and tools that were triggered: which agents ran, which tools were called, and what LLM inferences happened. In effect, it's the path the agent took to answer a given question. This is useful for developers building the agent, since they can see how it actually behaved on a run. The same traces are also a good basis for a behavioral test suite, an integration test that asserts on that behavior, and I've opened a companion PR that shows how that works: behavioral test suite. You can open the trace files directly, view them in the Monocle VS Code extension, or send them to Okahu for analysis across many runs.


Example trace (Okahu VS Code Extension)

DeerFlow's agent driving the model, with its tools (web_search, web_fetch, task, write_file) captured as trace spans.

image

Enable Monocle (OpenTelemetry tracing for LLM apps) with one setup call plus the monocle_apptrace dependency. setup_monocle_telemetry auto-instruments the frameworks already in use and writes traces to .monocle/. Additive; no changes to application logic.
@github-actions github-actions Bot added area:agents Agents, subagents, graph wiring, prompts, langgraph.json area:deps Dependency manifests / lockfiles needs-validation Touches front/back contract surface; needs real-path validation risk:high High risk: backend API, agents, sandbox, auth, deps, CI size/XS PR changes < 20 lines labels Jul 9, 2026
@CLAassistant

CLAassistant commented Jul 9, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@willem-bd willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing the Monocle observability integration (+5/-0). The feature itself is useful and the library (Apache-2.0, LF AI & Data) is reasonable, but the integration needs rework before merge — see inline comments.

Headline: setup_monocle_telemetry(...) is invoked at module import time in deerflow/agents/__init__.py. Verified from the monocle_apptrace 0.8.8 source (instrumentation/common/instrumentor.py) that this call installs the process-global OTel TracerProvider, monkey-patches ReadableSpan.to_json globally, and auto-instruments the openai/langchain/langgraph clients — all on import deerflow.agents.

Recommendation: request changes. Concretely:

  1. Move setup out of import time -> gateway lifespan (backend/app/gateway/app.py:169), gated by config (add a monocle provider to the existing TracingConfig / build_tracing_callbacks, or an env flag). Default off.
  2. Add .monocle/ to .gitignore.
  3. Pin the dependency (monocle_apptrace>=0.8.8).
  4. Guard the single-global-OTel-provider conflict with Langfuse/LangSmith.
  5. Add tests (default-off, toggle-on) — per AGENTS.md, backend features ship with tests.
  6. Document the data the file exporter captures and the MONOCLE_EXPORTERS exfil path in config.example.yaml.

What's good: additive, no app logic changed; check_duplicate_setup makes repeated calls idempotent; useful observability output.

Note (not tied to a specific line): no tests are added — AGENTS.md mandates TDD in backend/. A test asserting telemetry is not initialized on a plain import deerflow.agents (default off) and toggles on with the config flag would cover this.


from monocle_apptrace import setup_monocle_telemetry

setup_monocle_telemetry(workflow_name="deer-flow", monocle_exporters_list="file")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call runs at module import time and does heavy global mutation. Verified against monocle_apptrace 0.8.8 (instrumentation/common/instrumentor.py): setup_monocle_telemetry calls set_tracer_provider(TracerProvider(...)) (the process-global OTel provider — only one is allowed, first call wins), setup_readablespan_patch() (monkey-patches ReadableSpan.to_json for all OTel spans, not just Monocle's), and MonocleInstrumentor().instrument() (wraps the openai/langchain/langgraph clients).

Problems with placing it here:

  • Violates this package's own design. The __getattr__ below (see comment at lines 28-31) deliberately defers work away from import time "so lightweight submodules can be imported without pulling in the whole tool/subagent graph." A top-level setup_monocle_telemetry() reverses that.
  • Runs in every non-app context: the test suite, the TUI, langgraph CLI graph registration, scripts, MCP servers. Monocle's own README says to call it "in your main() function."
  • Conflicts with the existing tracing stack. DeerFlow already has a config-gated deerflow/tracing/ (build_tracing_callbacks(), config/tracing_config.py for LangSmith/Langfuse) wired at the four graph-invocation roots (client.py:734, lead_agent/agent.py:462, models/factory.py:291, subagents/executor.py:618). The global OTel provider is set-once, so if Langfuse (>=4, OTel-based) is enabled, whichever of Monocle (import) or Langfuse (first run) runs first wins and the other is silently dropped ("Overriding of current TracerProvider is not allowed"). The global to_json patch and double client instrumentation compound this.
  • No opt-out. Repo convention (config.example.yaml:27): tracing is disabled by default. This is always on.
  • Test-suite impact. Importing deerflow.agents now does real disk I/O (file exporter) + spawns a BatchSpanProcessor thread, which trips the repo's Blockbuster blocking-IO gate, and patches the OpenAI client that test doubles rely on.

Fix: move this to the gateway lifespan (backend/app/gateway/app.py:169), gated behind config (default off), and short-circuit when another tracing provider is active.


from monocle_apptrace import setup_monocle_telemetry

setup_monocle_telemetry(workflow_name="deer-flow", monocle_exporters_list="file")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

monocle_exporters_list="file" writes one JSON trace per run to .monocle/ (confirmed: exporters/file_exporter.py, DEFAULT_TRACE_FOLDER = ".monocle", open(path, "w"), no rotation/size cap).

  • .monocle/ is not gitignoredgit check-ignore .monocle/ returns nothing. Traces contain full prompts, completions, and tool inputs/outputs, so secrets/PII that flow through them could get committed.
  • Unbounded disk growth — one file per trace, no cleanup.
  • Exfiltration surface: setup_monocle_telemetry honors the MONOCLE_EXPORTERS env var, which overrides "file" to otlp/okahu/s3 (destination via OTEL_EXPORTER_OTLP_ENDPOINT). Because setup is always-on at import with no opt-in, a single env var silently ships every prompt/completion/tool-I/O to a third party (Okahu). For a system bridging IM channels (Feishu/Slack/Telegram/DingTalk) and running sandboxed tools, that's a real data-handling risk (the risk:high label fits).

Add .monocle/ to .gitignore at minimum, and document the captured data + the MONOCLE_EXPORTERS override in config.example.yaml.

Comment thread backend/pyproject.toml Outdated
"pyjwt>=2.13.0",
"email-validator>=2.0.0",
"e2b-code-interpreter>=2.8.1",
"monocle_apptrace",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unpinned. Every other dependency in this file uses a >= floor. monocle_apptrace is 0.x (0.8.8, 47 releases) — pre-1.0, so breaking changes can land in a minor bump and silently break installs/behavior. Add a floor (e.g. monocle_apptrace>=0.8.8) and account for its transitive OpenTelemetry footprint.

Addresses review on bytedance#4024: moves setup_monocle_telemetry out of agents/__init__ import time into the Gateway lifespan, gated by MonocleTracingConfig (MONOCLE_TRACING env, default off). Warns on the Langfuse/global-OTel-provider conflict and relies on monocle_apptrace's own duplicate-setup guard and existing-provider attach. Pins monocle_apptrace>=0.8.8 (+ uv.lock), adds .monocle/ to .gitignore, adds tests (default-off / toggle-on / no import-time setup), and documents exporters, Okahu, and the VS Code viewer in README, config.example.yaml, and backend/AGENTS.md.
@github-actions github-actions Bot added area:backend Gateway / runtime / core backend under backend/ area:docs Documentation and Markdown only size/M PR changes 100-300 lines and removed area:agents Agents, subagents, graph wiring, prompts, langgraph.json size/XS PR changes < 20 lines labels Jul 10, 2026
Make the docstring, warning, and AGENTS.md consistent with the README: only one library can own the global OpenTelemetry provider; Monocle initializes at startup before Langfuse's per-run handler, so enabling both drops Langfuse's spans — enable one OTel tracer (LangSmith, a callback, coexists fine).
@imohammedansari

Copy link
Copy Markdown
Contributor Author

@willem-bd Thanks for the detailed review 🙏 I've pushed changes covering all six points.

setup_monocle_telemetry is no longer called at import in agents/init.py. It now runs from the Gateway lifespan behind MonocleTracingConfig (MONOCLE_TRACING), and it's off by default, so importing deerflow.agents doesn't start tracing anymore, and there's a test that locks that in. I added .monocle/ to .gitignore, pinned monocle_apptrace>=0.8.8, and refreshed uv.lock, which fixes the lint-backend failure. For the single global provider concern, it now logs a warning when Langfuse is also active, since a process can only run one OpenTelemetry provider and enabling both would drop one side's traces. LangSmith is a callback so it still works alongside Monocle. Tests are in tests/test_monocle_tracing.py and cover default-off, toggle-on, and the no-import-time case. config.example.yaml now documents what the file exporter captures and the MONOCLE_EXPORTERS export path, and I updated the README and backend/AGENTS.md too.

Ready for another look 🚀

@willem-bd willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the Monocle observability integration. The architecture is sound and conservatively gated — Monocle is correctly kept out of the per-run callback path (enabled_providers still lists only langsmith/langfuse, so build_tracing_callbacks never touches it), setup was moved out of import time into the lifespan and can't break startup, and the Langfuse/Monocle global-OTel conflict is the right thing to warn about.

Findings below focus on the two things that affect users who never asked for Monocle — the hard dependency and secret/PII capture at the SDK layer — plus an exporter-validation gap. Nothing here is a blocker to the runtime logic itself.

Comment thread backend/pyproject.toml Outdated
"pyjwt>=2.13.0",
"email-validator>=2.0.0",
"e2b-code-interpreter>=2.8.1",
"monocle_apptrace>=0.8.8",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an optional extra, not a hard runtime dependency. Monocle is off by default, yet monocle_apptrace>=0.8.8 is in core dependencies, so every install pulls the full OTel stack (opentelemetry-api/-sdk/-exporter-otlp-proto-http/-instrumentation, wrapt, rfc3986, requests) whether tracing is used or not. The repo already has precedent: boxlite and tui are [project.optional-dependencies]. The lazy from monocle_apptrace import ... inside setup_monocle_tracing_if_enabled means converting to an optional extra is low-risk. Also worth weighing: pinning a pre-1.0 (0.8.x) vendor-affiliated auto-instrumentation library as a core dep — breaking releases are possible before 1.0.

exporters = get_tracing_config().monocle.exporters
from monocle_apptrace import setup_monocle_telemetry

setup_monocle_telemetry(workflow_name="deer-flow", monocle_exporters_list=exporters)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: spans capture prompts + tool I/O verbatim, with no DeerFlow-controlled scrubbing layer. Auto-instrumentation records span inputs/outputs — prompts, tool arguments (write_file body, bash command strings), and model responses. DeerFlow's existing tracing (tracing/metadata.py "never copies context") is careful about what it sends; Monocle captures at the SDK layer, which DeerFlow can't scrub. Request-scoped secrets are safe from bash args (they go through env=), but write_file/read_file/web_fetch I/O carries sensitive data and is captured unscrubbed.

Two suggestions: (1) log a loud startup warning whenever a non-file exporter is configured (e.g. okahu ships all of this off-box to an external collector — operators shouldn't be able to miss that); (2) document which DeerFlow data surfaces are/aren't captured. file-by-default + the config.example.yaml disclosure is a good baseline.


def validate(self) -> None:
# No external credentials are required; the "file" exporter writes locally.
return None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MONOCLE_EXPORTERS is unvalidated, and this validate() is dead code. TracingConfig.validate_enabled() only calls langsmith.validate() / langfuse.validate() — it never calls self.monocle.validate() — so this no-op never runs. Concretely: (a) a typo in MONOCLE_EXPORTERS (e.g. fle) is passed straight to setup_monocle_telemetry and fails inside monocle at runtime with an obscure error; (b) MONOCLE_EXPORTERS=okahu without OKAHU_API_KEY isn't caught early, unlike Langfuse which validates required settings and names them.

Recommend an exporter allowlist (file/console/okahu/s3/blob/gcs) + require OKAHU_API_KEY when okahu is selected, mirroring the Langfuse pattern — and actually wire self.monocle.validate() into validate_enabled() (or drop this method).

Comment thread backend/app/gateway/app.py
# startup), before Langfuse's per-run handler, so enabling both means Langfuse loses
# its spans — warn so the operator turns one off.
if "langfuse" in get_enabled_tracing_providers():
logger.warning("MONOCLE_TRACING is enabled alongside Langfuse; both need the global OpenTelemetry provider and only one can win. Enable only one of them.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Langfuse-conflict warning is the key logic branch in the module and has no test. Add a caplog test asserting the warning fires when MONOCLE_TRACING and Langfuse are both enabled (and stays silent when only one is). It's the one piece of real branching here and it's currently uncovered.

Comment thread config.example.yaml Outdated
# per-agent/LLM/tool spans, including span inputs and outputs (prompts, tool
# arguments, and model responses), token usage, and timings.
#
# Data-exfiltration note: `file` persists prompts and model outputs to local

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this block sits under the logging: section, but Monocle is telemetry/tracing, not logging. Consider placing it under a dedicated tracing header for discoverability. (Separately — the data-exfiltration disclosure here is well done; the actionable ask is in monocle.py: a startup warning when a non-file exporter sends data off-box.)

Comment thread backend/tests/test_monocle_tracing.py Outdated
"""
import deerflow.agents as agents

assert not hasattr(agents, "setup_monocle_telemetry")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regression check could be stronger, and idempotency is untested. not hasattr(agents, "setup_monocle_telemetry") only proves the name isn't on the agents namespace — it doesn't prove a global OTel TracerProvider wasn't installed. A more rigorous regression would assert the global provider wasn't replaced. Separately, the wrapper's idempotency claim (the docstring cites upstream check_duplicate_setup) is trusted, not exercised — consider a double-invoke test so a future caller can't silently double-instrument.

… tests

Responds to the second review round.

- Make monocle_apptrace an optional extra (deerflow-harness[monocle], re-exposed
  as deer-flow[monocle]) following the boxlite/tui precedent, so a default
  install no longer pulls the OpenTelemetry stack. It stays pinned in the dev
  group for the tracing tests, and enabling MONOCLE_TRACING without the extra
  raises a clear install error.
- Warn loudly at startup whenever any exporter other than `file` is configured,
  since those move prompts, tool inputs/outputs, and completions beyond the
  local .monocle/ directory.
- Validate MONOCLE_EXPORTERS against the known exporter names and require
  OKAHU_API_KEY when okahu is selected, mirroring the Langfuse pattern.
  Validation runs from Monocle's own init (not validate_enabled) so a config
  typo can never fail agent runs; errors surface at Gateway startup instead.
- Grow the tests from 5 to 13: caplog coverage for the Langfuse-conflict and
  off-box warnings, exporter validation cases, a stronger import-time
  regression that asserts the global TracerProvider is not replaced, and a
  subprocess double-invoke test exercising the real check_duplicate_setup.
- Docs: config.example.yaml block retitled to a dedicated tracing header;
  README documents the [monocle] install and scopes tracing to Gateway runs.
@github-actions github-actions Bot added size/L PR changes 300-700 lines and removed size/M PR changes 100-300 lines labels Jul 11, 2026
@imohammedansari

imohammedansari commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@willem-bd Thanks for the second pass, everything you flagged is in the latest commit.

monocle_apptrace moved out of core deps into an optional extra (deerflow-harness[monocle], re-exposed as deer-flow[monocle], same pattern as boxlite/tui), so default installs don't pull the OTel stack anymore. It's still pinned in the dev group so the tests can import it, and turning on MONOCLE_TRACING without the extra installed gets you a RuntimeError that tells you what to pip install.

Setup now warns whenever anything other than the file exporter is configured, since those move prompts and tool I/O beyond the local trace file. Exporters are validated against the known names and okahu requires OKAHU_API_KEY, same shape as the Langfuse validation. One thing I did differently from your suggestion: validate() is called from Monocle's own setup instead of validate_enabled(), because the only caller of validate_enabled() is build_tracing_callbacks(), and as you noted Monocle is deliberately not in that path. This way a typo in MONOCLE_EXPORTERS surfaces as a loud startup error and tracing stays off, but it can't break Gateway startup or an agent run.

Tests went from 5 to 13. The new ones cover the Langfuse conflict warning, the off-box warning, and the validation errors, plus the two you called out: the import-time regression now checks the global TracerProvider actually isn't replaced (not just hasattr), and there's a double-invoke test that exercises the real check_duplicate_setup in a subprocess so the global provider doesn't leak into the rest of the suite. Also retitled the config.example.yaml block to its own tracing header and noted in the README that only Gateway runs get traced.

Lead with what Monocle is and captures, drop the install step (the dev
group already ships monocle_apptrace via uv sync; unusual installs get
the RuntimeError), and point the missing-package error at the repo-native
command (uv sync --extra monocle / deerflow-harness[monocle]).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@imohammedansari
imohammedansari force-pushed the monocle-instrumentation branch from d499600 to c2910f2 Compare July 11, 2026 05:16

@willem-bd willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted findings inline. Overall the implementation is clean, off-by-default, startup-isolated, and well-tested for the surface it claims. The substantive concerns are (1) the Langfuse/OTel-conflict rationale being asserted but unverified, and (2) the embedded/TUI path being silently uninstrumented vs LangSmith/Langfuse.

Notes not tied to a specific line:

  • PR body is stale: it says setup lives in agents/__init__.py, but the diff moved it to the Gateway lifespan (AGENTS.md documents the move as deliberate). Worth updating so "what this adds" matches the diff.
  • Minor: monocle_exporters_list=exporters passes a comma-separated string, not a list — fine (it's monocle_apptrace's API) but a comment would help. test_monocle_tracing.py hard-imports opentelemetry/monocle_apptrace; a module-level pytest.importorskip("monocle_apptrace") would let the suite run cleanly in minimal installs, matching how other optional-extra tests in the repo behave.
  • Governance: this adds a vendor-specific tracer (Okahu/Monocle) as a first-class peer to LangSmith/Langfuse, and the PR body promotes the Okahu extension/platform and asks for a ⭐. Code quality is good; the open question for maintainers is whether a named-vendor integration is in scope vs. a generic "configure any OTel exporter" knob. The optional-extra packaging at least keeps it off default installs.

Recommendation: request changes — primarily to resolve the Langfuse-conflict claim (verify or drop the warning + README statement) and document the embedded-path gap.

# startup), before Langfuse's per-run handler, so enabling both means Langfuse loses
# its spans — warn so the operator turns one off.
if "langfuse" in get_enabled_tracing_providers():
logger.warning("MONOCLE_TRACING is enabled alongside Langfuse; both need the global OpenTelemetry provider and only one can win. Enable only one of them.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: the Langfuse/OTel conflict is asserted but not verified.

In this repo, Langfuse is wired as a LangChain CallbackHandler (tracing/factory.py::_create_langfuse_handlerfrom langfuse.langchain import CallbackHandler as LangfuseCallbackHandler), attached at the graph root — DeerFlow never calls langfuse.start_tracing() or trace.set_tracer_provider() for it. So the claim "both need the global OpenTelemetry provider and only one can win" isn't obviously true for this codebase's Langfuse integration.

Two concerns:

  • If the v4 callback doesn't actually contend for the global OTel provider, this warning is noise and the README's "not Langfuse" rule is misleading (Monocle would coexist with Langfuse just as it does with LangSmith).
  • test_warns_when_langfuse_also_enabled only asserts the warning is logged, not that spans are actually lost — so the conflict is claimed in prose/tests but never demonstrated.

Suggestion: verify against the installed langfuse version whether its callback installs/owns the global OTel provider at per-run time. If yes, add a test proving Monocle's provider survives (or Langfuse drops); if no, drop the warning and correct README/AGENTS.md.

Comment thread README.md Outdated

#### Using Multiple Providers

LangSmith and Langfuse attach as LangChain callbacks, so you can enable both and DeerFlow reports each run to both. If an enabled provider is missing required credentials or fails to initialize, DeerFlow fails fast and names it. Monocle uses a global OpenTelemetry provider rather than a callback, so it runs alongside LangSmith but not Langfuse.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User-facing version of the same Langfuse-conflict claim (see inline on monocle.py).

"...so it runs alongside LangSmith but not Langfuse" is stated as a flat rule, but rests on the same unverified assumption that Langfuse's callback path owns the global OTel provider. Worth softening ("may conflict; verify") or removing until the interaction is confirmed — otherwise Langfuse users are told they can't use Monocle when they likely can.

# MONOCLE_TRACING. Initialized here at startup — not at import time — so a
# plain `import deerflow.agents` never installs a process-global tracer.
try:
setup_monocle_tracing_if_enabled()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Embedded / TUI / CLI path gets no Monocle tracing.

LangSmith and Langfuse are attached at the graph invocation root for both runtime/runs/worker.py::run_agent (this gateway path) and client.py::DeerFlowClient.stream (embedded). Monocle is initialized only here in the Gateway lifespan, so DeerFlowClient (programmatic library use) and the deerflow TUI get no traces even with MONOCLE_TRACING=true (confirmed: no monocle wiring under client.py or deerflow/tui/).

That's a real asymmetry vs the other two providers — at minimum call it out in AGENTS.md (embedded/TUI users must call setup_monocle_tracing_if_enabled() themselves, or accept that only the Gateway is instrumented).

Also: this lifespan wiring — the one line that makes the feature work — has no test asserting it's invoked during startup. A focused test monkeypatching the helper and asserting it's called from the lifespan would close that gap.


@property
def is_configured(self) -> bool:
return self.enabled

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_configured diverges from the LangSmith/Langfuse pattern.

LangSmith/Langfuse fold credential presence into is_configured (self.enabled and bool(self.api_key)); here it's just self.enabled, so is_monocle_tracing_enabled() returns True even for a config validate() will reject (e.g. okahu without OKAHU_API_KEY). Validation is deferred to setup, where it's caught+logged rather than surfaced as "not enabled."

Defensible as a composite-validation choice, but a one-line comment explaining why Monocle's is_configured is intentionally coarser would stop a future reader from "fixing" it to match the others.

Responds to the third review round.

The Langfuse conflict claim was wrong, verified empirically against langfuse
4.5.1 in both init orders: whichever library initializes second reuses the
existing global TracerProvider and attaches its own span processor, so neither
side loses spans. Dropped the warning and its tests, corrected the README,
AGENTS.md, and config.example.yaml statements, and pinned the verified behavior
with test_coexists_with_langfuse (real monocle + real langfuse in a subprocess,
no mocks). One honest caveat documented: both processors see all spans, so
Monocle's exporters also capture Langfuse's spans when both are enabled.

Also from the review:
- Document the Gateway-only scope in AGENTS.md: the lifespan is the sole call
  site, so the embedded DeerFlowClient and TUI are not instrumented; embedded
  users call setup_monocle_tracing_if_enabled() themselves.
- Add test_gateway_lifespan_initializes_monocle pinning the lifespan wiring.
- Comment why MonocleTracingConfig.is_configured is intentionally coarser than
  LangSmith/Langfuse (composite validation lives in validate() at startup).
- Note that monocle_exporters_list takes the comma-separated string as-is.
- Module-level importorskip("monocle_apptrace") so minimal installs collect
  the test module cleanly.
@imohammedansari

Copy link
Copy Markdown
Contributor Author

@willem-bd You were right about the Langfuse conflict 🙏 I tested it against langfuse 4.5.1 in both init orders and there is no contention: whichever library initializes second reuses the existing global TracerProvider and attaches its own span processor, so nothing gets dropped. Removed the warning, corrected the README, AGENTS.md, and config.example.yaml, and pinned the actual behavior with test_coexists_with_langfuse, which runs real monocle and real langfuse in a subprocess with no mocks. One caveat is now documented: both processors see all spans, so Monocle's exporters also capture Langfuse's spans when both are on.

The rest is in too. AGENTS.md now calls out that the Gateway lifespan is the sole call site, so the embedded client and TUI are not instrumented unless they call setup_monocle_tracing_if_enabled() themselves, and there is a new test asserting the lifespan actually invokes the setup. Also added the is_configured comment, the comma-separated string note, importorskip for minimal installs, refreshed the PR body, and merged main. Ready for another look 🚀

@WillemJiang WillemJiang changed the title Add agent observability with Monocle feat(trace): add agent observability with Monocle Jul 11, 2026
WillemJiang and others added 3 commits July 11, 2026 23:30
test_no_import_time_setup deleted deerflow.agents* from sys.modules and
re-imported to force __init__ to re-execute. The re-import creates new module
objects, and restoring the old sys.modules entries afterwards leaves the parent
package's attribute bindings pointing at the new ones, so any later test that
resolves a deerflow.agents.* dotted path (monkeypatch.setattr in
test_summarization_middleware, test_thread_data_middleware, and others) failed
with "module 'deerflow.agents' has no attribute ...".

Run the check in a subprocess instead: the import is genuinely fresh, the
assertion is stronger (the provider must still be the SDK-less proxy, proving
nothing was installed at any point), and no module identity leaks into the
rest of the suite.
@imohammedansari

Copy link
Copy Markdown
Contributor Author

@WillemJiang Fixed a quick test failure from the last CI run; all tests pass now. Ready for another look, thanks.

@WillemJiang WillemJiang added this to the 2.1.0 milestone Jul 12, 2026
@WillemJiang WillemJiang added the reviewing A maintainer is reviewing this PR label Jul 12, 2026

@willem-bd willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing the Monocle observability integration. Solid, well-scoped, and well-tested — off by default, optional extra, process-global setup correctly moved into the Gateway lifespan and pinned by a subprocess regression test. Langfuse v4 coexistence is verified, and the upstream API contract is exercised against the real library (not just mocks). No correctness or security blockers.

Inline comments below flag a few design points to confirm and one minor accuracy issue. Additional minor nits (not blocking):

  • deerflow/tracing/__init__.py re-exports build_tracing_callbacks and the Langfuse helpers but not setup_monocle_tracing_if_enabled — re-exporting would match package convention (the Gateway imports the submodule directly, so it works either way).
  • test_tracing_config.py's autouse fixture clears LangSmith/Langfuse env but not MONOCLE_* now that monocle is part of TracingConfig; no assertion breaks today, but for hygiene it should clear MONOCLE_TRACING/MONOCLE_EXPORTERS/OKAHU_API_KEY.
  • No retention/cleanup for .monocle/ trace files (unlike cleanup_stale_upload_staging_files) — worth a follow-up or docs note, since traces contain prompts + completions.
  • The README claim Monocle's exporters also capture Langfuse's spans isn't directly proven by test_coexists_with_langfuse (it checks provider identity + processor presence, not actual cross-export).

Comment on lines +196 to +199
try:
setup_monocle_tracing_if_enabled()
except Exception: # pragma: no cover - observability must never break startup
logger.exception("Monocle tracing setup failed; continuing without it")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graceful-degrade vs. fail-fast asymmetry — worth documenting. This except Exception swallows the ValueError from monocle.validate() (bad exporter / missing OKAHU_API_KEY) and the RuntimeError for a missing extra, so a misconfigured Monocle setup logs at ERROR and continues without tracing — the Gateway stays up. That's a deliberate contrast with LangSmith/Langfuse, where build_tracing_callbacks() -> validate_enabled_tracing_providers() raises and aborts the agent run.

The behavior is internally consistent with "cannot break startup", but the asymmetry isn't called out in backend/AGENTS.md. A one-line note there (Monocle misconfiguration degrades gracefully rather than failing fast) would make the intent explicit instead of discovered.

Comment on lines +61 to +66
def is_configured(self) -> bool:
# Intentionally coarser than LangSmith/Langfuse (which fold credential
# presence in): whether Monocle needs credentials depends on the exporter
# mix, so that composite check lives in validate(), called from setup at
# Gateway startup — where a bad config fails loudly without touching runs.
return self.enabled

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_configured returns True even when the config is unusable. With MONOCLE_TRACING=true + MONOCLE_EXPORTERS=okahu + no OKAHU_API_KEY, is_monocle_tracing_enabled() is True, then setup_monocle_tracing_if_enabled() raises on validate(). The only caller re-validates, so it's safe today, but the name reads as "ready to use" — unlike LangSmith/Langfuse is_configured, which folds credentials in.

Consider renaming to is_enabled (matching explicitly_enabled_providers semantics) or folding the okahu-key check into is_configured so the boolean is truthful. The inline comment explains the intent, but the public name still invites misuse by a future caller.

# Fail fast on an unknown MONOCLE_EXPORTERS value or a missing OKAHU_API_KEY,
# with a clear message, before instrumenting. Validated here (not in the
# per-run callback path) so a config typo never breaks agent runs.
monocle.validate()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Embedded/TUI path gets no signal that Monocle is a no-op. Because validate() lives here (startup-only wrapper) rather than in validate_enabled_tracing_providers() (the per-run path), the embedded DeerFlowClient and TUI — which never hit the Gateway lifespan — neither validate nor set up Monocle. A user who sets MONOCLE_TRACING=true in embedded mode gets silently nothing, with no log.

This matches the docs (embedded users call setup_monocle_tracing_if_enabled() themselves), but a debug-log when enabled-but-uninitialized would help surface it. Alternatively, wiring monocle.validate() into TracingConfig.validate_enabled() would at least fail-fast on a bad config in every path.

# outputs, completions) beyond the local .monocle/ directory, e.g. to an
# external collector (okahu, s3, ...). Warn loudly so it can't happen
# unnoticed.
non_file = [e.strip() for e in exporters.split(",") if e.strip() and e.strip() != "file"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Off-box warning fires for console too. console writes to local stdout, but it lands in non_file and triggers the "beyond the local .monocle/ file ... Make sure that destination is trusted" warning — wording that implies network exfiltration. The intent is to warn about okahu/s3/blob/gcs.

Either exclude console from non_file (warn only for the off-box exporters) or adjust the message so console doesn't read as off-box. (Arguable in containerized depots where stdout is scraped by a log pipeline — but then say so explicitly.)

Langfuse(tracing_enabled=True)

assert trace.get_tracer_provider() is provider # provider not replaced
names = [type(p).__name__ for p in provider._active_span_processor._span_processors]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reaches into OTel SDK privates. provider._active_span_processor._span_processors are undocumented internals that can break on an opentelemetry-sdk bump (the lock pins 0.62b1). Consider asserting on observable behavior instead — e.g. create a span and confirm it's processed/exported by each provider — or guard the assertion with a version check so a future SDK upgrade fails loudly and obviously rather than as a mysterious CI break.

@WillemJiang

Copy link
Copy Markdown
Collaborator

@imohammedansari, please take a look at the latest review comments.

…doc alignment

Responds to the post-approval review round:

- Scope the off-box exporter warning to the remote exporters (okahu, s3,
  blob, gcs): console writes to local stdout and no longer trips it.
  config.example.yaml's data-handling note now distinguishes file /
  console / remote likewise.
- Rename MonocleTracingConfig.is_configured to is_enabled so the boolean
  reads as what it checks; the exporter-dependent credential check stays
  in validate(), run at Gateway startup.
- Hint on the embedded path: build_tracing_callbacks() logs a debug line
  when MONOCLE_TRACING is set but setup never ran in this process, so
  embedded DeerFlowClient/TUI users are not left with silent no-op
  tracing. Backed by a process-global setup flag.
- Re-export setup_monocle_tracing_if_enabled from deerflow.tracing,
  matching the package convention.
- Note the deliberate fail-open-at-startup contrast with
  LangSmith/Langfuse in the lifespan, and the OTel SDK-internals
  dependency in the coexistence test.
- Test hygiene: clear MONOCLE_* env in the tracing config/factory
  fixtures; reset the setup flag in the monocle test fixture; reword the
  README Langfuse-spans claim as the shared-provider inference it is.
- Document that .monocle/ trace files are never rotated or cleaned up.
@imohammedansari
imohammedansari force-pushed the monocle-instrumentation branch from 68bc1d2 to 6c36548 Compare July 13, 2026 23:25
@imohammedansari

Copy link
Copy Markdown
Contributor Author

@WillemJiang @willem-bd All review feedback addressed in 6c36548. Ready for another look 🚀

@willem-bd willem-bd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the Monocle observability integration. Strong design: the non-callback provider is correctly kept out of the callback path (TracingConfig's provider lists are hardcoded, so adding a monocle field doesn't leak), setup is off-by-default + lazy-imported + wrapped in the lifespan try/except, and the 16 tests (incl. subprocess-isolated real-setup tests for idempotency / coexistence / import-cleanliness) are thorough.

Inline notes below, ranked by what matters most:

  • The process-global OTel mutation and its fragile, private-introspection-based coexistence test (#1) is the real risk:high coupling to weigh.
  • The off-box warning doesn't mention co-exported Langfuse spans (#2); the lifespan exception path is untested (#7).
  • Lower-severity: beta opentelemetry-instrumentation 0.62b1 transitive dep (#3), env-var docs in config.example.yaml (#4), duplicated exporter parsing (#5), and the manually-copied exporter allow-list (#6).

Overall I'd merge with #1 acknowledged and ideally #2 / #7 addressed.

raise RuntimeError("MONOCLE_TRACING is enabled but monocle_apptrace is not installed. Install the 'monocle' extra: `uv sync --extra monocle` in backend/, or `pip install 'deerflow-harness[monocle]'`.") from exc

# monocle_exporters_list takes the comma-separated string as-is (monocle_apptrace's API).
setup_monocle_telemetry(workflow_name="deer-flow", monocle_exporters_list=exporters)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call installs a process-global OTel TracerProvider and patches span serialization for the whole process, and it shares that provider with Langfuse v4. Coexistence is verified by test_coexists_with_langfuse, but that test reaches into OTel SDK privates (provider._active_span_processor._span_processors) and depends on Langfuse reusing the existing provider rather than replacing it. A Langfuse major bump or an OTel-SDK change could silently break span export for both providers.

The mitigation (off by default, optional extra) is sound - just flagging this as the one coupling to weigh against the risk:high label. If a public API to list a provider's span processors ever lands in the OTel SDK, swapping the private introspection for it would harden the test.

# `console` stays on local stdout, so only the remote exporters are flagged.
off_box = [e.strip() for e in exporters.split(",") if e.strip() and e.strip() not in ("file", "console")]
if off_box:
logger.warning(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README notes that "Monocle's exporters also see Langfuse's spans when both are enabled," but this runtime warning only mentions Monocle's own trace data. A user running Langfuse + MONOCLE_EXPORTERS=okahu won't be warned from the logs that Langfuse's spans also leave the box via the Okahu exporter. Consider extending the message when Langfuse is also enabled, e.g. ... including spans from co-enabled OTel providers such as Langfuse.

def validate(self) -> None:
if not self.enabled:
return
selected = [e.strip() for e in self.exporters.split(",") if e.strip()]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exporters.split(",") + .strip() parse is duplicated in setup_monocle_tracing_if_enabled() (monocle.py, the off_box = [...] line). They're consistent today, but a future edit to one could silently diverge (e.g., the validation set and the off-box set disagreeing on whitespace). A small exporter_list property on MonocleTracingConfig that both call sites use would keep them in lockstep.

Comment thread backend/packages/harness/deerflow/config/tracing_config.py
boxlite = ["boxlite>=0.9.7"]
# Agent observability (Monocle). Optional so a default install stays free of the
# OpenTelemetry stack; only pulled in when MONOCLE_TRACING is used.
monocle = ["monocle_apptrace>=0.8.8"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

monocle_apptrace>=0.8.8 pulls in opentelemetry-instrumentation 0.62b1 (a beta) plus rfc3986 as transitive deps (new in uv.lock). Betas can ship breaking changes between point releases. It's correctly gated behind the optional extra so default/minimal installs stay clean, but anyone opting into [monocle] gets a beta OTel instrumentation in their tree. Worth a note here, or a watch on the OTel instrumentation pin floor.

Comment thread config.example.yaml
Comment thread backend/app/gateway/app.py Outdated
# a bad Monocle config only logs: the Gateway keeps serving without tracing.
try:
setup_monocle_tracing_if_enabled()
except Exception: # pragma: no cover - observability must never break startup

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This except is # pragma: no cover, and test_gateway_lifespan_initializes_monocle patches setup with a spy returning False, so the "setup raises -> Gateway keeps serving" guarantee isn't exercised. A test driving the lifespan with a setup that raises (e.g., monkeypatch setup_monocle_tracing_if_enabled to raise ValueError) and asserting the lifespan still completes would close it.

Side note: a Monocle misconfiguration is silently ignored after this one startup log (per-run build_tracing_callbacks() re-validates only langsmith/langfuse, not Monocle), so a user who fixes the config won't get tracing without a restart - worth a line in the docs.

configure_logging() from earlier tests in the full suite pins an explicit
INFO level on the logger hierarchy, so a root-level caplog.at_level(DEBUG)
never sees the factory's debug hint. Scope caplog to
deerflow.tracing.factory so the test is independent of suite ordering.
… parse dedup

- Off-box warning now notes that Langfuse's spans are exported too when
  both providers are enabled and share the global OTel provider; pinned
  both ways by tests.
- Pin the lifespan fail-open contract: a raising Monocle setup is logged
  and the Gateway keeps serving (pragma dropped now that the path is
  exercised). README notes a config error is reported at startup and
  tracing stays off until restart.
- Hoist exporter parsing into MonocleTracingConfig.exporter_list so
  validate() and the off-box warning cannot diverge, and note the
  upstream coupling on the exporter allow-list.
- Reduce config.example.yaml's Monocle block to a pointer; the capture,
  retention, and data-handling detail lives in README's Monocle section.
@imohammedansari

Copy link
Copy Markdown
Contributor Author

@willem-bd @WillemJiang Addressed in 2920b60 — the off-box warning now discloses that Langfuse's spans are co-exported when both providers are enabled (#2), the lifespan fail-open path has a test and the restart-to-apply behavior is documented (#7), plus the exporter parse dedup (#5), the allow-list coupling note (#6), and config.example.yaml reduced to a pointer with the detail in the README (#4). #1 acknowledged inline. Ready for another look 🚀

@WillemJiang
WillemJiang merged commit 4af6178 into bytedance:main Jul 14, 2026
15 checks passed
lllyfff added a commit to qin-chenghan/deer-flow that referenced this pull request Jul 14, 2026
Upstream since last merge: 12 commits (bytedance#4154 mcp, bytedance#4146/bytedance#4102 models,
bytedance#4024 trace Monocle, bytedance#4157/bytedance#4137 security escapes, bytedance#4116 sandbox, bytedance#4034
memory confidence tests, bytedance#4064 runs, bytedance#4136 agents SOUL, bytedance#4115 subagent cap,
bytedance#4139 docs).

Memory: only bytedance#4034 touches memory this batch, and it is test-only
(test_memory_staleness_review.py + test_memory_updater.py), which test the
upstream monolithic API and stay module-level skipped in the vendored setup.
No memory logic to re-port -- rename-drop pattern re-checked: vendored
updater/queue/storage/prompt/message_processing keep all ports, and bytedance#4074
_coerce_source_confidence already covers the raw-read coercion bytedance#4034 tests.

Conflicts resolved:
- CHANGELOG.md: combined [Unreleased] sections (memory breaking/changed +
  upstream's models bytedance#4146 Fixed).
- test_memory_staleness_review.py / test_memory_updater.py: kept ours
  (module-level skipped; upstream's monolithic-API tests don't apply to vendored).

Verified: ruff check + format clean; 418 tests passed (memory + lead_agent_prompt
+ custom_agent + client + e2e + consolidation + config reload), 12 skipped (env).

Co-Authored-By: Claude <noreply@anthropic.com>
marvin9551 pushed a commit to marvin9551/deer-flow that referenced this pull request Aug 21, 2026
* Add Monocle tracing

Enable Monocle (OpenTelemetry tracing for LLM apps) with one setup call plus the monocle_apptrace dependency. setup_monocle_telemetry auto-instruments the frameworks already in use and writes traces to .monocle/. Additive; no changes to application logic.

* Config-gate Monocle telemetry in the Gateway lifespan

Addresses review on bytedance#4024: moves setup_monocle_telemetry out of agents/__init__ import time into the Gateway lifespan, gated by MonocleTracingConfig (MONOCLE_TRACING env, default off). Warns on the Langfuse/global-OTel-provider conflict and relies on monocle_apptrace's own duplicate-setup guard and existing-provider attach. Pins monocle_apptrace>=0.8.8 (+ uv.lock), adds .monocle/ to .gitignore, adds tests (default-off / toggle-on / no import-time setup), and documents exporters, Okahu, and the VS Code viewer in README, config.example.yaml, and backend/AGENTS.md.

* Clarify Monocle/Langfuse single-provider guidance

Make the docstring, warning, and AGENTS.md consistent with the README: only one library can own the global OpenTelemetry provider; Monocle initializes at startup before Langfuse's per-run handler, so enabling both drops Langfuse's spans — enable one OTel tracer (LangSmith, a callback, coexists fine).

* Address review: optional extra, exporter validation, off-box warning, tests

Responds to the second review round.

- Make monocle_apptrace an optional extra (deerflow-harness[monocle], re-exposed
  as deer-flow[monocle]) following the boxlite/tui precedent, so a default
  install no longer pulls the OpenTelemetry stack. It stays pinned in the dev
  group for the tracing tests, and enabling MONOCLE_TRACING without the extra
  raises a clear install error.
- Warn loudly at startup whenever any exporter other than `file` is configured,
  since those move prompts, tool inputs/outputs, and completions beyond the
  local .monocle/ directory.
- Validate MONOCLE_EXPORTERS against the known exporter names and require
  OKAHU_API_KEY when okahu is selected, mirroring the Langfuse pattern.
  Validation runs from Monocle's own init (not validate_enabled) so a config
  typo can never fail agent runs; errors surface at Gateway startup instead.
- Grow the tests from 5 to 13: caplog coverage for the Langfuse-conflict and
  off-box warnings, exporter validation cases, a stronger import-time
  regression that asserts the global TracerProvider is not replaced, and a
  subprocess double-invoke test exercising the real check_duplicate_setup.
- Docs: config.example.yaml block retitled to a dedicated tracing header;
  README documents the [monocle] install and scopes tracing to Gateway runs.

* docs: align Monocle README section with the other tracing providers

Lead with what Monocle is and captures, drop the install step (the dev
group already ships monocle_apptrace via uv sync; unusual installs get
the RuntimeError), and point the missing-package error at the repo-native
command (uv sync --extra monocle / deerflow-harness[monocle]).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address review: verified Langfuse coexistence, lifespan test, scope docs

Responds to the third review round.

The Langfuse conflict claim was wrong, verified empirically against langfuse
4.5.1 in both init orders: whichever library initializes second reuses the
existing global TracerProvider and attaches its own span processor, so neither
side loses spans. Dropped the warning and its tests, corrected the README,
AGENTS.md, and config.example.yaml statements, and pinned the verified behavior
with test_coexists_with_langfuse (real monocle + real langfuse in a subprocess,
no mocks). One honest caveat documented: both processors see all spans, so
Monocle's exporters also capture Langfuse's spans when both are enabled.

Also from the review:
- Document the Gateway-only scope in AGENTS.md: the lifespan is the sole call
  site, so the embedded DeerFlowClient and TUI are not instrumented; embedded
  users call setup_monocle_tracing_if_enabled() themselves.
- Add test_gateway_lifespan_initializes_monocle pinning the lifespan wiring.
- Comment why MonocleTracingConfig.is_configured is intentionally coarser than
  LangSmith/Langfuse (composite validation lives in validate() at startup).
- Note that monocle_exporters_list takes the comma-separated string as-is.
- Module-level importorskip("monocle_apptrace") so minimal installs collect
  the test module cleanly.

* docs: reword Monocle intro sentence

* fix(tests): run the import-time regression in a subprocess

test_no_import_time_setup deleted deerflow.agents* from sys.modules and
re-imported to force __init__ to re-execute. The re-import creates new module
objects, and restoring the old sys.modules entries afterwards leaves the parent
package's attribute bindings pointing at the new ones, so any later test that
resolves a deerflow.agents.* dotted path (monkeypatch.setattr in
test_summarization_middleware, test_thread_data_middleware, and others) failed
with "module 'deerflow.agents' has no attribute ...".

Run the check in a subprocess instead: the import is genuinely fresh, the
assertion is stronger (the provider must still be the SDK-less proxy, proving
nothing was installed at any point), and no module identity leaks into the
rest of the suite.

* Address review: console warning scope, embedded hint, honest naming, doc alignment

Responds to the post-approval review round:

- Scope the off-box exporter warning to the remote exporters (okahu, s3,
  blob, gcs): console writes to local stdout and no longer trips it.
  config.example.yaml's data-handling note now distinguishes file /
  console / remote likewise.
- Rename MonocleTracingConfig.is_configured to is_enabled so the boolean
  reads as what it checks; the exporter-dependent credential check stays
  in validate(), run at Gateway startup.
- Hint on the embedded path: build_tracing_callbacks() logs a debug line
  when MONOCLE_TRACING is set but setup never ran in this process, so
  embedded DeerFlowClient/TUI users are not left with silent no-op
  tracing. Backed by a process-global setup flag.
- Re-export setup_monocle_tracing_if_enabled from deerflow.tracing,
  matching the package convention.
- Note the deliberate fail-open-at-startup contrast with
  LangSmith/Langfuse in the lifespan, and the OTel SDK-internals
  dependency in the coexistence test.
- Test hygiene: clear MONOCLE_* env in the tracing config/factory
  fixtures; reset the setup flag in the monocle test fixture; reword the
  README Langfuse-spans claim as the shared-provider inference it is.
- Document that .monocle/ trace files are never rotated or cleaned up.

* fix(tests): pin the factory logger level in the embedded-hint tests

configure_logging() from earlier tests in the full suite pins an explicit
INFO level on the logger hierarchy, so a root-level caplog.at_level(DEBUG)
never sees the factory's debug hint. Scope caplog to
deerflow.tracing.factory so the test is independent of suite ordering.

* Address review: co-export disclosure, lifespan failure test, exporter parse dedup

- Off-box warning now notes that Langfuse's spans are exported too when
  both providers are enabled and share the global OTel provider; pinned
  both ways by tests.
- Pin the lifespan fail-open contract: a raising Monocle setup is logged
  and the Gateway keeps serving (pragma dropped now that the path is
  exercised). README notes a config error is reported at startup and
  tracing stays off until restart.
- Hoist exporter parsing into MonocleTracingConfig.exporter_list so
  validate() and the off-box warning cannot diverge, and note the
  upstream coupling on the exporter allow-list.
- Reduce config.example.yaml's Monocle block to a pointer; the capture,
  retention, and data-handling detail lives in README's Monocle section.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
jihtsan pushed a commit to jihtsan/dnx-deer-flow that referenced this pull request Aug 29, 2026
* Add Monocle tracing

Enable Monocle (OpenTelemetry tracing for LLM apps) with one setup call plus the monocle_apptrace dependency. setup_monocle_telemetry auto-instruments the frameworks already in use and writes traces to .monocle/. Additive; no changes to application logic.

* Config-gate Monocle telemetry in the Gateway lifespan

Addresses review on bytedance#4024: moves setup_monocle_telemetry out of agents/__init__ import time into the Gateway lifespan, gated by MonocleTracingConfig (MONOCLE_TRACING env, default off). Warns on the Langfuse/global-OTel-provider conflict and relies on monocle_apptrace's own duplicate-setup guard and existing-provider attach. Pins monocle_apptrace>=0.8.8 (+ uv.lock), adds .monocle/ to .gitignore, adds tests (default-off / toggle-on / no import-time setup), and documents exporters, Okahu, and the VS Code viewer in README, config.example.yaml, and backend/AGENTS.md.

* Clarify Monocle/Langfuse single-provider guidance

Make the docstring, warning, and AGENTS.md consistent with the README: only one library can own the global OpenTelemetry provider; Monocle initializes at startup before Langfuse's per-run handler, so enabling both drops Langfuse's spans — enable one OTel tracer (LangSmith, a callback, coexists fine).

* Address review: optional extra, exporter validation, off-box warning, tests

Responds to the second review round.

- Make monocle_apptrace an optional extra (deerflow-harness[monocle], re-exposed
  as deer-flow[monocle]) following the boxlite/tui precedent, so a default
  install no longer pulls the OpenTelemetry stack. It stays pinned in the dev
  group for the tracing tests, and enabling MONOCLE_TRACING without the extra
  raises a clear install error.
- Warn loudly at startup whenever any exporter other than `file` is configured,
  since those move prompts, tool inputs/outputs, and completions beyond the
  local .monocle/ directory.
- Validate MONOCLE_EXPORTERS against the known exporter names and require
  OKAHU_API_KEY when okahu is selected, mirroring the Langfuse pattern.
  Validation runs from Monocle's own init (not validate_enabled) so a config
  typo can never fail agent runs; errors surface at Gateway startup instead.
- Grow the tests from 5 to 13: caplog coverage for the Langfuse-conflict and
  off-box warnings, exporter validation cases, a stronger import-time
  regression that asserts the global TracerProvider is not replaced, and a
  subprocess double-invoke test exercising the real check_duplicate_setup.
- Docs: config.example.yaml block retitled to a dedicated tracing header;
  README documents the [monocle] install and scopes tracing to Gateway runs.

* docs: align Monocle README section with the other tracing providers

Lead with what Monocle is and captures, drop the install step (the dev
group already ships monocle_apptrace via uv sync; unusual installs get
the RuntimeError), and point the missing-package error at the repo-native
command (uv sync --extra monocle / deerflow-harness[monocle]).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Address review: verified Langfuse coexistence, lifespan test, scope docs

Responds to the third review round.

The Langfuse conflict claim was wrong, verified empirically against langfuse
4.5.1 in both init orders: whichever library initializes second reuses the
existing global TracerProvider and attaches its own span processor, so neither
side loses spans. Dropped the warning and its tests, corrected the README,
AGENTS.md, and config.example.yaml statements, and pinned the verified behavior
with test_coexists_with_langfuse (real monocle + real langfuse in a subprocess,
no mocks). One honest caveat documented: both processors see all spans, so
Monocle's exporters also capture Langfuse's spans when both are enabled.

Also from the review:
- Document the Gateway-only scope in AGENTS.md: the lifespan is the sole call
  site, so the embedded DeerFlowClient and TUI are not instrumented; embedded
  users call setup_monocle_tracing_if_enabled() themselves.
- Add test_gateway_lifespan_initializes_monocle pinning the lifespan wiring.
- Comment why MonocleTracingConfig.is_configured is intentionally coarser than
  LangSmith/Langfuse (composite validation lives in validate() at startup).
- Note that monocle_exporters_list takes the comma-separated string as-is.
- Module-level importorskip("monocle_apptrace") so minimal installs collect
  the test module cleanly.

* docs: reword Monocle intro sentence

* fix(tests): run the import-time regression in a subprocess

test_no_import_time_setup deleted deerflow.agents* from sys.modules and
re-imported to force __init__ to re-execute. The re-import creates new module
objects, and restoring the old sys.modules entries afterwards leaves the parent
package's attribute bindings pointing at the new ones, so any later test that
resolves a deerflow.agents.* dotted path (monkeypatch.setattr in
test_summarization_middleware, test_thread_data_middleware, and others) failed
with "module 'deerflow.agents' has no attribute ...".

Run the check in a subprocess instead: the import is genuinely fresh, the
assertion is stronger (the provider must still be the SDK-less proxy, proving
nothing was installed at any point), and no module identity leaks into the
rest of the suite.

* Address review: console warning scope, embedded hint, honest naming, doc alignment

Responds to the post-approval review round:

- Scope the off-box exporter warning to the remote exporters (okahu, s3,
  blob, gcs): console writes to local stdout and no longer trips it.
  config.example.yaml's data-handling note now distinguishes file /
  console / remote likewise.
- Rename MonocleTracingConfig.is_configured to is_enabled so the boolean
  reads as what it checks; the exporter-dependent credential check stays
  in validate(), run at Gateway startup.
- Hint on the embedded path: build_tracing_callbacks() logs a debug line
  when MONOCLE_TRACING is set but setup never ran in this process, so
  embedded DeerFlowClient/TUI users are not left with silent no-op
  tracing. Backed by a process-global setup flag.
- Re-export setup_monocle_tracing_if_enabled from deerflow.tracing,
  matching the package convention.
- Note the deliberate fail-open-at-startup contrast with
  LangSmith/Langfuse in the lifespan, and the OTel SDK-internals
  dependency in the coexistence test.
- Test hygiene: clear MONOCLE_* env in the tracing config/factory
  fixtures; reset the setup flag in the monocle test fixture; reword the
  README Langfuse-spans claim as the shared-provider inference it is.
- Document that .monocle/ trace files are never rotated or cleaned up.

* fix(tests): pin the factory logger level in the embedded-hint tests

configure_logging() from earlier tests in the full suite pins an explicit
INFO level on the logger hierarchy, so a root-level caplog.at_level(DEBUG)
never sees the factory's debug hint. Scope caplog to
deerflow.tracing.factory so the test is independent of suite ordering.

* Address review: co-export disclosure, lifespan failure test, exporter parse dedup

- Off-box warning now notes that Langfuse's spans are exported too when
  both providers are enabled and share the global OTel provider; pinned
  both ways by tests.
- Pin the lifespan fail-open contract: a raising Monocle setup is logged
  and the Gateway keeps serving (pragma dropped now that the path is
  exercised). README notes a config error is reported at startup and
  tracing stays off until restart.
- Hoist exporter parsing into MonocleTracingConfig.exporter_list so
  validate() and the off-box warning cannot diverge, and note the
  upstream coupling on the exporter allow-list.
- Reduce config.example.yaml's Monocle block to a pointer; the capture,
  retention, and data-handling detail lives in README's Monocle section.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Gateway / runtime / core backend under backend/ area:deps Dependency manifests / lockfiles area:docs Documentation and Markdown only needs-validation Touches front/back contract surface; needs real-path validation reviewing A maintainer is reviewing this PR risk:high High risk: backend API, agents, sandbox, auth, deps, CI size/L PR changes 300-700 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants