Skip to content

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

Draft
MarioCadenas wants to merge 2 commits into
mainfrom
telemetry-manager-refactor
Draft

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

Conversation

@MarioCadenas

@MarioCadenas MarioCadenas commented Aug 14, 2026

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

Re-run after merging main up to date (past the Biome → oxlint/oxfmt migration in #538):

  • 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; oxlint + oxfmt — clean

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

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle size report

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

@databricks/appkit

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

dist raw gzip
JS (runtime) 940 KB (+34 KB) 327 KB (+9.6 KB)
Type declarations 345 KB (+1.3 KB) 120 KB (+512 B)
Source maps 1.8 MB (+64 KB) 612 KB (+16 KB)
Other 11 KB 3.7 KB
Total 3.1 MB (+99 KB) 1.0 MB (+26 KB)
Per-entry composition (own code — deps external (as shipped))
Entry Initial (gz) Lazy (gz) Total (gz) node_modules (min) Own code (min)
. 94 KB (+5.2 KB) 2.5 KB 96 KB (+5.2 KB) external 307 KB (+19 KB)
./beta 56 KB (+5.3 KB) 457 B 56 KB (+5.3 KB) external 167 KB (+20 KB)
./testing 17 KB (+327 B) 0 B 17 KB (+327 B) external 51 KB (+1.3 KB)
./tsdown 520 B 0 B 520 B external 813 B
./type-generator 21 KB 0 B 21 KB external 61 KB

Chunks:

Entry Chunk Load Size (gz)
. index.js initial 90 KB
. utils.js initial 4.0 KB
. remote-tunnel-manager.js lazy 2.5 KB
./beta beta.js initial 40 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 220 B
./beta supervisor-api.js lazy 192 B
./beta databricks.js lazy 142 B
./beta index.js lazy 123 B
./testing index.js initial 17 KB
./tsdown index.js initial 520 B
./type-generator index.js initial 21 KB

@databricks/appkit-ui

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

dist raw gzip
JS (runtime) 394 KB 132 KB
Type declarations 228 KB (+32 B) 83 KB (+9 B)
Source maps 764 KB 252 KB
CSS 16 KB 3.2 KB
Total 1.4 MB (+32 B) 471 KB (+9 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 481 KB 1.3 MB 177 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

github-actions Bot commented Aug 14, 2026

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 32473946646 -R databricks/appkit -n appkit-template-0.64.0-pr.189d13c-telemetry-manager-refactor-536 -D appkit-pr-536 \
  && unzip -o "appkit-pr-536/appkit-template-0.64.0-pr.189d13c-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.

@MarioCadenas
MarioCadenas marked this pull request as ready for review August 19, 2026 15:11
@MarioCadenas
MarioCadenas requested a review from a team as a code owner August 19, 2026 15:11
@MarioCadenas
MarioCadenas marked this pull request as draft August 21, 2026 12:37
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