feat(appkit): plugin-contributed otel span processors + single-provider agent mlflow tracing - #545
Open
MarioCadenas wants to merge 4 commits into
Open
feat(appkit): plugin-contributed otel span processors + single-provider agent mlflow tracing#545MarioCadenas wants to merge 4 commits into
MarioCadenas wants to merge 4 commits into
Conversation
Split TelemetryManager into two phases so a single global tracer provider can carry both the OTLP exporter and plugin-contributed processors (e.g. MLflow), instead of each SDK racing to register the global provider. - initialize() registers the meter and logger providers eagerly, because OTel's metrics API has no lazy proxy: an instrument bound against the NoOp meter stays NoOp for the process lifetime. - registerSpanProcessor() lets plugins contribute a span processor during setup(); ignored with a warning after start() since a started provider's processors are immutable in OTel JS 2.x. - start() (called after plugin setup) builds the global NodeTracerProvider with the OTLP processor plus every contributed one, and no-ops when nothing needs tracing. Deferring is safe: ProxyTracer rebinds tracers obtained earlier and no span is emitted during setup. Swaps the @opentelemetry/sdk-node dependency for @opentelemetry/sdk-trace-node, since the tracer provider is now built directly instead of via NodeSDK. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
The mlflow-tracing SDK's init() stands up and globally registers its own OpenTelemetry tracer provider. OTel's registerGlobal is allowOverride=false, so when an OTLP endpoint and agent tracing are both active, AppKit's provider and mlflow's race for the global slot and one exporter is silently dropped. Instead of calling init() (which self-registers), the agents plugin now builds mlflow's span processor itself and contributes it to AppKit's single provider via TelemetryManager.registerSpanProcessor() during setup(). mlflow's global config is seeded lazily on first trace (after start()), so init()'s own registration harmlessly loses the already-claimed global slot. A GatedMlflowSpanProcessor keeps the contributed processor inert until config is seeded, so AppKit's own spans created before the first agent turn don't hit mlflow's getConfig() throw. The processor/exporter are deep-imported from mlflow-tracing internals (no public export in 0.1.3), pinned and guarded by a tripwire test. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
…rom turn 1 The mlflow span processor rides AppKit's shared OTel provider, so it saw every span — including the exporters' own outbound HTTP calls. Each trace upload became a new span to trace and upload: a feedback loop that flooded the experiment and could wedge the process. - Drop parentless CLIENT spans in the gated processor. An outgoing request made outside any agent turn (mlflow/OTLP shipping a trace) is exactly the loop's return edge; spans inside a real request tree keep their parent (or are the incoming SERVER root), so agent turns are untouched. - Seed mlflow's config on the "setup:complete" lifecycle event — after TelemetryManager.start(), before the server serves — instead of lazily on the first trace. The first turn's request-root span is then already forwarded, so that turn assembles into a trace instead of being dropped as a cold-start. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
MarioCadenas
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stacked on #536 (the
registerSpanProcessorseam). Wires the agents plugin's MLflow tracing to contribute its span processor to AppKit's single OTel tracer provider, instead of lettingmlflow-tracing'sinit()stand up and globally register a competing provider.Why
mlflow-tracing0.1.3'sinit()builds its ownNodeSDKand registers the global tracer provider. OTel'sregisterGlobalisallowOverride=false, so when both an OTLP endpoint and agent tracing (MLFLOW_EXPERIMENT_ID) are active, AppKit's provider and mlflow's race — whichever registers second is silently dropped, losing either MLflow export or OTLP. This is the bug #536's infrastructure was built to fix; this PR supplies the consumer.How
initAgentTracing()(agents pluginsetup()) now builds mlflow'sMlflowSpanProcessoritself and contributes it viaTelemetryManager.registerSpanProcessor()— noinit()here, so no competing registration.mlflow.init()is called lazily on the first trace (afterTelemetryManager.start()), purely to seed mlflow's global config; its own provider registration harmlessly loses the already-claimed global slot.GatedMlflowSpanProcessorkeeps the contributed processor inert until config is seeded, so AppKit's own spans (HTTP, analytics) created before the first agent turn don't trip mlflow'sgetConfig()throw. AWeakSetkeepsonEndbalanced.mlflow-tracinginternals (0.1.3 exposes no publicSpanProcessor), pinned to the exact version and guarded by a tripwire test that fails loudly if a version bump renames them.Verification
pnpm --filter=@databricks/appkit typecheck— cleanGatedMlflowSpanProcessorgating), including a tripwire that constructs the real deep-imported symbolsoxlint+oxfmt— cleaninit()seeds global config even when its provider registration loses the global slot, and (b) that duplicate registration is non-fatal.Manual verification pending: dev-playground with both
MLFLOW_EXPERIMENT_IDandOTEL_EXPORTER_OTLP_ENDPOINTset — confirm agent traces reach MLflow and OTLP simultaneously.Follow-up
The clean long-term fix is upstream:
mlflow-tracingexposing a config-only setup (init({ register: false })) or a public span-processor export, which would remove the deep imports.This pull request and its description were written by Isaac.