feat(workspace): add load tracing, load-failure issues and debug-load command#10418
feat(workspace): add load tracing, load-failure issues and debug-load command#10418davidfirst wants to merge 11 commits into
Conversation
Code Review by Qodo
1. Scoped id version stripping
|
Code Review by Qodo
1. debug-load hardcoded chalk formatting
|
PR Summary by Qodofeat(workspace): add load tracing, load-failure issues and debug-load command WalkthroughsDescription• Introduces @teambit/harmony.modules.load-trace: an AsyncLocalStorage-based trace context that assigns a unique id to every top-level component/aspect load and propagates it through nested loads as a hierarchical span tree. • Adds stage-level timing spans across WorkspaceComponentLoader, ScopeComponentLoader, WorkspaceAspectsLoader, and the legacy ComponentLoader, recording cache hit/miss attributes per cache (workspace, scope, extensions, legacy). • Adds a new LoadFailures component issue (non-tag-blocking) that surfaces previously-swallowed aspect/env load errors in bit status, attached at aspectLoader.handleExtensionLoadingError, loadCompsAsAspects, and resolveInstalledAspectRecursively. • Adds bit debug-load command: clears the component's cache, loads it fresh inside a trace, and prints a four-section report (load stages/timings, extension-merge source table, resolved env + origin, load issues); supports --json. • Integrates trace prefixes into BitLogger so all log levels emit [trace: ] when a trace is active, replacing the ad-hoc callId in workspace-aspects-loader.ts. • Adds e2e tests (debug-load.e2e.ts, load-failures-issue.e2e.ts) and unit tests for the load-trace module; no behavioral changes to loading itself. Diagramgraph TD
A["CLI: bit debug-load"] --> B["DebugLoadCmd"]
B --> C["WorkspaceComponentLoader"]
C --> D["load-trace module"]
C --> E["WorkspaceAspectsLoader"]
C --> F["ScopeComponentLoader"]
C --> G["Legacy ComponentLoader"]
E --> D
F --> D
G --> D
H["AspectLoaderMain"] --> D
D --> I["BitLogger"]
C --> J["LoadFailures Issue"]
H --> J
J --> K["bit status"]
subgraph Legend
direction LR
_cmd["CLI Command"] ~~~ _loader["Loader"] ~~~ _mod(["New Module"]) ~~~ _issue{{"Issue Type"}}
end
High-Level AssessmentThe following are alternative approaches to this PR: 1. OpenTelemetry SDK
2. Thread traceId through function signatures
Recommendation: The AsyncLocalStorage approach is the right choice for this codebase. The main alternative worth considering is OpenTelemetry, but it would introduce a heavy external dependency and persistent telemetry infrastructure that the design explicitly rules out. The current approach is lightweight, zero-dependency, and perfectly scoped to the observability goals of Phase 1. File ChangesEnhancement (8)
Bug fix (1)
Tests (3)
Documentation (7)
Other (6)
|
|
Code review by qodo was updated up to the latest commit d07de94 |
…rkspace issue; make debug-load private
|
Code review by qodo was updated up to the latest commit bd53d5b |
…-config failure surfacing, gated span logs
|
Code review by qodo was updated up to the latest commit f75d97c |
|
Code review by qodo was updated up to the latest commit c85b920 |
…memoize span path, drop dead code
|
Code review by qodo was updated up to the latest commit e493ad9 |
…ient batch ones the loadCompsAsAspects batch catch could report a load-failure issue for an aspect that loads successfully on a later pass (clearComponentCache resets the self-load gate and the load retries, e.g. after on-the-fly compilation), and could not attribute a batch throw to a single id. drop its reporting; genuine terminal failures still surface via the aspect-loader require-aspects path (never silently re-attempted) and installed-aspect resolution.
|
Code review by qodo was updated up to the latest commit e8882fa |
| /** strip the version suffix from a raw id string (ids here are raw `stringId`s, not ComponentIDs). */ | ||
| const noVersion = (id: string) => id.split('@')[0]; |
There was a problem hiding this comment.
1. Scoped id version stripping 🐞 Bug ≡ Correctness
DebugLoadCmd strips versions using id.split('@')[0], which returns an empty string for
npm-scoped ids like @scope/pkg@1.2.3, causing extension/env origin matching to collapse and
misattribute sources in bit debug-load output.
Agent Prompt
### Issue description
`DebugLoadCmd` uses `id.split('@')[0]` to remove a version suffix. This breaks for npm-scoped package ids (e.g. `@scope/pkg@1.2.3`), where splitting on the first `@` yields an empty string, causing incorrect matching when attributing extension sources and env origin.
### Issue Context
The bug impacts `buildExtensionSources()` and `findEnvOrigin()` matching logic, and can produce wrong/missing origin attribution in `bit debug-load` output.
### Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[12-13]
- scopes/workspace/workspace/debug-load.cmd.ts[102-117]
- scopes/workspace/workspace/debug-load.cmd.ts[123-135]
### Suggested fix
Replace `id.split('@')[0]` with a helper that strips only a trailing version segment by using the **last** `@`:
```ts
const stripVersion = (id: string) => {
const at = id.lastIndexOf('@');
// at===0 covers "@scope/pkg" (no version)
if (at <= 0) return id;
return id.slice(0, at);
};
```
Then use `stripVersion()` in both `buildExtensionSources` and `findEnvOrigin` comparisons.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
reverts the over-correction in a572c2f. removing the loadCompsAsAspects reporting silenced a genuinely-terminal failure: an aspect that throws on require (never recovers) is surfaced via that path, not the require-aspects path, so dropping it made bit status silent about a real error that bit show already shows - an inconsistency. the transient-recovery concern is largely self-correcting (a successful later load replaces the cached component object, clearing the stale issue), whereas under-reporting hid real, terminal errors.
|
Code review by qodo was updated up to the latest commit 395e3de |
Phase 1 (observability) of the component-loading redesign — see
scopes/workspace/workspace/component-loading-redesign.mdfor the full multi-phase plan.@teambit/harmony.modules.load-tracemodule: AsyncLocalStorage-based trace context. Every top-level component/aspect load gets a trace id; nested loads join as child spans, soBIT_LOG=*output reads as a tree (replaces the ad-hoccallIdin the aspects loader).LoadFailurescomponent issue (non-tag-blocking): aspect/env load errors that were previously logged-and-ignored now surface inbit status, attached centrally viaaspectLoader.handleExtensionLoadingErrorandloadCompsAsAspects. Control flow unchanged — previously-swallowed errors are still non-fatal, just visible.bit debug-load <id>command: loads a component fresh with tracing and prints the stage/timing tree, the extension-merge source table (which config source won each extension), the resolved env and its origin, and load issues. Supports--json.e2e:
load-failures-issue.e2e.ts,debug-load.e2e.ts. No behavioral changes to loading itself.