-
Notifications
You must be signed in to change notification settings - Fork 36
Propagate broker telemetry to CommonCore through Protocol #1599
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
base: dev
Are you sure you want to change the base?
Conversation
|
Hi @swasti29, can you take a look as well? |
IdentityCore/src/telemetry/request_telemetry/MSIDRequestTelemetryConstants.h
Outdated
Show resolved
Hide resolved
IdentityCore/src/telemetry/request_telemetry/MSIDRequestTelemetryConstants.m
Outdated
Show resolved
Hide resolved
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 introduces a telemetry provider infrastructure to the IdentityCore project, enabling telemetry data collection and propagation throughout the authentication request flow. A new protocol MSIDTelemetryProviding defines telemetry methods, and this telemetry provider is threaded through controller constructors, factory methods, and token requests to enable diagnostics and monitoring.
- New telemetry provider protocol and test implementation files
- Updated controller initialization to accept and store telemetry providers
- Modified factory methods to propagate telemetry throughout the request stack
Reviewed Changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| MSIDTelemetryProviding.h | Defines the telemetry provider protocol interface |
| MSIDTestTelemetryProvider.h/.m | Implements a test stub for telemetry provider |
| MSIDClientTelemetryConstants.h/.m | Defines constants for telemetry keys and values |
| MSIDBaseRequestController.h/.m | Adds telemetry property and constructor parameter |
| MSIDSilentController.h/.m | Updates initializers to accept telemetry parameter |
| MSIDBrokerInteractiveController.h/.m | Updates initializers to accept telemetry parameter |
| MSIDLocalInteractiveController.m | Passes nil telemetry in controller initialization |
| MSIDRequestControllerFactory.h/.m | Adds telemetry parameter to factory methods |
| MSIDSilentTokenRequest.h/.m | Adds telemetry property and uses it to log telemetry data |
| MSIDDefaultSilentTokenRequest.h/.m | Adds telemetry parameter to initializer |
| MSIDTokenRequestProviding.h | Adds telemetry parameter to protocol method |
| MSIDDefaultTokenRequestProvider.m | Passes telemetry to silent token request |
| MSIDLegacyTokenRequestProvider.m | Adds telemetry parameter to protocol implementation |
| MSIDLegacySilentTokenRequest.m | Passes nil telemetry to parent initializer |
| MSIDSSORemoteSilentTokenRequest.m | Passes nil telemetry to parent initializer |
| MSIDHttpRequestErrorHandling.h | Adds telemetry parameter to error handling method |
| MSIDAADRequestErrorHandler.m | Uses telemetry to log HTTP error handling data |
| MSIDHttpRequest.h/.m | Adds telemetry provider property and passes it to error handler |
| Test files | Updated test code to pass telemetry parameter (mostly nil) |
| project.pbxproj | Registers new telemetry files in Xcode project |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
IdentityCore/src/telemetry/client_telemetry/MSIDClientTelemetryConstants.h
Outdated
Show resolved
Hide resolved
IdentityCore/src/telemetry/client_telemetry/MSIDClientTelemetryConstants.m
Outdated
Show resolved
Hide resolved
IdentityCore/src/telemetry/client_telemetry/MSIDTelemetryProviding.h
Outdated
Show resolved
Hide resolved
IdentityCore/src/telemetry/client_telemetry/MSIDTelemetryProviding.h
Outdated
Show resolved
Hide resolved
…yConstants.h Co-authored-by: Copilot <[email protected]>
…ding.h Co-authored-by: Copilot <[email protected]>
…yConstants.m Co-authored-by: Copilot <[email protected]>
…ding.h Co-authored-by: Copilot <[email protected]>
swasti29
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.
Changes looks, most of the changes are due to adding new parameter. Please test end 2 end.
Proposed changes
This pull request introduces support for passing a telemetry provider throughout the request controller stack in the IdentityCore project. The changes ensure that telemetry data can be collected and propagated as part of authentication flows, which will help with diagnostics and monitoring. The update includes new header and implementation files for telemetry provider interfaces and test providers, and modifies controller constructors and factory methods to accept and store telemetry providers.
Telemetry Provider Integration
Added references for new telemetry provider interface and test provider files (
MSIDTelemetryProviding.h,MSIDTestTelemetryProvider.h,MSIDTestTelemetryProvider.m) to the Xcode project, including build phases and groupings. [1] [2] [3] [4] [5] [6] [7] [8]Updated
MSIDBaseRequestControllerand its implementation to accept and store anid<MSIDTelemetryProviding>telemetry property, making it available to all request controllers. [1] [2] [3] [4]Modified controller constructors (e.g.,
MSIDLocalInteractiveController) to accept a telemetry provider parameter and pass it appropriately through initialization. [1] [2]Factory Method Updates
MSIDRequestControllerFactoryinterface and implementation to accept and forward the telemetry provider parameter in all relevant factory methods, ensuring telemetry is consistently propagated to created controllers. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Type of change
Risk
Additional information