Skip to content

Commit 189a2d0

Browse files
jeet1995Copilot
andcommitted
Fix partitionLevelCircuitBreakerCfg missing from CosmosDiagnostics clientCfgs
The partitionLevelCircuitBreakerCfg field disappeared from the clientCfgs section of CosmosDiagnostics when a customer explicitly enabled Per-Partition Circuit Breaker (PPCB) client-side. Root cause: the diagnostics write was coupled to the Per-Partition Automatic Failover (PPAF) initialization path, so the field was only populated when the service mandated PPAF, not when PPCB was configured client-side. Fix: move the diagnostics write into initializePerPartitionCircuitBreaker(), which is invoked unconditionally at client init, so the field appears whenever the circuit breaker is configured client-side. Adds a CI-runnable regression test asserting all clientCfgs keys are present, including partitionLevelCircuitBreakerCfg. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent fccd56c commit 189a2d0

2 files changed

Lines changed: 128 additions & 1 deletion

File tree

sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/RxDocumentClientImplTest.java

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.azure.core.credential.AzureKeyCredential;
66
import com.azure.core.http.ProxyOptions;
77
import com.azure.cosmos.BridgeInternal;
8+
import com.azure.cosmos.ConnectionMode;
89
import com.azure.cosmos.ConsistencyLevel;
910
import com.azure.cosmos.CosmosContainerProactiveInitConfig;
1011
import com.azure.cosmos.CosmosDiagnostics;
@@ -35,6 +36,11 @@
3536
import com.azure.cosmos.models.ModelBridgeInternal;
3637
import com.azure.cosmos.models.PartitionKey;
3738
import com.azure.cosmos.models.PartitionKeyDefinition;
39+
import com.fasterxml.jackson.core.JsonFactory;
40+
import com.fasterxml.jackson.core.JsonGenerator;
41+
import com.fasterxml.jackson.databind.ObjectMapper;
42+
import com.fasterxml.jackson.databind.SerializerProvider;
43+
import com.fasterxml.jackson.databind.node.ObjectNode;
3844
import io.netty.buffer.ByteBufInputStream;
3945
import io.netty.buffer.Unpooled;
4046
import io.netty.handler.codec.http.HttpResponseStatus;
@@ -47,6 +53,8 @@
4753
import reactor.test.StepVerifier;
4854

4955
import java.net.URI;
56+
import java.io.StringWriter;
57+
import java.lang.reflect.Method;
5058
import java.nio.charset.StandardCharsets;
5159
import java.time.Duration;
5260
import java.util.ArrayList;
@@ -324,6 +332,120 @@ public void readMany() {
324332
}
325333
}
326334

335+
// Regression test for the "partitionLevelCircuitBreakerCfg" diagnostics field silently disappearing from the
336+
// CosmosDiagnostics "clientCfgs" section. Prior to the fix, the field was only written on the PPAF
337+
// (service-mandated) path, so a client that explicitly enabled Per-Partition Circuit Breaker never surfaced it.
338+
// This test constructs a real RxDocumentClientImpl (exercising the actual constructor wiring), drives the
339+
// private initializePerPartitionCircuitBreaker() init path, serializes the resulting DiagnosticsClientConfig,
340+
// and asserts that every expected "clientCfgs" key is present (guarding against future serialization
341+
// truncation as well as the specific regression). It also asserts the effective PPCB config string.
342+
@Test(groups = {"unit"})
343+
public void diagnosticsClientConfigContainsAllClientCfgKeysIncludingPartitionLevelCircuitBreaker() throws Exception {
344+
System.setProperty(
345+
"COSMOS.PARTITION_LEVEL_CIRCUIT_BREAKER_CONFIG",
346+
"{\"isPartitionLevelCircuitBreakerEnabled\": true, "
347+
+ "\"circuitBreakerType\": \"CONSECUTIVE_EXCEPTION_COUNT_BASED\","
348+
+ "\"consecutiveExceptionCountToleratedForReads\": 10,"
349+
+ "\"consecutiveExceptionCountToleratedForWrites\": 5,}");
350+
351+
Mockito.when(this.connectionPolicyMock.getIdleHttpConnectionTimeout()).thenReturn(Duration.ZERO);
352+
Mockito.when(this.connectionPolicyMock.getMaxConnectionPoolSize()).thenReturn(1);
353+
Mockito.when(this.connectionPolicyMock.getProxy()).thenReturn(null);
354+
Mockito.when(this.connectionPolicyMock.getHttpNetworkRequestTimeout()).thenReturn(Duration.ZERO);
355+
Mockito.when(this.connectionPolicyMock.getHttp2ConnectionConfig()).thenReturn(new Http2ConnectionConfig());
356+
// The serializer eagerly calls getConnectionMode().toString() for the very first "connectionMode" key; if this
357+
// returns null (Mockito default), serialization would NPE and silently drop every subsequent key.
358+
Mockito.when(this.connectionPolicyMock.getConnectionMode()).thenReturn(ConnectionMode.DIRECT);
359+
360+
MockedStatic<HttpClient> httpClientMock = Mockito.mockStatic(HttpClient.class);
361+
httpClientMock
362+
.when(() -> HttpClient.createFixed(Mockito.any(HttpClientConfig.class)))
363+
.thenReturn(dummyHttpClient());
364+
365+
RxDocumentClientImpl rxDocumentClient = null;
366+
367+
try {
368+
rxDocumentClient = new RxDocumentClientImpl(
369+
this.serviceEndpointMock,
370+
this.masterKeyOrResourceTokenMock,
371+
this.permissionFeedMock,
372+
this.connectionPolicyMock,
373+
this.consistencyLevelMock,
374+
null,
375+
this.configsMock,
376+
this.cosmosAuthorizationTokenResolverMock,
377+
this.azureKeyCredentialMock,
378+
false,
379+
false,
380+
false,
381+
this.metadataCachesSnapshotMock,
382+
this.apiTypeMock,
383+
this.cosmosClientTelemetryConfigMock,
384+
this.clientCorrelationIdMock,
385+
this.endToEndOperationLatencyPolicyConfig,
386+
this.sessionRetryOptionsMock,
387+
this.containerProactiveInitConfigMock,
388+
this.defaultItemSerializer,
389+
false
390+
);
391+
392+
// Drive the exact wiring that regressed: explicit (client-side) Per-Partition Circuit Breaker
393+
// initialization. The constructor does not invoke init() (which would require network), so invoke the
394+
// private no-arg initializer reflectively.
395+
Method initPpcb = RxDocumentClientImpl.class.getDeclaredMethod("initializePerPartitionCircuitBreaker");
396+
initPpcb.setAccessible(true);
397+
initPpcb.invoke(rxDocumentClient);
398+
399+
ObjectMapper objectMapper = new ObjectMapper();
400+
StringWriter jsonWriter = new StringWriter();
401+
JsonGenerator jsonGenerator = new JsonFactory().createGenerator(jsonWriter);
402+
SerializerProvider serializerProvider = objectMapper.getSerializerProvider();
403+
DiagnosticsClientContext.DiagnosticsClientConfigSerializer.INSTANCE
404+
.serialize(rxDocumentClient.getConfig(), jsonGenerator, serializerProvider);
405+
jsonGenerator.flush();
406+
ObjectNode clientCfgs = (ObjectNode) objectMapper.readTree(jsonWriter.toString());
407+
408+
String serializedJson = clientCfgs.toString();
409+
410+
// Every key the serializer unconditionally writes, plus the (previously regressed)
411+
// partitionLevelCircuitBreakerCfg which is present whenever PPCB is enabled.
412+
String[] expectedKeys = new String[] {
413+
"id",
414+
"machineId",
415+
"connectionMode",
416+
"numberOfClients",
417+
"isPpafEnabled",
418+
"isFalseProgSessionTokenMergeEnabled",
419+
"excrgns",
420+
"clientEndpoints",
421+
"connCfg",
422+
"consistencyCfg",
423+
"proactiveInitCfg",
424+
"e2ePolicyCfg",
425+
"sessionRetryCfg",
426+
"partitionLevelCircuitBreakerCfg"
427+
};
428+
429+
for (String expectedKey : expectedKeys) {
430+
assertThat(clientCfgs.has(expectedKey))
431+
.withFailMessage("Expected clientCfgs key '%s' to be present. Serialized clientCfgs: %s",
432+
expectedKey, serializedJson)
433+
.isTrue();
434+
}
435+
436+
assertThat(clientCfgs.get("partitionLevelCircuitBreakerCfg").asText())
437+
.withFailMessage("Unexpected partitionLevelCircuitBreakerCfg value. Serialized clientCfgs: %s",
438+
serializedJson)
439+
.isEqualTo("(cb: true, type: CONSECUTIVE_EXCEPTION_COUNT_BASED, rexcntt: 10, wexcntt: 5)");
440+
} finally {
441+
System.clearProperty("COSMOS.PARTITION_LEVEL_CIRCUIT_BREAKER_CONFIG");
442+
if (rxDocumentClient != null) {
443+
rxDocumentClient.close();
444+
}
445+
httpClientMock.close();
446+
}
447+
}
448+
327449
private static HttpClient dummyHttpClient() {
328450
return new HttpClient() {
329451
@Override

sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RxDocumentClientImpl.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8968,7 +8968,6 @@ private synchronized void initializePerPartitionFailover(DatabaseAccount databas
89688968
checkNotNull(this.globalPartitionEndpointManagerForPerPartitionAutomaticFailover, "Argument 'globalPartitionEndpointManagerForPerPartitionAutomaticFailover' cannot be null.");
89698969
checkNotNull(this.globalPartitionEndpointManagerForPerPartitionCircuitBreaker, "Argument 'globalPartitionEndpointManagerForPerPartitionCircuitBreaker' cannot be null.");
89708970

8971-
this.diagnosticsClientConfig.withPartitionLevelCircuitBreakerConfig(this.globalPartitionEndpointManagerForPerPartitionCircuitBreaker.getCircuitBreakerConfig());
89728971
this.diagnosticsClientConfig.withIsPerPartitionAutomaticFailoverEnabled(this.globalPartitionEndpointManagerForPerPartitionAutomaticFailover.isPerPartitionAutomaticFailoverEnabled());
89738972
}
89748973

@@ -8997,6 +8996,12 @@ private void initializePerPartitionCircuitBreaker() {
89978996

89988997
this.globalPartitionEndpointManagerForPerPartitionCircuitBreaker.resetCircuitBreakerConfig(partitionLevelCircuitBreakerConfig);
89998998
this.globalPartitionEndpointManagerForPerPartitionCircuitBreaker.init();
8999+
9000+
// Populate the circuit breaker config in the diagnostics client config here (rather than in
9001+
// initializePerPartitionFailover) so the "partitionLevelCircuitBreakerCfg" field appears in
9002+
// CosmosDiagnostics whenever the circuit breaker is configured client-side, not only when
9003+
// Per-Partition Automatic Failover is mandated by the service.
9004+
this.diagnosticsClientConfig.withPartitionLevelCircuitBreakerConfig(this.globalPartitionEndpointManagerForPerPartitionCircuitBreaker.getCircuitBreakerConfig());
90009005
}
90019006

90029007
private void enableAvailabilityStrategyForReads() {

0 commit comments

Comments
 (0)