Fix: only 'false'/'0' disable ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS (adk-python parity)

[AmaadMartin]

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):
   N/A — no public issue is tracking this.

2. Or, if no issue exists, describe the change:

⚠️ Stacked PR. Base is fix/tracing-capture-env-gate-tests (#308), not main.
See "Collision check" below.

⚠️ Intentional behavior change. No public API, type, or signature changes — the
break is behavioral and config-triggered only. See "Migration note" below.

Problem: shouldAddRequestResponseToSpans() in core/src/telemetry/tracing.ts
documents an opt-out contract and implements an opt-in one.

const envValue = process.env.ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS || 'true';
return envValue === 'true' || envValue === '1';

The JSDoc promises "false only when ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS is
explicitly set to 'false' or '0'", but the comparison is an exact,
case-sensitive, untrimmed match against two literals. Any value that is not
byte-for-byte true or 1 silently disables span content capture — the opposite
of what the docstring promises. TRUE, True, " true" (a leading space, e.g.
from KEY = true in a .env file), "true\n" (a heredoc or kubectl value),
" ", and typos like treu all turn capture off today.

The failure is silent in the worst way: no warning is logged, and the symptom —
spans arriving with '{}' where content was expected — looks like a telemetry
pipeline problem rather than a config typo. The gate feeds six call sites
(traceToolCall ×2, traceMergedToolCalls, traceCallLlm ×2, traceSendData),
so a mis-parse blanks content everywhere at once.

Solution: normalize the value before comparing, and invert the comparison so
it actually expresses the documented opt-out:

const envValue = (process.env.ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS ?? 'true')
  .trim()
  .toLowerCase();
return envValue !== 'false' && envValue !== '0';

Two statements in one module-private function; no new types, constants, helpers,
or modules. None of the six call sites change.

Cross-language parity

This is the adk-python behavior, verified field by field against
src/google/adk/telemetry/context.py in this repo's reference checkout rather
than inferred from the symbol name:

ref	source
context.py:50	_FALSY_ENV_VALUES = frozenset({'0', 'false'})
context.py:211	os.getenv(ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS, 'true').strip().lower()
context.py:213	return env_value not in _FALSY_ENV_VALUES

Parity here is scoped strictly to how the env string is normalized. Python
resolves this knob through a TelemetryConfig precedence ladder (admin lock >
per-request RunConfig > env var > default) with an OTel-spec sibling knob;
adk-js has none of that machinery and this PR does not port it.

Where local convention and parity could conflict, parity wins: the accepted
values and their polarity are observable across the language boundary, so they
must agree. Nothing process-local (naming, module layout, privacy of the
function) was changed to match Python.

Before/after contract

raw env value	before	after	note
unset	ON	ON	default preserved
'true' / '1'	ON	ON
'false' / '0'	OFF	OFF
'TRUE' / 'True'	OFF	ON	case-insensitive affirmative
'FALSE'	OFF	OFF	same result, now for the right reason
' true' / 'true\n'	OFF	ON	whitespace tolerated
' false ' / '0\n'	OFF	OFF	same result, now for the right reason
'' (empty)	ON	ON
' ' (whitespace only)	OFF	ON	trims to empty ⇒ not disabling
'yes' / 'no' / 'treu'	OFF	ON	unrecognized ⇒ default ON

The FALSE / ' false ' rows agreed with Python before only by accident: the old
code returned false because the value matched nothing, not because it
recognized a disabling value.

Migration note (privacy-relevant — please read)

The only supported disabling values are false and 0. If you disabled capture
with any other value, switch to false before upgrading.

An operator who wrote a non-canonical negative such as no, off, or disabled
is currently getting capture OFF by accident — the value matches neither
true nor 1. After this change they get capture ON, meaning prompt/response
content starts flowing onto spans for a deployment that believed it had opted out.

This is nevertheless the correct resolution: adk-python behaves exactly this way
today ('no' → capture on), so honoring no/off in adk-js would create a
new parity gap while closing another; and the documented contract has always been
false/0, so no was never a supported disabling value. The alternative —
leaving it alone — keeps the docstring lying about TRUE and whitespace.

Unaffected: every deployment that leaves the var unset or sets lowercase
true/1/false/0. That includes anything mirroring adk-python's deploy-time
default of 'false' (cli_deploy.py).

Collision check

gh pr list --repo AmaadMartin/adk-js --state open --limit 100 plus
gh pr diff --name-only on the adjacent candidates. One real overlap:

• Test: Pin the ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS tracing gate with hermetic unit tests #308 fix/tracing-capture-env-gate-tests — a test-only PR that pinned the
  current gate behavior in the exact file this PR must change. It does not
  contain the production fix, so this is an overlap, not a duplicate. This PR is
  stacked on its branch rather than branched from main, so Test: Pin the ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS tracing gate with hermetic unit tests #308 must merge
  first. Merging this against main instead would conflict and would reinstate
  assertions that contradict the new contract.
• Fix: make env-var stubbing hermetic across the test suite (vitest unstubEnvs + ambient-env coverage) #281 (vitest-env-stub-hermeticity) and Fix: scrub ADK environment variables from unit test runs #302 (hermetic-unit-test-env) touch env
  stubbing but not core/test/telemetry/tracing_test.ts or the tracing gate. No
  collision.

Because #308 pinned behavior this PR intentionally changes, two of its table rows
are retargeted in a dedicated, clearly-labeled commit (3786b1b6) rather than
deleted:

row	#308 pinned	this PR	why
'TRUE'	redacts	captures	case-insensitive affirmative
'not-a-boolean'	redacts	captures	unrecognized resolves to the default

No other row, assertion, fixture, or description from #308 is modified, and no test
is deleted, skipped, or weakened. Both retargeted rows are still proven to fail
against a mutation (see below), so they retain regression signal.

Design notes

• Why not reuse getBooleanEnvVar (core/src/utils/env_aware_utils.ts:99)? Its
  polarity is the opposite of what this knob needs. It is a default-off helper
  (unset → false), whereas ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS is default-on.
  Wiring this gate to it — or to its negation — would flip the documented default and
  break every existing deployment. It also has four other live callers, so touching it
  widens the blast radius of a one-line telemetry fix. Its own missing-trim() gap is
  a separate concern and is deliberately not folded in here.
• Why ?? and not ||? ?? only defaults when the variable is absent,
  mirroring os.getenv(name, 'true'). The two are observationally identical here
  (an empty string trims to '', which is neither 'false' nor '0').
• Why keep the 'true' literal at all? With a deny-list, undefined would fail
  both inequalities on its own, so ?? 'true' is not load-bearing and
  ?.trim().toLowerCase() would be one line shorter. It is kept deliberately: this is
  a privacy-relevant default-on knob, and an explicit default states that contract at
  the point of the read instead of leaving it to emerge from two inequality checks. It
  also keeps the shape aligned with the Python reference, which passes the same literal.
• Deliberately not done: no FALSY_ENV_VALUES constant or new utils module (Python
  uses a frozenset because it has several such vars; adk-js has exactly one), no
  yes/no/on/off handling, no warning log for unrecognized values, no
  if (!process.env) browser guard, no change to the default, and no export — the
  function stays module-private and is reached through the exported trace* functions.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

Tests extend the existing table in core/test/telemetry/tracing_test.ts rather than
adding a second parallel matrix — that harness already runs every env value against all
four gated trace functions (traceCallLlm, traceToolCall, traceMergedToolCalls,
traceSendData), which also proves the gate is shared rather than per-function. Twelve
rows were added covering every equivalence class of the normalized parse:

class	values	expect
case-variant disabling	'FALSE'	redacts
padded disabling	' false ', '0\n'	redacts
case-variant affirmative	'True'	captures
padded affirmative	' true', 'true\n'	captures
whitespace only	' '	captures
unrecognized	'yes', 'no', 'treu'	captures

'no' carries an inline comment naming it as the deliberate flip.

Proof the tests can fail (three mutations). Coverage alone is not proof, so each new
row was run against mutated source and confirmed to fail. The three mutations fail
disjoint row sets, which is what shows the matrix pins the normalization and the
polarity independently:

#	mutation	result	failure message
1	restore the full pre-fix body (|| 'true', === 'true' || === '1')	36 failed / 43 passed — every ★ affirmative row ('TRUE', 'True', ' true', 'true\n', ' ', 'yes', 'no', 'treu', 'not-a-boolean') × 4 functions	AssertionError: expected '{}' to contain 'test-model' (and 'param1' / 'test-result' / 'hello-trace-data')
2	keep .trim().toLowerCase(), revert to the opt-in comparison	24 failed / 55 passed — only the polarity rows ('', ' ', 'yes', 'no', 'treu', 'not-a-boolean')	AssertionError: expected '{}' to contain 'test-model'
3	keep the opt-out comparison, drop .trim().toLowerCase()	12 failed / 67 passed — only the disabling-side normalization rows ('FALSE', ' false ', '0\n')	AssertionError: expected '{"param1":"value1"}' to be '{}'

Mutation 3 was added because the 'FALSE' / ' false ' / '0\n' rows pass under both
mutations 1 and 2 (they are "same result, different reason" rows); without it those three
rows would have been unproven. Every added row and both retargeted rows fail under at
least one mutation. Source was restored byte-exact afterwards and re-verified.

Coverage: the production delta is two statements in one function; the matrix exercises
both outcomes of both comparisons plus the default branch — 100% line and branch coverage
of the new code. No /* v8 ignore */ or any other coverage suppression was added, and no
threshold in vitest.config.ts was lowered.

CI is absent on this PR — validated locally instead. .github/workflows/validation.yaml
declares pull_request: branches: [main], so its run-tests job does not trigger for a PR
whose base is a stacked branch. The only check that runs here is auto-assign, which is not
validation. Once #308 merges and this PR is retargeted to main, run-tests will fire
normally.

Commands run on the exact pushed commit (87597f120a1dc5ef5e5e2c743da80c52e6800608):

npm run build                                                           # exit 0
npx vitest run --project unit:core core/test/telemetry/tracing_test.ts  # 79 passed
npx vitest run --project unit:core                                      # 160 files, 2281 passed
npm run lint                                                            # clean
npm run format:check                                                    # clean

The whole unit:core project was run (not just the targeted file) specifically to confirm
the env stubs do not leak across suites.

npm run ts:check exits non-zero both with and without this change: tsc --noEmit
reports 314 errors in 48 files on the base branch and 314 errors in 48 files here.
Normalizing and diffing the two error sets shows they are identical — the only difference
is that the 14 pre-existing errors in tracing_test.ts shift by exactly +52 lines, the
number of lines this PR adds. Those are the known core/dist/types vs core/src
dual-identity errors on pre-existing mock casts, not introduced or worsened here.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

1. npm install && npm run build at the repo root.
2. Run any agent that produces a tool call, with an OTel exporter configured, once with
   ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS=TRUE and once with
   ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS=' false '.
3. Confirm the first run emits populated gcp.vertex.agent.llm_request /
   gcp.vertex.agent.tool_call_args attributes and the second emits '{}'. Before this
   change, both runs emit '{}'.

No integration test is added: the gate has no I/O, no network, and no cross-component
wiring, so an integration test here would exercise the Vitest env stub rather than the
product.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.