Skip to content

Commit 6bd258b

Browse files
siddhijainCopilot
andauthored
Move ests telemetry related code behind a feature flag AB#3352490, Fixes AB#3352490 (#2742)
Emitting eSTS telemetry is an expensive operation that impacts the performance of calls. Therefore, we are placing it behind a feature flag to avoid executing telemetry-related operations. [AB#3352490](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3352490) --------- Co-authored-by: Copilot <[email protected]>
1 parent 566f14a commit 6bd258b

File tree

4 files changed

+113
-15
lines changed

4 files changed

+113
-15
lines changed

changelog.txt

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,17 @@
11
vNext
22
----------
3-
4-
Version 22.0.3
5-
----------
6-
- [PATCH] Fix DualScreenActivity theme color constant (#2737)
7-
8-
Version 22.0.1
9-
----------
10-
- [PATCH] Handling https scheme for device enrollment link (#2732)
11-
- [PATCH] Remove System Gesture Inset (#2730)
12-
13-
Version 22.0.0
14-
----------
3+
- [MINOR] Move ests telemetry behind feature flag (#2742)
154
- [MINOR] Update Broker ATS flow for Resource Account (#2704)
165
- [PATCH] Handle BadTokenException gracefully in CBA dialogs (#2731)
176
- [MINOR] Enable ShouldPreserveWebViewFlowOnSslError in Common (#2741)
187
- [MINOR] Add AAD authority validation for DUNA flow (#2740)
198
- [PATCH] Fix DualScreenActivity theme color constant (#2737)
209
- [MINOR] [SDL Assessment task] Secure webview settings (#2715)
2110

11+
Version 22.0.3
12+
----------
13+
- [PATCH] Fix DualScreenActivity theme color constant (#2737)
14+
2215
Version 22.0.1
2316
----------
2417
- [PATCH] Handling https scheme for device enrollment link (#2732)

common4j/src/main/com/microsoft/identity/common/java/eststelemetry/EstsTelemetry.java

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,14 @@
2626
import com.microsoft.identity.common.java.commands.ICommandResult;
2727
import com.microsoft.identity.common.java.exception.BaseException;
2828
import com.microsoft.identity.common.java.exception.ServiceException;
29+
import com.microsoft.identity.common.java.flighting.CommonFlight;
30+
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
2931
import com.microsoft.identity.common.java.interfaces.INameValueStorage;
3032
import com.microsoft.identity.common.java.interfaces.IPlatformComponents;
3133
import com.microsoft.identity.common.java.logging.DiagnosticContext;
3234
import com.microsoft.identity.common.java.logging.Logger;
35+
import com.microsoft.identity.common.java.opentelemetry.AttributeName;
36+
import com.microsoft.identity.common.java.opentelemetry.SpanExtension;
3337
import com.microsoft.identity.common.java.result.ILocalAuthenticationResult;
3438
import com.microsoft.identity.common.java.util.StringUtil;
3539
import com.microsoft.identity.common.java.util.ported.InMemoryStorage;
@@ -64,6 +68,82 @@ public class EstsTelemetry {
6468
private final INameValueStorage<CurrentRequestTelemetry> mTelemetryMap;
6569
private final INameValueStorage<Set<FailedRequest>> mSentFailedRequests;
6670

71+
/**
72+
* A no-op implementation of EstsTelemetry that silently ignores all operations.
73+
* Used when telemetry is disabled via feature flag.
74+
*/
75+
private static class NoopEstsTelemetry extends EstsTelemetry {
76+
NoopEstsTelemetry() {
77+
super(new InMemoryStorage<CurrentRequestTelemetry>(), new InMemoryStorage<Set<FailedRequest>>());
78+
}
79+
80+
@Override
81+
public synchronized void setUp(@NonNull LastRequestTelemetryCache lastRequestTelemetryCache) {
82+
// No-op
83+
}
84+
85+
@Override
86+
public synchronized void setUp(@NonNull IPlatformComponents platformComponents) {
87+
// No-op
88+
}
89+
90+
@Override
91+
public void initTelemetryForCommand(@NonNull final ICommand<?> command) {
92+
// No-op
93+
}
94+
95+
@Override
96+
public void emit(@Nullable Map<String, String> telemetry) {
97+
// No-op
98+
}
99+
100+
@Override
101+
public void emit(@Nullable String key, String value) {
102+
// No-op
103+
}
104+
105+
@Override
106+
public void emit(@Nullable String key, int value) {
107+
// No-op
108+
}
109+
110+
@Override
111+
public void emit(@Nullable String key, long value) {
112+
// No-op
113+
}
114+
115+
@Override
116+
public void emit(@Nullable String key, boolean value) {
117+
// No-op
118+
}
119+
120+
@Override
121+
public void emitApiId(@Nullable String apiId) {
122+
// No-op
123+
}
124+
125+
@Override
126+
public void emitForceRefresh(boolean forceRefresh) {
127+
// No-op
128+
}
129+
130+
@Override
131+
public synchronized void flush(@NonNull ICommand<?> command, @NonNull ICommandResult commandResult) {
132+
// No-op
133+
}
134+
135+
@NonNull
136+
@Override
137+
public Map<String, String> getTelemetryHeaders() {
138+
return Collections.emptyMap();
139+
}
140+
141+
@Override
142+
public synchronized void clear() {
143+
// No-op
144+
}
145+
}
146+
67147
/**
68148
* A supplemental cache that can used to store telemetry that is captured outside of the
69149
* DiagnosticContext. We have lots of code that is executed outside of a DiagnosticContext i.e.
@@ -89,12 +169,26 @@ public class EstsTelemetry {
89169
/**
90170
* Get an instance of {@link EstsTelemetry}. This method will return an existing
91171
* instance of EstsTelemetry or create and return a new instance if the existing instance is null.
172+
* If telemetry is disabled via the SKIP_ESTS_TELEMETRY feature flag, a NoopEstsTelemetry instance
173+
* will be returned that safely ignores all telemetry operations.
92174
*
93175
* @return EstsTelemetry object instance
94176
*/
95177
public static synchronized EstsTelemetry getInstance() {
96178
if (sEstsTelemetryInstance == null) {
97-
sEstsTelemetryInstance = new EstsTelemetry();
179+
// Check feature flag to decide whether to return NoopEstsTelemetry or real EstsTelemetry
180+
final boolean shouldSkipEstsTelemetry =
181+
CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.SKIP_ESTS_TELEMETRY);
182+
183+
if (shouldSkipEstsTelemetry) {
184+
Logger.verbose(TAG, "SKIP_ESTS_TELEMETRY feature flag enabled, using NoopEstsTelemetry");
185+
SpanExtension.current().setAttribute(AttributeName.skipped_ests_telemetry.name(), true);
186+
sEstsTelemetryInstance = new NoopEstsTelemetry();
187+
} else {
188+
Logger.verbose(TAG, "SKIP_ESTS_TELEMETRY feature flag disabled, using standard EstsTelemetry");
189+
SpanExtension.current().setAttribute(AttributeName.skipped_ests_telemetry.name(), false);
190+
sEstsTelemetryInstance = new EstsTelemetry();
191+
}
98192
}
99193

100194
return sEstsTelemetryInstance;

common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,12 @@ public enum CommonFlight implements IFlightConfig {
155155
/**
156156
* Flight to enable WebView security settings to prevent unauthorized access.
157157
*/
158-
ENABLE_WEBVIEW_SECURITY_SETTINGS("EnableWebViewSecuritySettings", false);
158+
ENABLE_WEBVIEW_SECURITY_SETTINGS("EnableWebViewSecuritySettings", false),
159+
160+
/**
161+
* Flight to skip ests telemetry.
162+
*/
163+
SKIP_ESTS_TELEMETRY("SkipEstsTelemetry", false);
159164

160165
private String key;
161166
private Object defaultValue;

common4j/src/main/com/microsoft/identity/common/java/opentelemetry/AttributeName.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,12 @@ public enum AttributeName {
432432
/**
433433
* Records the time (in milliseconds) spent on flight check for webcp.
434434
*/
435-
web_cp_flight_get_time
435+
web_cp_flight_get_time,
436+
437+
/**
438+
* Indicates if ests telemetry was skipped.
439+
*/
440+
skipped_ests_telemetry
441+
436442

437443
}

0 commit comments

Comments
 (0)