-
Notifications
You must be signed in to change notification settings - Fork 46
Add callback for OneAuth for measuring Broker Discovery Client Perf, Fixes AB#3416279 #2796
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
❌ Work item link check failed. Description does not contain AB#{ID}. Click here to Learn more. |
759b955 to
623021a
Compare
...n/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt
Outdated
Show resolved
Hide resolved
623021a to
1989713
Compare
|
✅ Work item link check complete. Description contains link AB#3416279 to an Azure Boards work item. |
5acb680 to
ecd9435
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new telemetry callback mechanism to the Broker Discovery Client to enable OneAuth to measure performance metrics during broker discovery operations. The changes introduce a new interface IBrokerDiscoveryClientTelemetryCallback with methods to track timing for various discovery stages, and adds an overloaded getActiveBroker method that accepts this callback as a parameter.
Key Changes
- Added
IBrokerDiscoveryClientTelemetryCallbackinterface defining 6 callback methods for tracking performance metrics during broker discovery - Extended
IBrokerDiscoveryClientinterface with newgetActiveBrokeroverload accepting the telemetry callback - Implemented telemetry callback support in
BrokerDiscoveryClientwith timing measurements for cache reads, package validation, and broker queries - Added stub implementation in
LegacyBrokerDiscoveryClientthat ignores the callback (as documented)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| IBrokerDiscoveryClientTelemetryCallback.kt | New interface defining callback methods for broker discovery performance telemetry |
| IBrokerDiscoveryClient.kt | Added new getActiveBroker overload accepting telemetry callback parameter |
| BrokerDiscoveryClient.kt | Implemented telemetry callback invocations with timing measurements throughout broker discovery flow |
| LegacyBrokerDiscoveryClient.kt | Added stub implementation of new overload that ignores callback |
| changelog.txt | Added changelog entry for this minor feature addition |
...ft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClientTelemetryCallback.kt
Show resolved
Hide resolved
.../com/microsoft/identity/common/internal/activebrokerdiscovery/LegacyBrokerDiscoveryClient.kt
Outdated
Show resolved
Hide resolved
.../java/com/microsoft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClient.kt
Outdated
Show resolved
Hide resolved
...n/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt
Outdated
Show resolved
Hide resolved
...n/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt
Outdated
Show resolved
Hide resolved
...ft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClientTelemetryCallback.kt
Outdated
Show resolved
Hide resolved
...ft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClientTelemetryCallback.kt
Outdated
Show resolved
Hide resolved
...ft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClientTelemetryCallback.kt
Outdated
Show resolved
Hide resolved
...ft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClientTelemetryCallback.kt
Outdated
Show resolved
Hide resolved
...n/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt
Show resolved
Hide resolved
612364e to
0d981d0
Compare
0d981d0 to
3ebfb0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
.../java/com/microsoft/identity/common/internal/activebrokerdiscovery/IBrokerDiscoveryClient.kt
Show resolved
Hide resolved
...n/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt
Show resolved
Hide resolved
nickbopp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking for a tag to measure time waiting for lock, and then we're good. Chase and I looked together so I'm signing off for both of us.
AB#3416279
This will allow OneAuth to wire telemetry into BrokerDiscoveryClient.getActiveBroker()