From 95491402a9d2dfc859adcde0c35482b57e6f1b87 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 14 Apr 2026 08:38:40 +0300 Subject: [PATCH] fix(otel,examples): schema-URL merge + stale embedded web dist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent fixes hit while running the cold-topic-explainer demo end-to-end. 1. observability/otel.go — schema URL conflict on Init When SYNAPBUS_OTEL_ENABLED=1, Init() failed with: observability: build resource: conflicting Schema URL: https://opentelemetry.io/schemas/1.26.0 and https://opentelemetry.io/schemas/1.21.0 resource.Default() ships with schema 1.26.0 (newer otel/sdk) but I was passing semconv.SchemaURL from v1.21 into a NewWithAttributes call. resource.Merge rejects that. Fix: use resource.NewSchemaless for the service.* attributes so our side of the merge has no schema URL and slots cleanly into whatever Default provides. ServiceVersion is now only attached when non-empty (avoids a stray service.version="" attribute). Two new regression tests: TestInit_EnabledSucceeds — Enabled=true with all fields set TestInit_EnabledWithNoVersion — Enabled=true with empty version Both point at an unroutable endpoint so the batcher never actually exports; the bug reproduced during Init(), which is all we need. 2. examples/cold-topic-explainer/start.sh — rebuild embedded SPA The Svelte Web UI loaded blank because internal/web/dist/ had a mismatched index.html + stale _app/immutable/entry/ assets (a build had updated index.html but not the chunks, so every asset URL fell through to the SPA HTML fallback and the browser tried to execute HTML as JavaScript). The canonical path is `make web`, but start.sh never ran it, so a working demo depended on the developer having run `make web` first. Fix: start.sh now rebuilds the SPA when web/src is newer than the embedded dist/index.html, using the already-installed web/node_modules (no reinstall). Falls back with a "run make web once" hint when node_modules isn't present. This keeps the fast path fast (~2s vite build after cache warm) and eliminates the silent-stale-dist trap. E2E verified after both fixes: * SYNAPBUS_OTEL_ENABLED=1 start.sh no longer crashes. * `curl /_app/immutable/entry/start.*.js` returns real JavaScript (Content-Type: text/javascript) instead of the index.html fallback. * Chrome-in-MCP navigation to http://localhost:18088/ renders the login form with no SynapBus-originated console errors. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/cold-topic-explainer/start.sh | 26 ++++++++++++++ internal/observability/otel.go | 16 ++++++--- internal/observability/otel_test.go | 47 ++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 4 deletions(-) diff --git a/examples/cold-topic-explainer/start.sh b/examples/cold-topic-explainer/start.sh index bf1a3f9..95cf0bb 100755 --- a/examples/cold-topic-explainer/start.sh +++ b/examples/cold-topic-explainer/start.sh @@ -40,6 +40,32 @@ if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then fi # --- build ------------------------------------------------------------- +# Rebuild the embedded Svelte SPA when web sources are newer than the +# baked dist. Without this, a stale internal/web/dist gets compiled +# into the binary and the Web UI loads a blank page. +if [ -d "$REPO_ROOT/web/node_modules" ]; then + need_web_build=0 + if [ ! -d "$REPO_ROOT/internal/web/dist" ]; then + need_web_build=1 + else + # Any .svelte/.ts source newer than the embedded index.html? + newest_src=$(find "$REPO_ROOT/web/src" -type f \( -name '*.svelte' -o -name '*.ts' -o -name '*.css' \) -print0 2>/dev/null | xargs -0 ls -t 2>/dev/null | head -1) + embedded_index="$REPO_ROOT/internal/web/dist/index.html" + if [ -n "$newest_src" ] && [ "$newest_src" -nt "$embedded_index" ]; then + need_web_build=1 + fi + fi + if [ "$need_web_build" = 1 ]; then + say "rebuilding Svelte SPA (sources newer than embedded dist)" + (cd "$REPO_ROOT/web" && ./node_modules/.bin/vite build) + rm -rf "$REPO_ROOT/internal/web/dist" + cp -r "$REPO_ROOT/web/build" "$REPO_ROOT/internal/web/dist" + fi +else + say "note: web/node_modules missing — using whatever internal/web/dist is embedded" + say " (run 'make web' once from the repo root to bootstrap)" +fi + say "building synapbus binary..." mkdir -p "$BIN_DIR" (cd "$REPO_ROOT" && go build -o "$BIN" ./cmd/synapbus) diff --git a/internal/observability/otel.go b/internal/observability/otel.go index e49643b..faed684 100644 --- a/internal/observability/otel.go +++ b/internal/observability/otel.go @@ -12,6 +12,7 @@ import ( "time" "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" "go.opentelemetry.io/otel/propagation" @@ -105,12 +106,19 @@ func Init(ctx context.Context, cfg Config, logger *slog.Logger) (func(context.Co return nil, fmt.Errorf("observability: create OTLP HTTP exporter: %w", err) } + // Merge our per-run attributes into the SDK's default resource. + // NewSchemaless avoids embedding our own schema URL, so the merge + // never conflicts with whatever version resource.Default() uses + // (which varies between otel/sdk releases — 1.21, 1.26, …). + attrs := []attribute.KeyValue{ + semconv.ServiceName(cfg.ServiceName), + } + if cfg.ServiceVersion != "" { + attrs = append(attrs, semconv.ServiceVersion(cfg.ServiceVersion)) + } res, err := resource.Merge( resource.Default(), - resource.NewWithAttributes(semconv.SchemaURL, - semconv.ServiceName(cfg.ServiceName), - semconv.ServiceVersion(cfg.ServiceVersion), - ), + resource.NewSchemaless(attrs...), ) if err != nil { return nil, fmt.Errorf("observability: build resource: %w", err) diff --git a/internal/observability/otel_test.go b/internal/observability/otel_test.go index a306036..ecde8aa 100644 --- a/internal/observability/otel_test.go +++ b/internal/observability/otel_test.go @@ -3,6 +3,7 @@ package observability_test import ( "context" "testing" + "time" "github.com/synapbus/synapbus/internal/observability" "go.opentelemetry.io/otel" @@ -58,6 +59,52 @@ func TestInit_DisabledIsNoopShutdown(t *testing.T) { } } +// TestInit_EnabledResourceMerge is a regression test for a schema-URL +// conflict: semconv/v1.21 + resource.Default() (which can be 1.26 in +// newer SDKs) used to fail resource.Merge with "conflicting Schema URL". +// Now we use resource.NewSchemaless so the merge is always accepted. +// Points at a dead endpoint so the OTLP client never sends anything. +func TestInit_EnabledSucceeds(t *testing.T) { + cfg := observability.Config{ + Enabled: true, + Endpoint: "127.0.0.1:1", // unroutable; Init must still return successfully + Insecure: true, + ServiceName: "synapbus-test", + ServiceVersion: "0.0.0-test", + } + shutdown, err := observability.Init(context.Background(), cfg, nil) + if err != nil { + t.Fatalf("Init err = %v", err) + } + if shutdown == nil { + t.Fatal("shutdown is nil") + } + // Shutdown is best-effort and may legitimately fail to export + // pending spans against a dead endpoint; surface it as a log not + // a test failure. + shutdownCtx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + defer cancel() + if err := shutdown(shutdownCtx); err != nil { + t.Logf("shutdown non-fatal: %v", err) + } +} + +func TestInit_EnabledWithNoVersion(t *testing.T) { + // ServiceVersion omitted — the attribute list must not include an + // empty string for service.version. + cfg := observability.Config{ + Enabled: true, + Endpoint: "127.0.0.1:1", + Insecure: true, + ServiceName: "synapbus-test", + } + shutdown, err := observability.Init(context.Background(), cfg, nil) + if err != nil { + t.Fatalf("Init err = %v", err) + } + _ = shutdown(context.Background()) +} + func TestInjectTraceContext_NoSpan(t *testing.T) { dst := map[string]string{} observability.InjectTraceContext(context.Background(), dst)