Skip to content

Add dubbogo adapter #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 26, 2020
Merged

Conversation

pantianying
Copy link
Contributor

@pantianying pantianying commented Feb 23, 2020

Describe what this PR does / why we need it

add dubbo-go adapter

Does this pull request fix one issue?

#55

Describe how you did it

Describe how to verify it

Special notes for reviews

@pantianying pantianying requested a review from sczyh30 February 23, 2020 06:19
@claassistantio
Copy link

claassistantio commented Feb 23, 2020

CLA assistant check
All committers have signed the CLA.

@sczyh30 sczyh30 added area/integrations Issue related to integrations with open-source components to-review PRs to review labels Feb 23, 2020
@sczyh30
Copy link
Member

sczyh30 commented Feb 23, 2020

Could you please sign the CLA here: https://cla-assistant.io/alibaba/sentinel-golang?pullRequest=60

@louyuting louyuting self-requested a review February 23, 2020 15:16
}

func (d *consumerFilter) OnResponse(result protocol.Result, _ protocol.Invoker, _ protocol.Invocation) protocol.Result {
return result
Copy link
Member

Choose a reason for hiding this comment

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

Actually, when the invocation completes (whether successful or with error), we need to call e.Exit() to mark it as completed, or the response time, complete count and error count will be lost. We may need to carry the *SentinelEntry in the RPC context (if possible) and handle it in OnResponse callback.

FYI: onResponse logic in Java version (1.7.1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please check it

sentinel.WithTrafficType(base.Outbound),
sentinel.WithArgs(invocation.Attachments()))
if b != nil { // blocked
result := &protocol.RPCResult{}
Copy link
Member

Choose a reason for hiding this comment

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

Can we also provide a fallback registry where developers could register their own fallback logic when blocked? We could provide the default behavior.

Refer: https://github.com/alibaba/Sentinel/tree/1.7.1/sentinel-adapter/sentinel-apache-dubbo-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/dubbo/fallback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please check it

@louyuting louyuting removed their request for review February 24, 2020 10:56
return consumerDubboFallback(ctx, invoker, invocation, b)
}
} else {
// TODO : Need to implement asynchronous current limiting
Copy link
Member

Choose a reason for hiding this comment

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

What're the differences between async and sync mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is mainly to see that in Java code, async entry is used instead of entry to restrict current. At this stage, Dubbo go has not added async field in attachment, so it will not come to async logic

Copy link
Member

Choose a reason for hiding this comment

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

Okay. Sentinel Go has unified entry and asyncEntry, so we could use sentinel.Entry(args) for both sync and async scenarios.

Copy link
Member

@sczyh30 sczyh30 left a comment

Choose a reason for hiding this comment

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

LGTM

@sczyh30 sczyh30 merged commit dae7cbf into alibaba:master Feb 26, 2020
@sczyh30 sczyh30 removed the to-review PRs to review label Feb 26, 2020
@sczyh30 sczyh30 added this to the 0.2.0 milestone Feb 26, 2020
@sczyh30 sczyh30 linked an issue Feb 26, 2020 that may be closed by this pull request
@sczyh30
Copy link
Member

sczyh30 commented Feb 26, 2020

Nice work. Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/integrations Issue related to integrations with open-source components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add integration module for dubbo-go
3 participants