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

filter work flow rafactor #307

Merged
merged 27 commits into from
Jan 2, 2022
Merged

filter work flow rafactor #307

merged 27 commits into from
Jan 2, 2022

Conversation

mark4z
Copy link
Member

@mark4z mark4z commented Nov 27, 2021

What this PR does:
Http Filter的生命周期现在变更为3个阶段
1.HttpFilterPlugin 可以认为是Filter工厂的工厂,CreateFilterFactory()用于注册自己
2.HttpFilterFactory Filter工厂函数,被filter_manager持有,内部会包含创建Filter所需的配置
3.当请求来临,filter_manager会创建一个filter_chain,同步调用每一个HttpFilterFactory.PrepareFilterChain(), 由HttpFilterFactory生成Filter实例并添加到链中
一个例子:

func (f *DemoFilterFactory) PrepareFilterChain(ctx *contexthttp.HttpContext, chain FilterChain) error {
	c := f.conf
	str := fmt.Sprintf("%s is drinking in the %s", c.Foo, c.Bar)
	filter := &DemoFilter{str: str}

	chain.AppendDecodeFilters(filter)
	chain.AppendEncodeFilters(filter)
	return nil
}

Which issue(s) this PR fixes:

Fixes #253

Special notes for your reviewer:
从pkg/common/http/manager.go:70开始看,会非常直观

// handleHTTPRequest handle http request
func (hcm *HttpConnectionManager) handleHTTPRequest(c *pch.HttpContext) {
	filterChain := hcm.filterManager.CreateFilterChain(c)

	// recover any err when filterChain run
	defer func() {
		if err := recover(); err != nil {
			logger.Warnf("[dubbopixiu go] Occur An Unexpected Err: %+v", err)
			c.SendLocalReply(http.StatusInternalServerError, []byte(fmt.Sprintf("Occur An Unexpected Err: %v", err)))
		}
	}()

	//todo timeout
	filterChain.OnDecode(c)
	hcm.buildTargetResponse(c)
	filterChain.OnEncode(c)
	hcm.writeResponse(c)
}

Does this PR introduce a user-facing change?:


@mark4z mark4z marked this pull request as ready for review December 11, 2021 14:42
@mark4z mark4z changed the title [WIP] filter split filter split Dec 11, 2021
@mark4z mark4z requested review from ztelur and cityiron December 11, 2021 14:42
@mark4z mark4z changed the title filter split filter work flow refsctor Dec 11, 2021
@mark4z mark4z changed the title filter work flow refsctor filter work flow rafactor Dec 11, 2021
@mark4z
Copy link
Member Author

mark4z commented Dec 11, 2021

#253

mengchao.lv added 2 commits December 12, 2021 12:54
@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2021

Codecov Report

Merging #307 (707a632) into develop (04933a9) will increase coverage by 2.11%.
The diff coverage is 45.16%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #307      +/-   ##
===========================================
+ Coverage    36.78%   38.89%   +2.11%     
===========================================
  Files           54       53       -1     
  Lines         3262     3121     -141     
===========================================
+ Hits          1200     1214      +14     
+ Misses        1938     1778     -160     
- Partials       124      129       +5     
Impacted Files Coverage Δ
pkg/client/response.go 0.00% <0.00%> (ø)
pkg/common/extension/filter/filter.go 29.16% <ø> (ø)
pkg/common/util/response.go 46.29% <0.00%> (ø)
pkg/context/http/context.go 0.00% <0.00%> (-6.67%) ⬇️
pkg/filter/host/host.go 41.17% <20.00%> (-2.58%) ⬇️
pkg/filter/header/header.go 25.80% <23.07%> (-40.87%) ⬇️
pkg/filter/metric/metric.go 34.21% <43.75%> (-30.50%) ⬇️
pkg/filter/accesslog/access_log.go 54.41% <44.44%> (+48.25%) ⬆️
pkg/common/http/manager.go 54.09% <44.73%> (-12.57%) ⬇️
pkg/filter/http/proxyrewrite/rewrite.go 53.33% <47.61%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04933a9...707a632. Read the comment docs.

@mark4z mark4z closed this Dec 12, 2021
@mark4z mark4z reopened this Dec 12, 2021
@AlexStocks
Copy link
Contributor

@mark4z plx fix the github action bugs

@ztelur ztelur requested a review from AlexStocks December 12, 2021 13:40
pkg/common/http/manager.go Outdated Show resolved Hide resolved
pkg/filter/tracing/tracing.go Outdated Show resolved Hide resolved
pkg/filter/tracing/tracing.go Outdated Show resolved Hide resolved
samples/dubbogo/multi/config/conf.yaml Show resolved Hide resolved
pkg/common/extension/filter/filter_manager.go Show resolved Hide resolved
@ztelur
Copy link
Contributor

ztelur commented Dec 19, 2021

Thanks for preparing the PR! rest LGTM

mengchao.lv added 2 commits December 26, 2021 15:53
# Conflicts:
#	pkg/filter/http/apiconfig/api_config.go
#	pkg/filter/http/grpcproxy/descriptor_operation.go
#	pkg/filter/http/grpcproxy/grpc.go
#	samples/http/grpc/pixiu/conf.yaml
@mark4z
Copy link
Member Author

mark4z commented Dec 27, 2021

@mark4z plx fix the github action bugs

Approve pls

@ztelur ztelur requested a review from xiaoliu10 December 31, 2021 01:26
case Continue:
continue
case Stop:
return
Copy link
Member

Choose a reason for hiding this comment

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

状态变更这里是否要加日志,方便排查定位

@PhilYue
Copy link
Member

PhilYue commented Jan 2, 2022

Case:如果我自定义的 Filter 没有成对的 Append EncodeDecode,比如只 chain.AppendDecodeFilters(f),对 Filter 整体执行结果有什么影响?

@AlexStocks AlexStocks merged commit c3fa875 into apache:develop Jan 2, 2022
bobtthp pushed a commit to bobtthp/dubbo-go-pixiu that referenced this pull request Dec 12, 2022
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.

filter机制问题
6 participants