Skip to content

[SCB-506]服务治理相关的需要事件上报#666

Closed
xuyiyun0929 wants to merge 3 commits into
apache:masterfrom
xuyiyun0929:alarm
Closed

[SCB-506]服务治理相关的需要事件上报#666
xuyiyun0929 wants to merge 3 commits into
apache:masterfrom
xuyiyun0929:alarm

Conversation

@xuyiyun0929

Copy link
Copy Markdown
Contributor

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SCB-XXX] Fixes bug in ApproximateQuantiles, where you replace SCB-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

@@ -0,0 +1,32 @@
package org.apache.servicecomb.loadbalance.event;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License header please.

@@ -0,0 +1,27 @@
package org.apache.servicecomb.bizkeeper.event;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License header please.

@@ -0,0 +1,50 @@
package org.apache.servicecomb.bizkeeper.event;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License header please.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 87.395% when pulling 9c358ad on xuyiyun0929:alarm into 43cec89 on apache:master.

@liubao68

Copy link
Copy Markdown
Contributor

You have make some related merge. Please fix it or submit a new one.
You can refer http://servicecomb.incubator.apache.org/developers/submit-codes/ to create new PR's. And you can directly commit to your new branch for comments fix to your PR.

public enum Type {
OPEN,
CLOSE,
SUCCED,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use SUCCEDED or SUCCESSFUL. When should we use OPEN vs SUCCED?

HystrixCircuitBreaker circuitBreaker =
HystrixCircuitBreaker.Factory.getInstance(CommandKey.toHystrixCommandKey(groupname, invocation));
if (circuitBreaker != null && circuitBreaker.isOpen()) {
EventManager.post(new CircutBreakerEvent(invocation, this.groupname, Type.OPEN));

@liubao68 liubao68 Apr 24, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Format your code use code template in /etc folder and it's better to turn on show blanks in your IDE

public static Response getFallbackResponse(String type, Throwable error, Invocation invocation) {
FallbackPolicy policy = getPolicy(type, invocation);
if (policy != null) {
EventManager.post(new FallbackEvent(policy, invocation, Type.OPEN));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallback is called in HystrixCommand, send event there. But I think this will send a lot of unnecessary events because any error will call fallback.

if (circuitBreaker != null && circuitBreaker.isOpen()) {
EventManager.post(new CircutBreakerEvent(invocation, this.groupname, Type.OPEN));
}else {
EventManager.post(new CircutBreakerEvent(invocation, this.groupname, Type.CLOSE));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need only send event when last time is OPEN and turn to be CLOSED. Can't send for each invocation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants