Skip to content
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

feat: return entry when request is blocked. #494

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 48 additions & 55 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,73 +7,66 @@ on:
branches: "*"

jobs:

build:
name: ${{ matrix.os }} - Go ${{ matrix.go_version }}
runs-on: ubuntu-latest
strategy:
# If you want to matrix build , you can append the following list.
# If you want to matrix build , you can append the following list.
matrix:
go_version:
- 1.13
- 1.14
- 1.18
- 1.19
os:
- ubuntu-latest

steps:
- name: Set up Go ${{ matrix.go_version }}
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.go_version }}
id: go

- name: Set up Go ${{ matrix.go_version }}
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.go_version }}
id: go

- name: Check out code into the Go module directory
uses: actions/checkout@v2

- name: Cache build dependence
uses: actions/cache@v2
with:
# Cache
path: ~/go/pkg/mod
# Cache key
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
# An ordered list of keys to use for restoring the cache if no cache hit occurred for key
restore-keys: ${{ runner.os }}-go-
- name: Check out code into the Go module directory
uses: actions/checkout@v2

- name: Install goimports
run: go get golang.org/x/tools/cmd/goimports
Comment on lines -44 to -45
Copy link
Contributor

Choose a reason for hiding this comment

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

goimports check is necessary to format code

Copy link
Author

@sysulq sysulq Jan 13, 2023

Choose a reason for hiding this comment

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

It's recommend to use linters in golangci-lint.

https://golangci-lint.run/usage/linters/#goimports

- name: Cache build dependence
uses: actions/cache@v2
with:
# Cache
path: ~/go/pkg/mod
# Cache key
key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
# An ordered list of keys to use for restoring the cache if no cache hit occurred for key
restore-keys: ${{ runner.os }}-go-

- name: Install go ci lint
run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.27.0
- name: Install go ci lint
run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.50.1

- name: Run Linter
run: golangci-lint run --timeout=10m -v --disable-all --enable=govet --enable=staticcheck --enable=ineffassign --enable=misspell
- name: Run Linter
run: golangci-lint run --timeout=10m -v --disable-all --enable=govet --enable=staticcheck --enable=ineffassign --enable=misspell

- name: Test
run: |
diff -u <(echo -n) <(gofmt -d -s .)
diff -u <(echo -n) <(goimports -d .)
Comment on lines -55 to -56
Copy link
Contributor

@jnan806 jnan806 Dec 16, 2022

Choose a reason for hiding this comment

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

this two line will help to locate code which needs to be re-format

go test -v -race ./... -coverprofile=coverage.txt -covermode=atomic
cd ./pkg/datasource/consul
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../etcdv3
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../k8s
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../nacos
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../apollo
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../../adapters/echo
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../gear
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../gin
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../grpc
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../micro
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
- name: Coverage
run: bash <(curl -s https://codecov.io/bash)
- name: Test
run: |
go test -v -race ./... -coverprofile=coverage.txt -covermode=atomic
cd ./pkg/datasource/consul
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../etcdv3
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../k8s
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../nacos
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../apollo
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../../adapters/echo
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../gear
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../gin
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../grpc
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
cd ../micro
go test -race -count=1 ./... -coverprofile=coverage.txt -covermode=atomic
- name: Coverage
run: bash <(curl -s https://codecov.io/bash)
2 changes: 1 addition & 1 deletion api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func entry(resource string, options *EntryOptions) (*base.SentinelEntry, *base.B
// must finish the lifecycle of r.
blockErr := base.NewBlockErrorFromDeepCopy(r.BlockError())
e.Exit()
return nil, blockErr
return e, blockErr
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why return entry when blocked, and what do you need to do with it

Copy link
Author

@sysulq sysulq Jan 13, 2023

Choose a reason for hiding this comment

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

Leave the control to caller, and they can do any thing like log/metrics/fallback which needs entry info.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand. Do you want to get an EntryContext through entry? or you can give an example, how do you do things like log/metric/fallback

Copy link
Author

Choose a reason for hiding this comment

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

Here is the use case. Right now, there is no way to fetch entry info like ResourceType/TrafficType... when error happens.

https://github.com/douyu/jupiter/blob/493bb847aeb6fd28addbd6c7d621c34f0f2a2819/pkg/core/sentinel/config.go#LL103

}

return e, nil
Expand Down
3 changes: 2 additions & 1 deletion api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ func Test_entryWithArgsAndChainBlock(t *testing.T) {
flag: 0,
slotChain: sc,
})
assert.Nil(t, entry)
assert.NotNil(t, entry)
assert.Equal(t, "ResourceWrapper{name=abc, flowType=Inbound, classification=0}", entry.Resource().String())
assert.NotNil(t, b)
assert.Equal(t, blockType, b.BlockType())

Expand Down
3 changes: 1 addition & 2 deletions core/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package config

import (
"io/ioutil"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -100,7 +99,7 @@ func loadGlobalConfigFromYamlFile(filePath string) error {
if err != nil && !os.IsExist(err) {
return err
}
content, err := ioutil.ReadFile(filePath)
content, err := os.ReadFile(filePath)
if err != nil {
return err
}
Expand Down
10 changes: 2 additions & 8 deletions core/hotspot/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package hotspot

import (
"encoding/json"
"fmt"
"reflect"
"strconv"
Expand Down Expand Up @@ -101,13 +100,8 @@ type Rule struct {
}

func (r *Rule) String() string {
b, err := json.Marshal(r)
if err != nil {
// Return the fallback string
return fmt.Sprintf("{Id:%s, Resource:%s, MetricType:%+v, ControlBehavior:%+v, ParamIndex:%d, ParamKey:%s, Threshold:%d, MaxQueueingTimeMs:%d, BurstCount:%d, DurationInSec:%d, ParamsMaxCapacity:%d, SpecificItems:%+v}",
r.ID, r.Resource, r.MetricType, r.ControlBehavior, r.ParamIndex, r.ParamKey, r.Threshold, r.MaxQueueingTimeMs, r.BurstCount, r.DurationInSec, r.ParamsMaxCapacity, r.SpecificItems)
}
return string(b)
return fmt.Sprintf("{Id:%s, Resource:%s, MetricType:%+v, ControlBehavior:%+v, ParamIndex:%d, ParamKey:%s, Threshold:%d, MaxQueueingTimeMs:%d, BurstCount:%d, DurationInSec:%d, ParamsMaxCapacity:%d, SpecificItems:%+v}",
r.ID, r.Resource, r.MetricType, r.ControlBehavior, r.ParamIndex, r.ParamKey, r.Threshold, r.MaxQueueingTimeMs, r.BurstCount, r.DurationInSec, r.ParamsMaxCapacity, r.SpecificItems)
Comment on lines +103 to +104
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it is more appropriate to keep the original

Copy link
Author

@sysulq sysulq Jan 13, 2023

Choose a reason for hiding this comment

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

json marshal do not support map[interface{}]int64 like SpecificItems

}
func (r *Rule) ResourceName() string {
return r.Resource
Expand Down
3 changes: 1 addition & 2 deletions core/log/metric/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package metric

import (
"io/ioutil"
"os"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -83,7 +82,7 @@ func filenameMatches(filename, baseFilename string) bool {
}

func listMetricFilesConditional(baseDir string, filePattern string, predicate func(string, string) bool) ([]string, error) {
dir, err := ioutil.ReadDir(baseDir)
dir, err := os.ReadDir(baseDir)
if err != nil {
return nil, err
}
Expand Down
9 changes: 1 addition & 8 deletions ext/datasource/file/refreshable_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package file

import (
"io/ioutil"
"os"
"time"

Expand Down Expand Up @@ -47,13 +46,7 @@ func NewFileDataSource(sourceFilePath string, handlers ...datasource.PropertyHan
}

func (s *RefreshableFileDataSource) ReadSource() ([]byte, error) {
f, err := os.Open(s.sourceFilePath)
if err != nil {
return nil, errors.Errorf("RefreshableFileDataSource fail to open the property file, err: %+v.", err)
}
defer f.Close()

src, err := ioutil.ReadAll(f)
src, err := os.ReadFile(s.sourceFilePath)
if err != nil {
return nil, errors.Errorf("RefreshableFileDataSource fail to read file, err: %+v.", err)
}
Expand Down
5 changes: 2 additions & 3 deletions ext/datasource/file/refreshable_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package file

import (
"io/ioutil"
"os"
"reflect"
"strings"
Expand Down Expand Up @@ -56,7 +55,7 @@ var (

func prepareSystemRulesTestFile() error {
content := []byte(TestSystemRules)
return ioutil.WriteFile(TestSystemRulesFile, content, os.ModePerm)
return os.WriteFile(TestSystemRulesFile, content, os.ModePerm)
}

func deleteSystemRulesTestFile() error {
Expand All @@ -74,7 +73,7 @@ func TestRefreshableFileDataSource_ReadSource(t *testing.T) {
sourceFilePath: TestSystemRulesFile + "NotExisted",
}
got, err := s.ReadSource()
assert.True(t, got == nil && err != nil && strings.Contains(err.Error(), "RefreshableFileDataSource fail to open the property file"))
assert.True(t, got == nil && err != nil && strings.Contains(err.Error(), "RefreshableFileDataSource fail to read file"))

err = deleteSystemRulesTestFile()
if err != nil {
Expand Down
61 changes: 5 additions & 56 deletions ext/datasource/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package datasource

import (
"fmt"
"io/ioutil"
"os"
"reflect"
"strings"
Expand All @@ -32,17 +31,7 @@ import (
)

func TestFlowRuleJsonArrayParser(t *testing.T) {
// Prepare test data
f, err := os.Open("../../tests/testdata/extension/helper/FlowRule.json")
defer func() {
if err := f.Close(); err != nil {
t.Fatal(err)
}
}()
if err != nil {
t.Errorf("The rules file is not existed, err:%+v.", errors.WithStack(err))
}
normalSrc, err := ioutil.ReadAll(f)
normalSrc, err := os.ReadFile("../../tests/testdata/extension/helper/FlowRule.json")
if err != nil {
t.Errorf("Fail to read file, err: %+v.", errors.WithStack(err))
}
Expand Down Expand Up @@ -156,17 +145,7 @@ func TestFlowRulesUpdater(t *testing.T) {

func TestSystemRuleJsonArrayParser(t *testing.T) {
t.Run("TestSystemRuleJsonArrayParser_Normal", func(t *testing.T) {
// Prepare test data
f, err := os.Open("../../tests/testdata/extension/helper/SystemRule.json")
defer func() {
if err := f.Close(); err != nil {
t.Fatal(err)
}
}()
if err != nil {
t.Errorf("The rules file is not existed, err:%+v.", errors.WithStack(err))
}
normalSrc, err := ioutil.ReadAll(f)
normalSrc, err := os.ReadFile("../../tests/testdata/extension/helper/SystemRule.json")
if err != nil {
t.Errorf("Fail to read file, err: %+v.", errors.WithStack(err))
}
Expand Down Expand Up @@ -260,17 +239,7 @@ func TestCircuitBreakerRuleJsonArrayParser(t *testing.T) {
})

t.Run("TestCircuitBreakerRuleJsonArrayParser_Succeed", func(t *testing.T) {
// Prepare test data
f, err := os.Open("../../tests/testdata/extension/helper/CircuitBreakerRule.json")
defer func() {
if err := f.Close(); err != nil {
t.Fatal(err)
}
}()
if err != nil {
t.Errorf("The rules file is not existed, err:%+v.", err)
}
src, err := ioutil.ReadAll(f)
src, err := os.ReadFile("../../tests/testdata/extension/helper/CircuitBreakerRule.json")
if err != nil {
t.Errorf("Fail to read file, err: %+v.", err)
}
Expand Down Expand Up @@ -382,17 +351,7 @@ func TestHotSpotParamRuleJsonArrayParser(t *testing.T) {
})

t.Run("TestHotSpotParamRuleJsonArrayParser_Normal", func(t *testing.T) {
// Prepare test data
f, err := os.Open("../../tests/testdata/extension/helper/HotSpotParamFlowRule.json")
defer func() {
if err := f.Close(); err != nil {
t.Fatal(err)
}
}()
if err != nil {
t.Errorf("The rules file is not existed, err:%+v.", err)
}
src, err := ioutil.ReadAll(f)
src, err := os.ReadFile("../../tests/testdata/extension/helper/HotSpotParamFlowRule.json")
if err != nil {
t.Errorf("Fail to read file, err: %+v.", err)
}
Expand Down Expand Up @@ -513,17 +472,7 @@ func TestIsolationRuleJsonArrayParser(t *testing.T) {
})

t.Run("TestIsolationRuleJsonArrayParser_Normal", func(t *testing.T) {
// Prepare test data
f, err := os.Open("../../tests/testdata/extension/helper/IsolationRule.json")
defer func() {
if err := f.Close(); err != nil {
t.Fatal(err)
}
}()
if err != nil {
t.Errorf("The rules file is not existed, err:%+v.", err)
}
src, err := ioutil.ReadAll(f)
src, err := os.ReadFile("../../tests/testdata/extension/helper/IsolationRule.json")
if err != nil {
t.Errorf("Fail to read file, err: %+v.", err)
}
Expand Down
10 changes: 5 additions & 5 deletions ext/datasource/property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ package datasource

import (
"encoding/json"
"io/ioutil"
"os"
"testing"

"github.com/alibaba/sentinel-golang/core/system"
Expand Down Expand Up @@ -50,7 +50,7 @@ func TestSinglePropertyHandler_Handle(t *testing.T) {
assert.True(t, r1 == nil, "Fail to execute Handle func.")

h2 := NewDefaultPropertyHandler(MockSystemRulesConverter, MockSystemRulesUpdaterReturnError)
src, err := ioutil.ReadFile("../../tests/testdata/extension/SystemRule.json")
src, err := os.ReadFile("../../tests/testdata/extension/SystemRule.json")
if err != nil {
t.Errorf("Fail to get source file, err:%+v", err)
}
Expand All @@ -60,7 +60,7 @@ func TestSinglePropertyHandler_Handle(t *testing.T) {

func TestSinglePropertyHandler_isPropertyConsistent(t *testing.T) {
h := NewDefaultPropertyHandler(MockSystemRulesConverter, MockSystemRulesUpdaterReturnNil)
src, err := ioutil.ReadFile("../../tests/testdata/extension/SystemRule.json")
src, err := os.ReadFile("../../tests/testdata/extension/SystemRule.json")
if err != nil {
t.Errorf("Fail to get source file, err:%+v", err)
}
Expand All @@ -69,7 +69,7 @@ func TestSinglePropertyHandler_isPropertyConsistent(t *testing.T) {
isConsistent := h.isPropertyConsistent(ret1)
assert.True(t, isConsistent == false, "Fail to execute isPropertyConsistent.")

src2, err := ioutil.ReadFile("../../tests/testdata/extension/SystemRule2.json")
src2, err := os.ReadFile("../../tests/testdata/extension/SystemRule2.json")
if err != nil {
t.Errorf("Fail to get source file, err:%+v", err)
}
Expand All @@ -78,7 +78,7 @@ func TestSinglePropertyHandler_isPropertyConsistent(t *testing.T) {
isConsistent = h.isPropertyConsistent(ret2)
assert.True(t, isConsistent == true, "Fail to execute isPropertyConsistent.")

src3, err := ioutil.ReadFile("../../tests/testdata/extension/SystemRule3.json")
src3, err := os.ReadFile("../../tests/testdata/extension/SystemRule3.json")
if err != nil {
t.Errorf("Fail to get source file, err:%+v", err)
}
Expand Down
Loading