feat(config): instrument auto-install-extensions with timing and tracing - #7194
Draft
claude[bot] wants to merge 7 commits into
Draft
feat(config): instrument auto-install-extensions with timing and tracing#7194claude[bot] wants to merge 7 commits into
claude[bot] wants to merge 7 commits into
Conversation
Adds an OTel span and duration attribute around the account-level extension auto-install flow in @netlify/config, which previously had no timing instrumentation at all. Also wires up @netlify/opentelemetry-sdk-setup in the netlify-config CLI entrypoint so trace context passed in by Buildbot (traceId/traceParentSpanId/traceFlags/baggage) gets picked up and this process's spans are stitched into the same trace, and switches the function's console.log/console.error calls to @netlify/config's own buffered logger so they no longer leak onto stdout (which is reserved for the CLI's JSON output).
Contributor
|
This pull request adds or modifies JavaScript ( |
Contributor
e18e dependency analysisNo dependency warnings found. |
claude
Bot
changed the title
@netlify/build
@netlify/build-info
@netlify/cache-utils
@netlify/config
@netlify/edge-bundler
@netlify/functions-utils
@netlify/git-utils
@netlify/headers-parser
@netlify/api
@netlify/nock-udp
@netlify/opentelemetry-sdk-setup
@netlify/opentelemetry-utils
@netlify/redirect-parser
@netlify/run-utils
@netlify/zip-it-and-ship-it
commit: |
knip flags @netlify/opentelemetry-sdk-setup as an unused optional peerDependency in packages/config because it's only resolved via a dynamic import() at runtime (defensively, since it's optional), which knip's static analysis can't trace. Add it to ignoreDependencies for the packages/config workspace, matching the identical existing entry for packages/build.
packages/config/package.json added @opentelemetry/api as a direct dependency and @netlify/opentelemetry-sdk-setup as an optional peer dependency, but package-lock.json was never regenerated, so npm ci had a stale lockfile. Add the missing dependency/peerDependencies entries to the packages/config lockfile node (the version itself was already resolved at the root via packages/build's identical devDependency, so no new package needed downloading).
# Conflicts: # package-lock.json
…ension-install-timing # Conflicts: # package-lock.json
The prior merge of main manually resolved a package-lock.json conflict
in a way that left it internally inconsistent: many packages'
optionalDependencies (e.g. @rollup/rollup-*, @esbuild/*, @nx/nx-*,
@oxc-parser/binding-*) referenced platform variants that had no
corresponding top-level lockfile entry. npm <11 tolerates this, but
the npm 11.x bundled with Node 24 (used across this repo's CI) treats
it as fatal ("Missing: X from lock file") and refuses `npm ci`,
breaking the formatting, knip, e2e, test, and test-build checks
identically since they all install with `npm ci` first.
Regenerated the lockfile with npm 11 via
`npm install --package-lock-only` so it is complete and consistent,
then re-merged main (which advanced again during investigation) and
regenerated once more. Verified `npm ci`, build, knip, format:ci, and
lint:ci all pass locally with npm 11.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDdYBYHPj9CF5vG4WAX5EC
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.
Requested by Philippe Serhal · Slack thread
Summary
The account-level "auto-install required extensions" flow (
handleAutoInstallExtensions()inpackages/config/src/utils/extensions/auto-install-extensions.ts) had zero timinginstrumentation, and the
@netlify/configCLI process it runs in never picked up trace contextfrom Buildbot, so none of this work showed up in traces or duration metrics.
This PR:
auto-install-extensions), recording aduration_msattribute and any thrown exception.@netlify/opentelemetry-sdk-setupin thenetlify-configCLI entrypoint(
packages/config/src/bin/main.js), so trace context passed in by Buildbot gets picked up andthis process's spans are correctly parented under the same trace as the rest of the build.
Buildbot passes this as
--traceId/--traceParentSpanId/--traceFlags/--tracingBaggageFilePath(see companion PR netlify/buildbot#4442); these are mapped onto thetraceId/parentSpanId/traceFlags/baggageFilePathfieldsopentelemetry-sdk-setupexpects.
console.log/console.errorcalls with@netlify/config's ownbuffered logger (
log/logWarningfromsrc/log/logger.js). This is also a smallcorrectness fix:
console.logwrites straight to the process's real stdout, which this CLIreserves for its JSON result — a raw
console.logline here was landing in the same stream asthe JSON output.
How
packages/config/src/utils/extensions/auto-install-extensions.ts: added aconfigtracer,wraps the install logic (past the early precondition checks) in a
tracer.startActiveSpan('auto-install-extensions', ...), recordsduration_msandextensions.installed_count, and swaps the raw console calls forlog/logWarning.packages/config/src/main.ts: threadslogsthrough tohandleAutoInstallExtensions().packages/config/src/bin/main.js: parses the trace-context flags, callsstartTracing()/stopTracing()from@netlify/opentelemetry-sdk-setup(an optional peerdependency, so this is a no-op if it isn't installed or no trace context is passed), and runs
resolveConfig()inside that root context.packages/config/src/bin/flags.ts: added the four hidden CLI flags Buildbot now passes.packages/config/package.json: added@opentelemetry/apias a dependency and@netlify/opentelemetry-sdk-setupas an optional peer dependency, mirroring howpackages/builddepends on the same tracing packages.packages/config/tests/extensions/tests.js: two tests that trigger an actual install nowextract the JSON result from
outputrather thanJSON.parse-ing it directly, since theinstall now (correctly) logs to the buffered stderr that
runConfigServerprepends tooutputin these tests.Not done / follow-ups
package.json/package-lock.json: the new@opentelemetry/apidependency needs a lockfileupdate; I couldn't produce a clean one in my sandbox (an unrelated pre-existing lockfile entry,
playwright@1.61.0underpackages/build-info, is no longer resolvable from the registrymirror available to me, so a full
npm installfails there regardless of this change). Aregular
npm installin a normal dev/CI environment should pick up the new dependency fine.For us to review and ship your PR efficiently, please perform the following steps:
Generated by Claude Code