Skip to content

refactor(appkit): let plugins contribute otel span processors - #536

Draft
MarioCadenas wants to merge 1 commit into
mainfrom
telemetry-manager-refactor
Draft

refactor(appkit): let plugins contribute otel span processors#536
MarioCadenas wants to merge 1 commit into
mainfrom
telemetry-manager-refactor

Conversation

@MarioCadenas

Copy link
Copy Markdown
Collaborator

What

Splits TelemetryManager into two phases so a single global OpenTelemetry tracer provider can carry both the OTLP trace exporter and span processors contributed by plugins — instead of each SDK (AppKit's and, e.g., MLflow's) racing to register the process-global provider, where the second registration is silently dropped.

  • initialize() — registers the meter and logger providers eagerly (unchanged timing, at app bootstrap). This is deliberate: OTel's metrics API has no lazy proxy, so a counter/histogram bound against the NoOp meter (as connectors and the cache do in their constructors) stays NoOp for the process lifetime. It no longer registers a tracer provider.
  • registerSpanProcessor(sp) — new; lets a plugin 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() — new; called by core after all plugin setup() completes. Builds the global NodeTracerProvider with the OTLP processor (when configured) plus every contributed processor, and no-ops when nothing needs tracing. NodeTracerProvider.register() reinstates the async-hooks context manager and W3C propagators that NodeSDK.start() used to provide, so span nesting across awaits is preserved.
  • shutdown() now flushes all three providers.

Also swaps the @opentelemetry/sdk-node dependency for @opentelemetry/sdk-trace-node, since the tracer provider is now built directly rather than via NodeSDK.

Why

When both an OTLP endpoint and a plugin tracer (MLflow) are active, they each build a NodeSDK and call .start(), which registers the global tracer provider. OTel's registerGlobal uses allowOverride=false, so the second registration is rejected — and because AppKit initializes first, MLflow's spans silently route to a provider that has no MLflow processor. OTel 2.x removed addSpanProcessor(), so the only way to have both coexist is one provider built after every processor is known — which requires deferring the tracer provider past plugin setup.

Deferring is safe for traces (verified against the OTel source): ProxyTracer rebinds tracers obtained before registration, and no span is emitted during setup(). It is not safe for metrics, hence the eager meter/logger split above.

Scope

This is the enabling infrastructure only. No plugin calls registerSpanProcessor() yet — wiring the MLflow plugin to contribute its processor is a follow-up, gated on mlflow-tracing exposing its MlflowSpanProcessor (it is not exported in 0.1.3).

Verification

  • pnpm --filter=@databricks/appkit typecheck — clean
  • Telemetry suite — 44 pass, including new two-phase tests (contributed-only-no-OTLP routing, idempotent start(), post-start() register ignored, no-op when unconfigured, and a guard that a metric instrument obtained before start() still records)
  • knip — clean; Biome — clean

Draft

Opening as a draft: it's the base layer for the agent-eval tracing work and has no consumer yet.


This pull request and its description were written by Isaac.

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>
@github-actions

Copy link
Copy Markdown
Contributor

📦 Bundle size report

Compared against bundle-size-baseline.json (main).

@databricks/appkit

npm tarball (packed): 843 KB (+2.9 KB) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 873 KB (+3.2 KB) 304 KB (+1.1 KB)
Type declarations 315 KB (+419 B) 109 KB (+225 B)
Source maps 1.7 MB (+6.3 KB) 568 KB (+1.9 KB)
Other 11 KB 3.7 KB
Total 2.9 MB (+9.9 KB) 985 KB (+3.2 KB)
Per-entry composition (own code — deps external (as shipped))
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
. 89 KB (+355 B) 2.5 KB 91 KB (+355 B) external 290 KB (+1.3 KB)
./beta 49 KB (+359 B) 456 B (-2 B) 49 KB (+357 B) external 144 KB (+1.3 KB)
./type-generator 21 KB (+41 B) 0 B 21 KB (+41 B) external 61 KB (+48 B)

Chunks:

Entry Chunk Load Size (gz)
. index.js initial 85 KB
. utils.js initial 4.0 KB
. remote-tunnel-manager.js lazy 2.5 KB
./beta beta.js initial 33 KB
./beta stream-manager.js initial 5.8 KB
./beta wide-event-emitter.js initial 3.2 KB
./beta databricks.js initial 3.0 KB
./beta configuration.js initial 2.1 KB
./beta service-context.js initial 1.3 KB
./beta client.js initial 434 B
./beta client-options.js initial 219 B
./beta supervisor-api.js lazy 192 B
./beta databricks.js lazy 141 B
./beta index.js lazy 123 B
./type-generator index.js initial 21 KB

@databricks/appkit-ui

npm tarball (packed): 342 KB (+351 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).

dist raw gzip
JS (runtime) 390 KB 130 KB
Type declarations 228 KB (+412 B) 83 KB (+360 B)
Source maps 753 KB 248 KB
CSS 16 KB 3.3 KB
Total 1.4 MB (+412 B) 465 KB (+360 B)
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
./js 5.3 KB 49 KB 55 KB 208 KB 14 KB
./js/beta 20 B 0 B 20 B 0 B 0 B
./react 432 KB 49 KB 480 KB 1.3 MB 175 KB
./react/beta 1.0 KB 0 B 1.0 KB 0 B 1.9 KB

Chunks:

Entry Chunk Load Size (gz)
./js index.js initial 5.2 KB
./js chunk initial 120 B
./js apache-arrow lazy 49 KB
./js/beta beta.js initial 20 B
./react index.js initial 430 KB
./react tslib initial 2.1 KB
./react apache-arrow lazy 49 KB
./react/beta beta.js initial 1.0 KB

@github-actions

Copy link
Copy Markdown
Contributor

🤖 AppKit PR bot

🔬 Run evals

Start an eval for this PR from the evals-monitor app: Go to Evals Monitor →

📦 Try this PR's app template

Scaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh auth login — and the Databricks CLI):

gh run download 31790399947 -R databricks/appkit -n appkit-template-0.60.0-pr.baa8725-telemetry-manager-refactor-536 -D appkit-pr-536 \
  && unzip -o "appkit-pr-536/appkit-template-0.60.0-pr.baa8725-telemetry-manager-refactor-536.zip" -d "appkit-pr-536" \
  && databricks apps init --template "appkit-pr-536"

The template pins @databricks/appkit and @databricks/appkit-ui to tarballs built from this branch, so the scaffolded app runs against this PR's code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant