Skip to content

Commit 0a72116

Browse files
p3dr0rvCopilot
andauthored
Emit ipc_strategy attribute and refactor execute flow, Fixes AB#3571861 (#3124)
- Set the existing AttributeName.ipc_strategy on the DeviceRegistrationIpc span when an IPC strategy succeeds, making it easy to identify the winning strategy. - Refactor communicateProtocolWithStrategy into sendProtocolRequestToBroker to separate bundle packing from IPC transport. Packing now happens once before the strategy loop instead of on every retry. - Move cache update and unpack back into execute for clarity. - [AB#3571861](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3571861) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent e536e1e commit 0a72116

2 files changed

Lines changed: 38 additions & 29 deletions

File tree

changelog.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
vNext
22
----------
3+
- [PATCH] Emit ipc_strategy telemetry attribute for successful device registration IPC strategy and refactor execute flow to pack protocol request once before strategy retries (#3124)
34
- [PATCH] Fix Edge browser selection on devices where Microsoft Edge is the default browser: add the rotated Edge signing certificate hash to the Edge BrowserDescriptor and accept multi-signer browsers when any signature intersects the safelist, instead of requiring strict set-equality (resolves MSAL #2414)
45
- [MINOR] Refactor Auth Tab integration to use provider-based strategy selection. Adds AuthTabStrategyProvider and BrowserLaunchStrategy with Custom Tabs fallback. Compatible with androidx.browser:browser:1.7.0.
56
- [MINOR] Add provisionResourceAccountCredentials API to DeviceRegistrationClientApplication with V0 protocol params/response and add IPPhone to AppRegistry (#3086)

common/src/main/java/com/microsoft/identity/deviceregistration/AndroidDeviceRegistrationClientController.java

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,26 @@ public final byte[] execute(@NonNull final IDeviceRegistrationProtocolParameters
141141
span.setAttribute(AttributeName.device_registration_protocol_name.name(), protocolParameters.getProtocolName());
142142
span.setAttribute(AttributeName.calling_package_name.name(), mCallerPackageName);
143143
span.setAttribute(AttributeName.active_broker_package_name.name(), mActiveBrokerPackageName);
144+
final Bundle protocolParametersBundle;
145+
try {
146+
protocolParametersBundle = mProtocolPacker.pack(protocolParameters);
147+
} catch (final Throwable throwable) {
148+
Logger.error(methodTag, "Serialization error while packing the protocol", throwable);
149+
throw new DeviceRegistrationException(
150+
DeviceRegistrationException.INTERNAL_ERROR_CODE,
151+
DeviceRegistrationException.SERIALIZATION_ERROR_MESSAGE,
152+
throwable
153+
);
154+
}
144155
for (final IIpcStrategy strategy : mIpcStrategies) {
156+
Logger.info(methodTag, "Executing " + protocolParameters.getProtocolName() + " with strategy: " + strategy.getType());
145157
try {
146-
byte[] protocolResult = communicateProtocolWithStrategy(protocolParameters, strategy);
158+
final Bundle brokerResultBundle = sendProtocolRequestToBroker(protocolParametersBundle, strategy);
147159
setIpcStrategyTelemetryAttributes(strategy.getType(), "OK");
160+
span.setAttribute(AttributeName.ipc_strategy.name(), strategy.getType().name());
148161
span.setStatus(StatusCode.OK);
149-
return protocolResult;
162+
mCacheUpdater.updateCachedActiveBrokerFromResultBundle(brokerResultBundle);
163+
return mProtocolPacker.unpackData(brokerResultBundle);
150164
} catch (final BrokerCommunicationException communicationException) {
151165
setIpcStrategyTelemetryAttributes(strategy.getType(), communicationException.getMessage());
152166
// Fails to communicate to the broker. Try next strategy in list.
@@ -172,46 +186,40 @@ public final byte[] execute(@NonNull final IDeviceRegistrationProtocolParameters
172186
}
173187

174188
/**
175-
* Communicates the protocol associated with the given parameters using the provided strategy.
189+
* Sends the serialized device registration protocol parameters to the active broker over the
190+
* provided IPC strategy and returns the raw response bundle.
191+
* <p>
192+
* The parameters bundle is wrapped in a {@link BrokerOperationBundle} tagged with the
193+
* {@link com.microsoft.identity.common.internal.broker.ipc.BrokerOperationBundle.Operation#DEVICE_REGISTRATION_OPERATIONS}
194+
* operation and targeted at {@link #mActiveBrokerPackageName}. The returned bundle is the
195+
* unmodified payload produced by the broker; unpacking it into a protocol response is the
196+
* caller's responsibility (see {@link AndroidDeviceRegistrationProtocolPacker#unpackData}).
197+
* </p>
176198
*
177-
* @param protocolParameters protocol parameters to execute.
178-
* @param ipcStrategy strategy to be invoked.
179-
* @return a serialized protocol response.
180-
* @throws BrokerCommunicationException if the strategy fails.
199+
* @param protocolParametersBundle serialized protocol parameters to send to the broker.
200+
* @param ipcStrategy IPC strategy used to deliver the request to the broker.
201+
* @return the non-null response {@link Bundle} returned by the broker.
202+
* @throws BrokerCommunicationException if the IPC strategy fails to communicate with the
203+
* broker, allowing the caller to fall back to the next available strategy.
204+
* @throws ClientException with {@link ClientException#INVALID_BROKER_BUNDLE} if the broker
205+
* returns a {@code null} bundle.
206+
* @throws BaseException for any other failure surfaced by the underlying IPC strategy.
181207
*/
182208
@NonNull
183-
private byte[] communicateProtocolWithStrategy(
184-
@NonNull final IDeviceRegistrationProtocolParameters protocolParameters,
209+
private Bundle sendProtocolRequestToBroker(
210+
@NonNull final Bundle protocolParametersBundle,
185211
@NonNull final IIpcStrategy ipcStrategy) throws BaseException {
186-
final String methodTag = TAG + ":executeProtocolWithStrategy";
187-
Logger.info(methodTag, "Executing " + protocolParameters.getProtocolName()
188-
+ " with strategy: " + ipcStrategy.getType());
189-
final Bundle protocolParametersBundle;
190-
try {
191-
protocolParametersBundle = mProtocolPacker.pack(protocolParameters);
192-
} catch (final Throwable throwable) {
193-
Logger.error(methodTag, "Serialization error while packing the protocol", throwable);
194-
throw new DeviceRegistrationException(
195-
DeviceRegistrationException.INTERNAL_ERROR_CODE,
196-
DeviceRegistrationException.SERIALIZATION_ERROR_MESSAGE,
197-
throwable
198-
);
199-
}
200212
final Bundle protocolResultBundle = ipcStrategy.communicateToBroker(
201213
new BrokerOperationBundle(
202214
DEVICE_REGISTRATION_OPERATIONS,
203215
mActiveBrokerPackageName,
204216
protocolParametersBundle
205217
)
206218
);
207-
208219
if (protocolResultBundle == null) {
209-
throw new ClientException(INVALID_BROKER_BUNDLE, "Broker Result not returned from Broker.");
220+
throw new ClientException(INVALID_BROKER_BUNDLE, "Broker returned null bundle");
210221
}
211-
212-
mCacheUpdater.updateCachedActiveBrokerFromResultBundle(protocolResultBundle);
213-
214-
return mProtocolPacker.unpackData(protocolResultBundle);
222+
return protocolResultBundle;
215223
}
216224

217225
/**

0 commit comments

Comments
 (0)