Alanxtl commented on code in PR #1: URL: https://github.com/apache/dubbo-go-extensions/pull/1#discussion_r2902955637
########## filter/hystrix/filter.go: ########## @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package hystrix provides hystrix filter. +// To use hystrix, you need to configure commands using hystrix-go API: +// +// import "github.com/afex/hystrix-go/hystrix" +// +// // Resource name format: dubbo:consumer:InterfaceName:group:version:Method(param1,param2) +// // Example: dubbo:consumer:com.example.GreetService:::Greet(string,string) +// hystrix.ConfigureCommand("dubbo:consumer:com.example.GreetService:::Greet(string,string)", hystrix.CommandConfig{ +// Timeout: 1000, +// MaxConcurrentRequests: 20, +// RequestVolumeThreshold: 20, +// SleepWindow: 5000, +// ErrorPercentThreshold: 50, +// }) +package hystrix Review Comment: ```suggestion package hystrix ``` ########## filter/hystrix/filter.go: ########## @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package hystrix provides hystrix filter. +// To use hystrix, you need to configure commands using hystrix-go API: +// +// import "github.com/afex/hystrix-go/hystrix" +// +// // Resource name format: dubbo:consumer:InterfaceName:group:version:Method(param1,param2) +// // Example: dubbo:consumer:com.example.GreetService:::Greet(string,string) +// hystrix.ConfigureCommand("dubbo:consumer:com.example.GreetService:::Greet(string,string)", hystrix.CommandConfig{ +// Timeout: 1000, +// MaxConcurrentRequests: 20, +// RequestVolumeThreshold: 20, +// SleepWindow: 5000, +// ErrorPercentThreshold: 50, +// }) +package hystrix + +import ( + "context" + "fmt" + "strings" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/common" + "dubbo.apache.org/dubbo-go/v3/common/constant" + "dubbo.apache.org/dubbo-go/v3/common/extension" + "dubbo.apache.org/dubbo-go/v3/filter" + "dubbo.apache.org/dubbo-go/v3/protocol/base" + "dubbo.apache.org/dubbo-go/v3/protocol/result" + + "github.com/afex/hystrix-go/hystrix" + + "github.com/dubbogo/gost/log/logger" +) + +const ( + // Filter keys + HystrixConsumerFilterKey = "hystrix_consumer" + HystrixProviderFilterKey = "hystrix_provider" + + // Prefixes for resource naming + DefaultProviderPrefix = "dubbo:provider:" + DefaultConsumerPrefix = "dubbo:consumer:" +) + +func init() { + extension.SetFilter(HystrixConsumerFilterKey, newFilterConsumer) + extension.SetFilter(HystrixProviderFilterKey, newFilterProvider) +} + +// FilterError implements error interface +type FilterError struct { + err error + failByHystrix bool +} + +func (hfError *FilterError) Error() string { + return hfError.err.Error() +} + +// FailByHystrix returns whether the fails causing by Hystrix +func (hfError *FilterError) FailByHystrix() bool { + return hfError.failByHystrix +} + +// NewHystrixFilterError return a FilterError instance +func NewHystrixFilterError(err error, failByHystrix bool) error { + return &FilterError{ + err: err, + failByHystrix: failByHystrix, + } +} + +// Filter for Hystrix +type Filter struct { + COrP bool // true for consumer, false for provider Review Comment: `COrP` is a bit hard to read. Would `isConsumer` be clearer here? It would make the code easier to understand without relying on the inline comment. ########## filter/hystrix/README_CN.md: ########## @@ -0,0 +1,92 @@ +# Hystrix 过滤器 + +[English](./README.md) | 简体中文 + +基于 [hystrix-go](https://github.com/afex/hystrix-go) 的 dubbo-go 熔断器过滤器。 + +## 安装 + +```bash +go get github.com/apache/dubbo-go-extensions/filter/hystrix +``` + +## 使用方法 + +### 消费者端 (Consumer Side) + +```go +import ( + "context" + "github.com/afex/hystrix-go/hystrix" + _ "github.com/apache/dubbo-go-extensions/filter/hystrix" + "dubbo.apache.org/dubbo-go/v3" + "dubbo.apache.org/dubbo-go/v3/client" + "dubbo.apache.org/dubbo-go/v3/registry" +) + +func init() { + // 为服务方法配置 hystrix 命令 + // 资源名称格式: dubbo:consumer:接口名:group:version:方法名 + cmdName := "dubbo:consumer:greet.GreetService:::Greet" + + hystrix.ConfigureCommand(cmdName, hystrix.CommandConfig{ + Timeout: 1000, + MaxConcurrentRequests: 10, + RequestVolumeThreshold: 5, + SleepWindow: 5000, + ErrorPercentThreshold: 50, + }) +} + +func main() { + ins, _ := dubbo.NewInstance( + dubbo.WithRegistry( + registry.WithZookeeper(), + registry.WithAddress("127.0.0.1:2181"), + ), + ) + cli, _ := ins.NewClient() + svc, _ := greet.NewGreetService(cli, client.WithFilter("hystrix_consumer")) + + resp, err := svc.Greet(context.Background(), &greet.GreetRequest{Name: "test"}) +} +``` + +### 提供者端 (Provider Side) + +```go +import ( + _ "github.com/apache/dubbo-go-extensions/filter/hystrix" + "dubbo.apache.org/dubbo-go/v3/server" +) + +func main() { + srv, _ := server.NewServer( + server.WithFilter("hystrix_provider"), + ) + // ... 服务器其余配置 +} +``` + +## 过滤器名称 + +- **消费者端**: `"hystrix_consumer"` +- **提供者端**: `"hystrix_provider"` + +## 资源命名规则 + +Hystrix 命令命名格式如下： +``` +dubbo:{consumer|provider}:{interface}:{group}:{version}:{method} Review Comment: The README's resource naming rule is inconsistent with the current implementation. `getResourceName()` now generates: `dubbo:{consumer|provider}:{interface}:{group}:{version}:{method}(paramTypes)` but the README still documents: `dubbo:{consumer|provider}:{interface}:{group}:{version}:{method}` ########## .github/workflows/github-actions.yml: ########## @@ -0,0 +1,100 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +name: CI + +on: + push: + branches: [ main, develop ] + pull_request: + branches: [ main, develop ] + +# Cancel in-progress runs for the same branch or PR +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + license-check-and-code-format: + name: License Check and Code Format + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v5 + + - name: Setup Go + uses: actions/setup-go@v6 + with: + go-version-file: 'go.mod' + cache: true + + - name: Check License Header + uses: apache/skywalking-eyes/header@main # NOSONAR + + - name: Check Golang fmt + run: | + gofmt -s -w . + git diff --exit-code Review Comment: import-format没加进来 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
