add langsmith tracing initialized from environment variables#6271
add langsmith tracing initialized from environment variables#6271
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to configure tracing providers, specifically LangSmith, via environment variables. It adds a new tracingEnv utility to handle environment variable detection, memoization, and merging with UI-based configurations. The PR also fixes a critical bug in AnalyticHandler where recursive onChainStart calls would overwrite the chainRun map, leading to orphaned traces in nested agent flows. A review comment suggests improving the consistency of environment variable detection to ensure all relevant flags are checked before clearing them, preventing potential duplicate traces.
| const tracingFlag = getEnvironmentVariable('LANGSMITH_TRACING') ?? getEnvironmentVariable('LANGCHAIN_TRACING_V2') | ||
| if (tracingFlag !== 'true') return undefined |
There was a problem hiding this comment.
The tracingFlag check only evaluates LANGSMITH_TRACING and LANGCHAIN_TRACING_V2. However, the LANGCHAIN_TRACING_FLAG_VARS array (defined on line 46 and cleared on line 71) includes other variables like LANGSMITH_TRACING_V2 and LANGCHAIN_TRACING (v1).
If a user attempts to enable tracing using one of these other variables, Flowise will not detect it, the function will return undefined, and the environment variables will not be cleared. This would allow LangChain's internal auto-tracer to run, causing the duplicate tracing issue this PR intends to solve. It is better to check all variables in the flag list to ensure consistency.
| const tracingFlag = getEnvironmentVariable('LANGSMITH_TRACING') ?? getEnvironmentVariable('LANGCHAIN_TRACING_V2') | |
| if (tracingFlag !== 'true') return undefined | |
| const tracingFlag = LANGCHAIN_TRACING_FLAG_VARS.map((v) => getEnvironmentVariable(v)).find((val) => val === 'true') | |
| if (tracingFlag !== 'true') return undefined |
5f2568a to
a692436
Compare
There was a problem hiding this comment.
Thanks for the fix — the env-var tracing approach and the chainRun bug fix are both solid additions.
One thing worth flagging for maintainability: the LangSmith env var names are currently spread across four separate places — which vars activate tracing (getLangSmithEnvConfig), which to delete (LANGCHAIN_TRACING_FLAG_VARS), which to read for config (inline reads for apiKey/endpoint/projectName), and which to register as CLI flags (base.ts). These lists need to stay in sync manually, and they've already drifted slightly in this PR — the delete list has 4 vars but the read logic only checks 2.
A single schema object per provider could help keep everything aligned:
const LANGSMITH_ENV_SCHEMA = {
tracingFlags: ['LANGSMITH_TRACING', 'LANGCHAIN_TRACING_V2'],
apiKey: ['LANGSMITH_API_KEY', 'LANGCHAIN_API_KEY'],
endpoint: ['LANGSMITH_ENDPOINT', 'LANGCHAIN_ENDPOINT'],
projectName: ['LANGSMITH_PROJECT', 'LANGCHAIN_PROJECT'],
} as constEvery operation — reading, deleting, CLI flag registration — would derive from this one definition, making drift between the lists impossible.
Co-reviewed by Claude Sonnet 4.6
| * `process.env` so the auto-tracer doesn't emit duplicate top-level runs alongside Flowise's | ||
| * manual `onLLMStart`/`onToolStart` child RunTrees. | ||
| */ | ||
| const LANGCHAIN_TRACING_FLAG_VARS = ['LANGSMITH_TRACING', 'LANGCHAIN_TRACING_V2', 'LANGSMITH_TRACING_V2', 'LANGCHAIN_TRACING'] as const |
There was a problem hiding this comment.
LANGCHAIN_TRACING_FLAG_VARS deletes 4 vars (LANGSMITH_TRACING, LANGCHAIN_TRACING_V2, LANGSMITH_TRACING_V2, LANGCHAIN_TRACING), but getLangSmithEnvConfig only checks LANGSMITH_TRACING and LANGCHAIN_TRACING_V2 as activation flags (line 58). A user with LANGSMITH_TRACING_V2=true would have their var silently deleted with no tracing activated. Either check all 4 as valid activation flags, or remove the unchecked ones from the delete list.
Co-reviewed by Claude Sonnet 4.6
| // emission from this point on; leaving the flags set would letLangChain's auto-tracer fire on | ||
| // every `.invoke()`/`.call()` and produce orphan top-level runs next to the manually-emitted | ||
| // parent/child RunTree. | ||
| for (const k of LANGCHAIN_TRACING_FLAG_VARS) delete process.env[k] |
There was a problem hiding this comment.
This function violates Command-Query Separation — it's named like a query (get...) but mutates global state (process.env) as a side effect. Callers don't expect a read function to delete env vars. Consider splitting into two functions: a pure readLangSmithEnvConfig() that only reads, and a separate clearLangChainTracingFlags() called explicitly by the caller. This also makes the side effect visible at the call site rather than hidden inside a getter.
Co-reviewed by Claude Sonnet 4.6
| } | ||
|
|
||
| /** @internal Test-only: drop cached env-var tracing configs so a subsequent call re-reads env. */ | ||
| export const resetTracingEnvCache = (): void => { |
There was a problem hiding this comment.
resetTracingEnvCache exists only because tracingEnvConfigCache is hidden module-level global state. Exporting a test-only reset function is a sign the design is hard to test cleanly. Consider dependency injection — accept the cache or resolver as a parameter — so tests can pass a fresh instance without needing a reset hook.
Co-reviewed by Claude Sonnet 4.6
| * activate the same provider. Returns a credentialData map for env-injected providers so the loop | ||
| * can skip DB credential loading for those entries. | ||
| */ | ||
| export const applyEnvTracingProviders = ( |
There was a problem hiding this comment.
applyEnvTracingProviders implies mutation, but this is a pure transform — it returns a new object and explicitly does not mutate its input (there's even a test verifying this). A name like mergeEnvTracingProviders or resolveTracingProviders better reflects what it does.
Co-reviewed by Claude Sonnet 4.6
FLOWISE-565
Problem
When langchain is provided with certain environment variables it automatically starts auto tracing requests for Langsmith. See this. This results in Agentflow logging to show single record traces, for every node execution in the agent.
Solution
A new tracingEnv module reads LangSmith settings from the environment (both LANGSMITH_* and legacy LANGCHAIN_* prefixes) and strips the LangChain tracing-flag vars out of process.env on first read so the built-in auto-tracer can no longer fire. applyEnvTracingProviders shapes env configs into the same provider entries the UI produces and merges them into additionalCallbacks and AnalyticHandler.init(), with UI winning on conflict and env-injected providers supplying credentials inline.
This will essentially treat the environment variable as if it were loaded via the UI. Tracing works the exact same.
Verification
Plugged into Langsmith environment. All runs are in there but here is the data.
Before
Before any changes made
Case: Chatflow
Case: Agentflow
After
Case: Agentflow - UI Configured
Case: Agentflow - Env Var Configured
Case: Chatflow - UI Configured
Case: Chatflow - Env Var Configured
Case: Multistep Agent - Env Var Configured
Case: Multistep Agent - UI Configured