Ftr: add sentinel-golang flow control/circuit breaker #748
Conversation
|
Hi, guys I'm still working on this PR. I will mark Ready if I finish the dev. Thanks |
louyuting
force-pushed
the
feature/sentinel-integration
branch
from
05a7251
to
b534cc9
Compare
louyuting
changed the title
AlexStocks
changed the title
…flow control/circuit breaker
louyuting
force-pushed
the
feature/sentinel-integration
branch
from
2c1b9cf
to
d294b14
Compare
|
@pantianying Could you please review this PR? Besides, we might need another example to demo how to use Sentinel in dubbo-go |
I will do it soon |
| "github.com/apache/dubbo-go/protocol/invocation" | ||
| ) | ||
|
|
||
| func TestConsumerFilter_Invoke(t *testing.T) { |
There was a problem hiding this comment.
Can you provide another test case that tests whether the current limiter is working
There was a problem hiding this comment.
I will add a case.
But I am not familiar with dubbogo. Could you please help to review case later.
| if getInterfaceGroupAndVersionEnabled() { | ||
| interfaceResourceName = getColonSeparatedKey(invoker.GetUrl()) | ||
| } else { | ||
| interfaceResourceName = invoker.GetUrl().Service() | ||
| } |
There was a problem hiding this comment.
refactor this code into getInterfaceResourceName func
There was a problem hiding this comment.
or simply introduce a func to return both interface resource name and method resource name? func getResourceNames(invoker protocol.Invoker, invocation protocol.Invocation, prefix string) (string, string)
There was a problem hiding this comment.
From my perspective, encapsulating a func getResourceName(invoker protocol.Invoker, invocation protocol.Invocation, prefix string) (interfaceResourceName, methodResourceName string) is better.
| type SentinelProviderFilter struct{} | ||
|
|
||
| func (d *SentinelProviderFilter) Invoke(ctx context.Context, invoker protocol.Invoker, invocation protocol.Invocation) protocol.Result { | ||
| methodResourceName := getResourceName(invoker, invocation, getProviderPrefix()) |
There was a problem hiding this comment.
change this func name to getMethodResourceName?
| if methodEntry := ctx.Value(MethodEntryKey); methodEntry != nil { | ||
| e := methodEntry.(*base.SentinelEntry) | ||
| sentinel.TraceError(e, result.Error()) | ||
| e.Exit() | ||
| } |
There was a problem hiding this comment.
it looks to me this block deserves to extract a func so that it can be reused by the following 'InterfaceEntryKey'
| } | ||
| ctx = context.WithValue(ctx, InterfaceEntryKey, interfaceEntry) | ||
|
|
||
| methodEntry, b = sentinel.Entry(methodResourceName, sentinel.WithResourceType(base.ResTypeRPC), |
|
|
||
| func getResourceName(invoker protocol.Invoker, invocation protocol.Invocation, prefix string) string { | ||
| var ( | ||
| buf bytes.Buffer |
There was a problem hiding this comment.
Maybe it's more efficient to use strings.Builder to replace bytes.Buffer.
There was a problem hiding this comment.
I will refactor here
louyuting
requested review from
AlexStocks,
watermelo,
pantianying and
beiwei30
…tion Ftr: add sentinel-golang flow control/circuit breaker # Conflicts: # go.sum
Ftr: add sentinel-golang flow control/circuit breaker # Conflicts: # go.sum
…tion Ftr: add sentinel-golang flow control/circuit breaker
What this PR does:
Integrate sentinel-golang into dubbo-go
Which issue(s) this PR fixes:
Special notes for your reviewer:
@pantianying
Does this PR introduce a user-facing change?: