Skip to content

Revert: Fix getCallerBaggagePairs userId fallback#253

Open
fpfp100 wants to merge 2 commits into
mainfrom
revert-userid-fallback
Open

Revert: Fix getCallerBaggagePairs userId fallback#253
fpfp100 wants to merge 2 commits into
mainfrom
revert-userid-fallback

Conversation

@fpfp100
Copy link
Copy Markdown
Contributor

@fpfp100 fpfp100 commented May 13, 2026

Summary

Reverts #250.

The ingest service only accepts GUIDs for user.id. Any non-GUID value gets replaced with an all-zeros GUID (00000000-0000-0000-0000-000000000000). Allowing non-GUID values as userId (via the aadObjectId → agenticUserId → from.id fallback chain) breaks downstream scenarios that expect an AAD object ID.

Test plan

  • Verify existing tests pass
  • Confirm userId is only set from aadObjectId again

🤖 Generated with Claude Code

Note

The original PR will not take effect since UPN (non-GUID) values are always ignored by the ingestion service.

Copilot AI review requested due to automatic review settings May 13, 2026 21:28
@fpfp100 fpfp100 requested review from a team as code owners May 13, 2026 21:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Reverts the previously-added userId fallback chain in OpenTelemetry baggage generation so user.id is only populated from aadObjectId, preventing non-GUID values from being sent to ingest.

Changes:

  • Update getCallerBaggagePairs to set user.id exclusively from from.aadObjectId
  • Remove/adjust tests that asserted fallback to agenticUserId / from.id
  • Update Jest config by removing the @microsoft/agents-a365-observability-hosting moduleNameMapper entry

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
tests/observability/extension/hosting/TurnContextUtils.test.ts Removes tests that asserted userId fallback behavior
tests/observability/extension/hosting/BaggageBuilderUtils.test.ts Updates expectation so user.id is not emitted when aadObjectId is absent
tests/jest.config.cjs Removes Jest moduleNameMapper for the hosting package
packages/agents-a365-observability-hosting/src/utils/TurnContextUtils.ts Reverts user.id to only use aadObjectId

Comment thread tests/observability/extension/hosting/TurnContextUtils.test.ts
Comment thread tests/observability/extension/hosting/BaggageBuilderUtils.test.ts
Comment thread tests/jest.config.cjs
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.

4 participants