Skip to content

Revert: Fix get_caller_pairs userId fallback#141

Merged
nikhilNava merged 2 commits into
microsoft:mainfrom
fpfp100:revert-userid-fallback
May 14, 2026
Merged

Revert: Fix get_caller_pairs userId fallback#141
nikhilNava merged 2 commits into
microsoft:mainfrom
fpfp100:revert-userid-fallback

Conversation

@fpfp100
Copy link
Copy Markdown
Contributor

@fpfp100 fpfp100 commented May 13, 2026

Summary

Reverts #118.

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 aad_object_id → agentic_user_id → frm.id fallback chain) breaks downstream scenarios that expect an AAD object ID.

Test plan

  • Verify existing tests pass
  • Confirm userId is only set from aad_object_id 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
Copy link
Copy Markdown
Member

@rads-1996 rads-1996 left a comment

Choose a reason for hiding this comment

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

LGTM

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 PR #118, restoring the original behavior where userId is set strictly from aad_object_id to satisfy the ingest service's GUID-only requirement.

Changes:

  • Revert get_caller_pairs to yield only frm.aad_object_id for USER_ID_KEY.
  • Remove the four tests that validated the fallback chain behavior.

Reviewed changes

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

File Description
src/microsoft/opentelemetry/a365/hosting/scope_helpers/utils.py Reverts the aad_object_id → agentic_user_id → frm.id fallback to use only aad_object_id.
tests/a365/hosting/scope_helpers/test_scope_helper_utils.py Removes tests that asserted the fallback behavior being reverted.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rads-1996
Copy link
Copy Markdown
Member

@fpfp100 Please fix the lint errors and I can re-approve after that.

requests 2.34.1 changed HeadersType from Mapping to MutableMapping,
which is invariant in its value type. Widen headers annotation from
dict[str, str] to dict[str, str | bytes] to match.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nikhilNava nikhilNava merged commit 9383632 into microsoft:main May 14, 2026
8 checks passed
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.

6 participants