refactor(telemetry): retire UserService; services owns identity - W-22848993#7397
Open
mshanemc wants to merge 25 commits into
Open
refactor(telemetry): retire UserService; services owns identity - W-22848993#7397mshanemc wants to merge 25 commits into
mshanemc wants to merge 25 commits into
Conversation
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Introduces seedTelemetryIdentities Effect (run as a blocking yield in services activate before updateTelemetryUserIds). It owns the stable cliId slot in defaultOrgRef, persisting to globalState via TELEMETRY_GLOBAL_USER_ID and seeding webUserId to UNAUTHENTICATED_USER when missing. - cliTelemetry.getCliId now returns Effect<Option<string>>; cached invocation retained for defensive fallback callers. - updateTelemetryUserIds drops the userId path (cliId is owned by the seed) and switches to ExtensionContextService for consistency. - connectionService.maybeUpdateDefaultOrgRef adapts the Option return with Option.getOrElse to keep the schema slot's string|undefined.
userId on spans was previously the stable cliId; this commit makes it the SOQL User.Id and adds a separate cliId attribute. AppInsights ai.user.id is unaffected (driven by reporter ctor, not span attrs). - applicationInsightsWebExporter: pull cliId from defaultOrgRef and emit alongside userId/webUserId. - o11ySpanExporter: replace userId: cliId with separate userId / cliId span properties. - spanTransformProcessor: top-level span userId attr now sources from defaultOrgRef.userId; cliId becomes its own attr.
Replaces UserService.getTelemetryUserId + getWebTelemetryUserId calls in initializeService and updateReporters with a single getIdentityFromServices method that reads identity from defaultOrgRef via the services extension API. ServicesExtensionNotFoundError and InvalidServicesApiError are logged and recovered to a degraded session (cliId undefined, webUserId UNAUTHENTICATED_USER) so reporter init never throws. When cliId is undefined, a single warning is appended to the extension channel — reporter ctors receive empty string for the cliId-shaped userId arg. Core's workspaceContext.handleTelemetryUpdate stops calling UserService.getTelemetryUserId; refreshAllExtensionReporters is the only remaining responsibility (each reporter pulls its own identity).
UserService and DefaultSharedTelemetryProvider are obsolete now that TelemetryService sources identity from the services extension. Removes: - packages/salesforcedx-utils-vscode/src/services/userService.ts - the matching jest spec - the UserService re-export from utils-vscode index - getSharedTelemetryUserId, hashUserIdentifier, and the local SalesforceVSCodeCoreApi helper from helpers/telemetryUtils.ts (updateUserIDOnTelemetryReporters retained — it is the public refreshAllExtensionReporters surface)
- seedTelemetryIdentities.test.ts: globalState hit, desktop CLI fallback, desktop UUID fallback, web UUID, and UNAUTHENTICATED_USER seeding. - getIdentityFromServices.test.ts: happy path, webUserId fallback, ServicesExtensionNotFoundError + InvalidServicesApiError recovery to degraded identity (Effect.runPromise resolves rather than rejects). - core telemetry test: drop dead globalState assertions; mock createOutputChannel for the degraded-session ChannelService warn path. - services index.test: globalState.update mock now returns a Promise (Effect.promise requires a thenable). - e2e: add orgDesktopWithOrgTest fixture (real org, palette command emits a span) and telemetryIdentitySeeding.desktop.spec asserting cliId + webUserId attrs land on the dumped span.
- drop unused userId field from IdentityFromServices (unused by callers) - hoist FALLBACK_IDENTITY into a shared Effect.succeed binding - drop redundant Schema.UUID decode on crypto.randomUUID() output - flatten nested Option.match in seedTelemetryIdentities Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cliId never exists on web (no CLI present), so the destructure and optional-spread in the web exporter is dead. Stick to userId + webUserId. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…re-userservice-services-owns-telemet
…-services-owns-telemet' into sm/W-22848993-retire-userservice-services-owns-telemet
…re-userservice-services-owns-telemet # Conflicts: # packages/salesforcedx-utils-vscode/src/index.ts # packages/salesforcedx-vscode-core/src/context/workspaceContext.ts
…ding - W-21973088 (#7396) * chore: plan for W-21973088 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(services,metadata): wrap URI in HashableUri instead of extending - Rewrite HashableUri as a structural type + namespace { fromUri, with } wrapping a URI. - Cross-bundle Equal compares uri.toString(); Hash hashes uri.toString(). - Drop subclass methods (.file/.parse/.from/.toUri/.with) and protected ctor. - Re-export HashableUri as value (namespace) from services index. - Migrate all callers in services + metadata to access .uri (RPC, Utils.*, uriToPath). - Update diffTypes structural schema check to look at .uri.scheme. * refactor(metadata,eslint): migrate HashableUri test imports + add no-direct-hashableuri-imports rule * docs(services): add CONTEXT.md with HashableUri glossary * fix(services): tighten HashableUri Equal contract + drive-letter scope - isHashableUriShape requires Equal.symbol method so plain {uri} literals are rejected; keeps Equal/Hash symmetric. - Document scheme==='file' gate on drive-letter normalization. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: minor review cleanups - noDirectHashableUriImports: catch lib/ and relative paths. - sourceDiff catchAll: Effect.sync for fire-and-forget showErrorMessage. - diffTypes isHashableUri: drop redundant Object cast. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(services): add HashableUri unit tests Cover the wrapper's Equal/Hash contracts, Windows drive-letter normalization, and behavior in HashSet/HashMap/Array.dedupe — the collection types it was built for. Validates that case-different drive letters collapse to a single key and that a freshly constructed HashableUri can serve as a lookup key for an equal stored entry. Tests use URI.parse / URI.file directly (no filesystem touch), so Windows CI runs them identically and confirms the drive-letter behavior on its native paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: human pr review --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: plan for W-22857708 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: add local/no-self-barrel-import rule * chore(eslint): enable no-self-barrel-import repo-wide * refactor: replace bare-ancestor barrel imports * fix: detect self-barrel re-exports + tighten rule types Extend rule to handle ExportAllDeclaration and ExportNamedDeclaration (re-exports through self barrel were previously missed). Drop dead TemplateLiteral branch since ImportDeclaration.source is always a string literal per ESTree. Add re-export test coverage. * refactor: extract DocumentContext type to dedicated file Avoid type drift between local DocumentContext copy in htmlLinks.ts and the public export in index.ts. Move the type to services/documentContext.ts so both index re-exports it and htmlLinks imports it directly (no barrel). Also tighten plan wording. * test: fix workspaceContext test mock after import change Update test to mock getOrgShape from workspaceOrgShape module instead of workspaceContextUtils barrel, matching the refactored direct import in workspaceContext.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: plan for W-22773045 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): decouple setupHelpers from migrated apex command label * chore(core): remove unused i18n keys Removes 340 unused keys from i18n.ts (and parity entries from i18n.ja.ts, package.nls.json, package.nls.ja.json): - 326 metadata-type labels (AccessControlPolicy ... XOrgHub) — unused by core src/test; metadata-type labels live in salesforcedx-vscode-metadata. - 14 migrated-command keys (apex_generate_class_text, apex_generate_unit_test_class_text, delete_source_text, deploy_this_source_text, lightning_generate_lwc_text, project_deploy_start_default_org_text, project_deploy_start_ignore_conflicts_default_org_text, project_generate_manifest, project_retrieve_start_default_org_text, project_retrieve_start_ignore_conflicts_default_org_text, retrieve_this_source_text, view_all_changes_text, view_local_changes_text, view_remote_changes_text) — owned by the apex-log, metadata, and lwc extensions post-migration. Audit confirms no dynamic Object.keys(messages) or messages[var] use. * chore(core): drop default dynamic-key pattern for i18n lint Adds a per-file override for core's i18n.ts and i18n.ja.ts that sets local/no-unused-i18n-messages dynamicKeyPatterns to []. Core has no runtime dynamic key lookup, so the global default (^[A-Z][a-zA-Z0-9]*$) was masking dead metadata-type keys. With this override any future PascalCase key must be referenced or the lint flags it. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: plan for W-22846720 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * chore(playwright-vscode-ext): make package publishable * chore(playwright-vscode-ext): add LICENSE and version hook * docs(playwright-vscode-ext): rewrite README for npm consumers * ci(playwright-vscode-ext): add npm publish workflow * fix(playwright-vscode-ext): correct README usage example and drop runtime @salesforce/core dep - README usage example referenced non-existent `test`/`expect`/`vscodePage` exports. Rewrote to use real exports (`createDesktopTest`, `openCommandPalette`, `WORKBENCH`) and noted the desktop fixture is monorepo-internal. - `desktopWorkspace.ts` imported `Global` from `@salesforce/core/global` as a runtime value, defeating the optional-peer-dep contract. Replaced with literal `.sf` constant (mirrors `Global.SF_STATE_FOLDER`). * ci(playwright-vscode-ext): harden publish workflow and version hook - Add concurrency group to prevent racing version bumps on back-to-back pushes. - Pin `salesforcecli/github-workflows` reusable workflow + composite action to SHA (b1f70470) instead of floating `@main` — supply-chain risk for an NPM_TOKEN job. - Drop `commit: develop` from `ncipollo/release-action` so the GH release attaches to the tagged version-bump commit rather than `develop` HEAD. - Version hook: strip prerelease/build suffix before parsing semver and validate; previously `1.0.0-beta` would yield `1.0.NaN`. - Include `CHANGELOG.md` in the npm tarball. * fix: update eslint rule for playwright-vscode-ext package rename Update noDuplicatePlaywrightLocators rule and tests to reference @salesforcedx/playwright-vscode-ext instead of the old @salesforcedx/vscode-playwright package name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: correct path resolution for playwright-vscode-ext eslint rule Fix isInsidePlaywrightPackage check to properly resolve paths and use npm package import for all external consumers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(playwright-vscode-ext): align with publishable-package conventions - package.json: drop engines.vscode; drop @salesforce/core from peerDependencies/peerDependenciesMeta (kept as devDep for type-only imports); drop "./" prefix on main/types; remove explicit CHANGELOG.md from files (npm auto-includes it). - version hook: simplify preVersionGeneration to match the eslint / i18n / soql-common hooks byte-for-byte (modulo package-name strings). - noDuplicatePlaywrightLocators: switch external import path from the broken '@salesforcedx/playwright-vscode-ext/utils/locators' (wrong scope, deep import not exposed by the package's exports field) to '@salesforce/playwright-vscode-ext' (root entry; index.ts already re-exports every locator constant). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
…ap_scraped.json (20260607-134123) (#7407) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…re-userservice-services-owns-telemet # Conflicts: # package-lock.json # packages/playwright-vscode-ext/CHANGELOG.md # packages/playwright-vscode-ext/package.json
…re-userservice-services-owns-telemet
…re-userservice-services-owns-telemet
…re-userservice-services-owns-telemet
…re-userservice-services-owns-telemet
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
2 participants
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.
Summary
cliId(stable) andwebUserId(org-bound sha256) on activation viaseedTelemetryIdentitiesUserService.getTelemetryUserIdplumbingcliId(stable, new attr),userId(SOQL User.Id from org, semantic shift),webUserId(org-bound sha256); AppInsightsai.user.idstill tags cliId via legacy reporter ctorPlan
.claude/plans/W-22848993.md
Reviewer notes
High severity
userId=SOQL Salesforce userId+ separatecliId; AppInsights still tagsai.user.idwith cliId. Same field name carries different identities across telemetry channels. No migration note in CHANGELOG. Recommend documenting the schema split in CHANGELOG and notifying telemetry/dashboard owners before merge. (packages/salesforcedx-vscode-services/src/observability/o11ySpanExporter.ts:86)Medium severity
warnDegradedSessionspamsChannelService.appendLineon cold start for every extension that initializes telemetry before services activates. N copies of same warning across the extension pack. Consider gating the warning behind a once-per-session flag or only logging from core.Low severity
.claude/plans/W-22848993.mdhas multiple verbose lines (lines 21,22,23,32,37,88,95,128,136) that should use sentence fragments per concise skill. Optional polish — does not affect code.Test plan
npm run vscode:package; reset profile; enable span dump;SFDX: List Org Aliases→cliIdpopulated in spans; quit/rerun →cliIdpersists;SFDX: Authorize an Org→webUserIdbecomes sha256,userIdbecomes SOQL User.Id"telemetry seed missing — degraded session"to its channel and reporters initialize with empty cliId (no crash)cliId;webUserId = UNAUTHENTICATED_USERuntil first authWhat issues does this PR fix or reference?
@W-22848993@
🤖 Generated by auto-build pipeline. Original WI: https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00002bYJjbYAG/view