[SCB-2514]governance rate limiting support service name match (#2909)
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/handler/InstanceIsolationHandler.java b/governance/src/main/java/org/apache/servicecomb/governance/handler/InstanceIsolationHandler.java
index c798870..85afecd 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/handler/InstanceIsolationHandler.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/handler/InstanceIsolationHandler.java
@@ -51,7 +51,7 @@
@Override
protected String createKey(GovernanceRequest governanceRequest, CircuitBreakerPolicy policy) {
return InstanceIsolationProperties.MATCH_INSTANCE_ISOLATION_KEY
- + "." + governanceRequest.getServiceId()
+ + "." + governanceRequest.getServiceName()
+ "." + governanceRequest.getInstanceId();
}
@@ -68,13 +68,13 @@
@Override
public CircuitBreakerPolicy matchPolicy(GovernanceRequest governanceRequest) {
- if (StringUtils.isEmpty(governanceRequest.getServiceId()) || StringUtils.isEmpty(
+ if (StringUtils.isEmpty(governanceRequest.getServiceName()) || StringUtils.isEmpty(
governanceRequest.getInstanceId())) {
LOGGER.info("Isolation is not properly configured, service id or instance id is empty.");
return null;
}
CircuitBreakerPolicy circuitBreakerPolicy =
- instanceIsolationProperties.getParsedEntity().get(governanceRequest.getServiceId());
+ instanceIsolationProperties.getParsedEntity().get(governanceRequest.getServiceName());
if (circuitBreakerPolicy == null) {
return instanceIsolationProperties.getParsedEntity().get(DEFAULT_SERVICE_NAME);
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/marker/GovernanceRequest.java b/governance/src/main/java/org/apache/servicecomb/governance/marker/GovernanceRequest.java
index bd70dbf..a42c229 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/marker/GovernanceRequest.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/marker/GovernanceRequest.java
@@ -21,17 +21,40 @@
import org.springframework.util.LinkedCaseInsensitiveMap;
public class GovernanceRequest {
+ /**
+ headers with this request, maybe null.
+ For provider: headers indicates the request headers to me.
+ For consumer: headers indicates the request headers to the target.
+ */
private Map<String, String> headers;
+ /**
+ uri with this request, maybe null.
+ For provider: uri indicates the request uri to me.
+ For consumer: uri indicates the request uri to the target.
+ */
private String uri;
+ /**
+ method with this request, maybe null.
+ For provider: method indicates the request method to me.
+ For consumer: method indicates the request method to the target.
+ */
private String method;
- // instance id with request, maybe null.
+ /**
+ instance id with this request, maybe null.
+ For provider: instanceId indicates who calls me.
+ For consumer: instanceId indicates the target instance.
+ */
private String instanceId;
- // microservice id (microservice name or application name + microservice name) with this request, maybe null.
- private String serviceId;
+ /**
+ microservice id (microservice name or application name + microservice name) with this request, maybe null.
+ For provider: serviceName indicates who calls me.
+ For consumer: serviceName indicates the target service.
+ */
+ private String serviceName;
public Map<String, String> getHeaders() {
return headers;
@@ -68,11 +91,11 @@
this.instanceId = instanceId;
}
- public String getServiceId() {
- return serviceId;
+ public String getServiceName() {
+ return serviceName;
}
- public void setServiceId(String serviceId) {
- this.serviceId = serviceId;
+ public void setServiceName(String serviceName) {
+ this.serviceName = serviceName;
}
}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/marker/Matcher.java b/governance/src/main/java/org/apache/servicecomb/governance/marker/Matcher.java
index 95184a4..f0a70cd 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/marker/Matcher.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/marker/Matcher.java
@@ -30,11 +30,22 @@
private List<String> method;
+ private String serviceName;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+
public Map<String, RawOperator> getHeaders() {
return headers;
}
- public void setHeaders(Map<String, RawOperator> headers) {
+ public void setHeaders(
+ Map<String, RawOperator> headers) {
this.headers = headers;
}
@@ -54,11 +65,11 @@
this.method = method;
}
- public String getName() {
- return name;
+ public String getServiceName() {
+ return serviceName;
}
- public void setName(String name) {
- this.name = name;
+ public void setServiceName(String serviceName) {
+ this.serviceName = serviceName;
}
}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/marker/RequestProcessor.java b/governance/src/main/java/org/apache/servicecomb/governance/marker/RequestProcessor.java
index 95f26b8..db03b9d 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/marker/RequestProcessor.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/marker/RequestProcessor.java
@@ -24,6 +24,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+/**
+ * Request Processor checks if a request matches a configuration.
+ */
public class RequestProcessor {
private static final Logger LOGGER = LoggerFactory.getLogger(RequestProcessor.class);
@@ -37,10 +40,26 @@
}
public boolean match(GovernanceRequest request, Matcher matcher) {
- if ((matcher.getMethod() != null && !matcher.getMethod().contains(request.getMethod())) ||
- (matcher.getApiPath() != null && !operatorMatch(request.getUri(), matcher.getApiPath()))) {
+ if (!methodMatch(request, matcher)) {
return false;
}
+ if (!apiPathMatch(request, matcher)) {
+ return false;
+ }
+ if (!headersMatch(request, matcher)) {
+ return false;
+ }
+ return serviceNameMatch(request, matcher);
+ }
+
+ private boolean serviceNameMatch(GovernanceRequest request, Matcher matcher) {
+ if (matcher.getServiceName() == null) {
+ return true;
+ }
+ return matcher.getServiceName().equals(request.getServiceName());
+ }
+
+ private boolean headersMatch(GovernanceRequest request, Matcher matcher) {
if (matcher.getHeaders() == null) {
return true;
}
@@ -53,6 +72,20 @@
return true;
}
+ private boolean apiPathMatch(GovernanceRequest request, Matcher matcher) {
+ if (matcher.getApiPath() == null) {
+ return true;
+ }
+ return operatorMatch(request.getUri(), matcher.getApiPath());
+ }
+
+ private boolean methodMatch(GovernanceRequest request, Matcher matcher) {
+ if (matcher.getMethod() == null) {
+ return true;
+ }
+ return matcher.getMethod().contains(request.getMethod());
+ }
+
private boolean operatorMatch(String str, RawOperator rawOperator) {
if (rawOperator.isEmpty()) {
return false;
@@ -61,7 +94,7 @@
for (Entry<String, String> entry : rawOperator.entrySet()) {
MatchOperator operator = operatorMap.get(entry.getKey() + OPERATOR_SUFFIX);
if (operator == null) {
- LOGGER.error("unsupported operator:" + entry.getKey() + ", plz use one of :" + operatorMap.keySet().toString());
+ LOGGER.error("unsupported operator:" + entry.getKey() + ", please use one of :" + operatorMap.keySet());
return false;
}
if (!operator.match(str, entry.getValue())) {
diff --git a/governance/src/test/java/org/apache/servicecomb/governance/FlowControlTest.java b/governance/src/test/java/org/apache/servicecomb/governance/FlowControlTest.java
index 3a6288f..52d067a 100644
--- a/governance/src/test/java/org/apache/servicecomb/governance/FlowControlTest.java
+++ b/governance/src/test/java/org/apache/servicecomb/governance/FlowControlTest.java
@@ -23,7 +23,6 @@
import org.apache.servicecomb.governance.handler.RateLimitingHandler;
import org.apache.servicecomb.governance.marker.GovernanceRequest;
-import org.apache.servicecomb.governance.properties.RateLimitProperties;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
@@ -40,33 +39,17 @@
public class FlowControlTest {
private RateLimitingHandler rateLimitingHandler;
- private RateLimitProperties rateLimitProperties;
-
- private MatchersManager matchersManager;
-
@Autowired
public void setRateLimitingHandler(RateLimitingHandler rateLimitingHandler) {
this.rateLimitingHandler = rateLimitingHandler;
}
- @Autowired
- public void setRateLimitProperties(RateLimitProperties rateLimitProperties) {
- this.rateLimitProperties = rateLimitProperties;
- }
-
- @Autowired
- public void setMatchersManager(MatchersManager matchersManager) {
- this.matchersManager = matchersManager;
- }
-
public FlowControlTest() {
}
@Test
public void test_rate_limiting_work() throws Throwable {
- DecorateCheckedSupplier<Object> ds = Decorators.ofCheckedSupplier(() -> {
- return "test";
- });
+ DecorateCheckedSupplier<Object> ds = Decorators.ofCheckedSupplier(() -> "test");
GovernanceRequest request = new GovernanceRequest();
request.setUri("/hello");
@@ -81,23 +64,60 @@
AtomicBoolean expected = new AtomicBoolean(false);
AtomicBoolean notExpected = new AtomicBoolean(false);
for (int i = 0; i < 10; i++) {
- new Thread() {
- public void run() {
- try {
- Object result = ds.get();
- if (!"test".equals(result)) {
- notExpected.set(true);
- }
- } catch (Throwable e) {
- if (e instanceof RequestNotPermitted) {
- expected.set(true);
- } else {
- notExpected.set(true);
- }
+ new Thread(() -> {
+ try {
+ Object result = ds.get();
+ if (!"test".equals(result)) {
+ notExpected.set(true);
}
- cd.countDown();
+ } catch (Throwable e) {
+ if (e instanceof RequestNotPermitted) {
+ expected.set(true);
+ } else {
+ notExpected.set(true);
+ }
}
- }.start();
+ cd.countDown();
+ }).start();
+ }
+ cd.await(1, TimeUnit.SECONDS);
+ Assertions.assertTrue(expected.get());
+ Assertions.assertFalse(notExpected.get());
+ }
+
+ @Test
+ public void test_rate_limiting_service_name_work() throws Throwable {
+ DecorateCheckedSupplier<Object> ds = Decorators.ofCheckedSupplier(() -> "test");
+
+ GovernanceRequest request = new GovernanceRequest();
+ request.setUri("/helloServiceName");
+ request.setServiceName("srcService");
+
+ RateLimiter rateLimiter = rateLimitingHandler.getActuator(request);
+ ds.withRateLimiter(rateLimiter);
+
+ Assertions.assertEquals("test", ds.get());
+
+ // flow control
+ CountDownLatch cd = new CountDownLatch(10);
+ AtomicBoolean expected = new AtomicBoolean(false);
+ AtomicBoolean notExpected = new AtomicBoolean(false);
+ for (int i = 0; i < 10; i++) {
+ new Thread(() -> {
+ try {
+ Object result = ds.get();
+ if (!"test".equals(result)) {
+ notExpected.set(true);
+ }
+ } catch (Throwable e) {
+ if (e instanceof RequestNotPermitted) {
+ expected.set(true);
+ } else {
+ notExpected.set(true);
+ }
+ }
+ cd.countDown();
+ }).start();
}
cd.await(1, TimeUnit.SECONDS);
Assertions.assertTrue(expected.get());
diff --git a/governance/src/test/java/org/apache/servicecomb/governance/GovernancePropertiesTest.java b/governance/src/test/java/org/apache/servicecomb/governance/GovernancePropertiesTest.java
index 6f6bbc0..aaec8e1 100644
--- a/governance/src/test/java/org/apache/servicecomb/governance/GovernancePropertiesTest.java
+++ b/governance/src/test/java/org/apache/servicecomb/governance/GovernancePropertiesTest.java
@@ -159,7 +159,7 @@
@Test
public void test_match_properties_successfully_loaded() {
Map<String, TrafficMarker> markers = matchProperties.getParsedEntity();
- Assertions.assertEquals(4, markers.size());
+ Assertions.assertEquals(5, markers.size());
TrafficMarker demoRateLimiting = markers.get("demo-rateLimiting");
List<Matcher> matchers = demoRateLimiting.getMatches();
Assertions.assertEquals(1, matchers.size());
@@ -177,17 +177,17 @@
@Test
public void test_match_properties_delete() {
Map<String, TrafficMarker> markers = matchProperties.getParsedEntity();
- Assertions.assertEquals(4, markers.size());
+ Assertions.assertEquals(5, markers.size());
dynamicValues.put("servicecomb.matchGroup.test", "matches:\n"
+ " - apiPath:\n"
+ " exact: \"/hello2\"\n"
+ " name: match0");
GovernanceEventManager.post(new GovernanceConfigurationChangedEvent(new HashSet<>(dynamicValues.keySet())));
markers = matchProperties.getParsedEntity();
- Assertions.assertEquals(5, markers.size());
+ Assertions.assertEquals(6, markers.size());
tearDown();
markers = matchProperties.getParsedEntity();
- Assertions.assertEquals(4, markers.size());
+ Assertions.assertEquals(5, markers.size());
}
@Test
@@ -204,7 +204,7 @@
GovernanceEventManager.post(new GovernanceConfigurationChangedEvent(new HashSet<>(dynamicValues.keySet())));
Map<String, TrafficMarker> markers = matchProperties.getParsedEntity();
- Assertions.assertEquals(5, markers.size());
+ Assertions.assertEquals(6, markers.size());
TrafficMarker demoRateLimiting = markers.get("demo-rateLimiting");
List<Matcher> matchers = demoRateLimiting.getMatches();
Assertions.assertEquals(1, matchers.size());
@@ -344,7 +344,7 @@
@Test
public void test_rate_limit_properties_successfully_loaded() {
Map<String, RateLimitingPolicy> policies = rateLimitProperties.getParsedEntity();
- Assertions.assertEquals(1, policies.size());
+ Assertions.assertEquals(2, policies.size());
RateLimitingPolicy policy = policies.get("demo-rateLimiting");
Assertions.assertEquals(1, policy.getRate());
}
diff --git a/governance/src/test/java/org/apache/servicecomb/governance/InstanceIsolationTest.java b/governance/src/test/java/org/apache/servicecomb/governance/InstanceIsolationTest.java
index fa24219..86b19bf 100644
--- a/governance/src/test/java/org/apache/servicecomb/governance/InstanceIsolationTest.java
+++ b/governance/src/test/java/org/apache/servicecomb/governance/InstanceIsolationTest.java
@@ -66,7 +66,7 @@
GovernanceRequest request = new GovernanceRequest();
request.setInstanceId("instance01");
- request.setServiceId("service01");
+ request.setServiceName("service01");
CircuitBreaker circuitBreaker = instanceIsolationHandler.getActuator(request);
ds.withCircuitBreaker(circuitBreaker);
@@ -85,7 +85,7 @@
// isolation do not influence other instances
GovernanceRequest request2 = new GovernanceRequest();
request2.setInstanceId("instance02");
- request2.setServiceId("service01");
+ request2.setServiceName("service01");
CircuitBreaker circuitBreaker2 = instanceIsolationHandler.getActuator(request2);
ds2.withCircuitBreaker(circuitBreaker2);
diff --git a/governance/src/test/resources/application.yaml b/governance/src/test/resources/application.yaml
index 6891f69..b76e489 100644
--- a/governance/src/test/resources/application.yaml
+++ b/governance/src/test/resources/application.yaml
@@ -21,6 +21,11 @@
matches:
- apiPath:
exact: "/hello"
+ demo-rateLimiting-servicename: |
+ matches:
+ - apiPath:
+ exact: "/helloServiceName"
+ serviceName: "srcService"
wrong-name-inogred: |
wrong: some
demo-retry: |
@@ -48,6 +53,8 @@
rateLimiting:
demo-rateLimiting: |
rate: 1
+ demo-rateLimiting-servicename: |
+ rate: 1
wrongIngored: |
rate: 0
retry:
