Repository navigation
Conversation
JobContext.init_recording returned before _setup_cloud_tracer for all-off options, so no logger provider, log pipeline, or job registration existed. _upload_session_report still emitted evaluations, tag metadata, and the outcome from _on_session_end's has_evals branch, but those records landed in a no-op provider (fresh process) or were dropped by the log gate (reused process). Configure a report-only pipeline for all-off jobs: enable the log pipeline without Python log export, traces, or metrics, and let session-report logger scopes through the gate for a registered job even when logs are disabled.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 4 potential issues.
1 flag not posted on this PR by your GitHub settings β view it in Devin Review. (Configure)
| def _is_session_report_record(record: ReadableLogRecord) -> bool: | ||
| scope = getattr(record, "instrumentation_scope", None) | ||
| return getattr(scope, "name", None) in _SESSION_REPORT_SCOPES |
There was a problem hiding this comment.
π‘ Disabled Python logs still upload
When a shared handler captures a record=False job's Python log named evaluations, _is_session_report_record accepts it. The log reaches Cloud despite logs_enabled=False.
Learn more
The cloud log pipeline is shared between jobs. When another job has enabled Python logging, its root handler also sees Python logs from a job with recording disabled. OpenTelemetry assigns the logger name as the instrumentation scope, so the name evaluations satisfies this allowlist even for a Python log. The exporter then forwards the log because the job is registered, although its logs_enabled flag is false. Only records emitted by _upload_session_report must bypass that flag; a scope name alone cannot establish their origin.
Example: Job A records logs and leaves the root OTLP handler attached. Concurrent job B starts with record=False and logs logging.getLogger("evaluations").warning("ordinary diagnostic"). B's log uploads to Cloud instead of remaining disabled.
Recommended fix: Give _upload_session_report records a dedicated internal attribute and allow only those marked records through _GatedLogExporter when logs_enabled=False. Do not use public logger scope names as proof of origin.
Was this helpful? React with π or π to provide feedback.
| enable_traces=False, | ||
| enable_logs=False, | ||
| enable_session_report=True, | ||
| enable_metrics=False, |
There was a problem hiding this comment.
π‘ Disabled sessions still export shared metrics
When another job has installed the process-wide meter, enable_metrics=False leaves that meter active for record=False jobs. Their measurements still upload because _job_attrs stamps job identity without gating export.
Learn more
Metric instruments in otel_metrics bind to the OpenTelemetry global meter, which outlives individual jobs. Skipping _ensure_meter_provider only prevents creating a meter when none exists; it cannot turn off one installed by a previous or concurrent job. The metric helpers continue recording measurements with the disabled job's identity, and the existing meter's reader continues exporting them. This affects reused workers and concurrent jobs, not the fresh-process test.
Example: Job A enables recording and creates the Cloud meter provider. After Job A, Job B starts with record=False. A usage counter recorded by B still flows through A's meter provider to Cloud even though B passed enable_metrics=False.
Recommended fix: Gate metric recording or exporting per job using its resolved recording state, while preserving metrics for other jobs sharing the meter. Add a reused-worker test covering a recorded job followed by an all-off job.
Was this helpful? React with π or π to provide feedback.
| self._telemetry_state = _setup_cloud_tracer( | ||
| room_id=self.job.room.sid, | ||
| job_id=self.job.id, | ||
| agent_name=self.job.agent_name, | ||
| observability_url=obs_url, | ||
| enable_traces=False, | ||
| enable_logs=False, | ||
| enable_session_report=True, | ||
| enable_metrics=False, | ||
| metadata=self._otel_metadata(options, redaction_enabled=redaction_enabled), |
There was a problem hiding this comment.
π¨ Redacted sessions export unfiltered judge reasoning
With record=False and redaction enabled, a judge evaluation can contain conversation text in its reasoning. The evaluation log uses an unmarked attribute, so PII filtering preserves and uploads that text.
Was this helpful? React with π or π to provide feedback.
| self._telemetry_state = _setup_cloud_tracer( | ||
| room_id=self.job.room.sid, | ||
| job_id=self.job.id, | ||
| agent_name=self.job.agent_name, | ||
| observability_url=obs_url, | ||
| enable_traces=False, | ||
| enable_logs=False, | ||
| enable_session_report=True, | ||
| enable_metrics=False, | ||
| metadata=self._otel_metadata(options, redaction_enabled=redaction_enabled), |
There was a problem hiding this comment.
π¨ Redacted sessions export unfiltered tag metadata
With record=False, redaction enabled, and an evaluation, tag metadata containing conversation content now uploads. The tag log lacks a PII marker, so the redaction processor preserves it.
Was this helpful? React with π or π to provide feedback.
This branch has not been deployed
1 participant
fix(telemetry): upload evals when recording is fully disabled
What breaks
When a session runs with every recording option off (
record=Falsewith no logs/metrics/traces),JobContext._prepare_telemetry()never registers the job and never creates the log pipeline β so session report data (evaluations, tags, outcome) set on the session is silently dropped: nothing is exported at session end. Recording options should govern trace/log/metric volume, not the session report, which is an explicit product feature (session.evaluations,session.tags,session.outcome).Fix
traces.py:_GatedLogExporternow passes the session-report scopes (chat_history,evaluations) for registered jobs even when the job'slogsoption is off;configure()gainsenable_session_report(creates the log pipeline without Python-log export) andenable_metrics;_setup_cloud_tracerforwards both.job.py: the all-off case with a cloud observability URL runsinit_recording()β_setup_cloud_tracer(enable_traces=False, enable_logs=False, enable_session_report=True, enable_metrics=False), registering the job and creating the log pipeline. Traces, Python logs and metric providers stay off;_on_cleanupflushes the batch processor while registered, then unregisters.Tests
tests/test_session_report_recording_disabled.py: drives the realJobContext._prepare_telemetry()βinit_recording(_RECORDING_ALL_OFF)β_on_session_end()with a synchronousInMemoryLogRecordExporter(no network); asserts the exported bodies are exactly["evaluation", "outcome", "tag"]and that ordinary Python logs are not exported.exported records: [](everything dropped; reproduced in both a fresh process and one where a prior job had created the pipeline).SdkMeterProvider/ metric exporter created); ruff + type checks clean on the changed files.Relationship to open PR #7836
#7836 covers the sibling case (
logs=Falsebutconfigure()ran). This change covers the all-off case whereconfigure()is never reached. They are complementary, but they share thetraces.pysurface: both introduce a session-report scope set and the same gate/configure()hunks, so whichever lands first, the other needs a mechanical rebase. A clean reconciliation would keep one shared scope constant and one gate check, make log-pipeline creation unconditional inconfigure(), and keepenable_metrics(then #7836's dedicated flag can be dropped).Known non-blocking edges (documented in review): OTel derives instrumentation scope from the logger name, so a user logger named exactly
evaluations/chat_historycould pass the allowlist (no framework logger uses those names); and the policy-wide gate means alogs=Falsejob's session report is uploaded only when a process log pipeline already exists β the exact gap #7836 fills.