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

[Ftr] tracing #236

Merged
merged 12 commits into from
Sep 18, 2021
Merged

[Ftr] tracing #236

merged 12 commits into from
Sep 18, 2021

Conversation

LvBay
Copy link
Contributor

@LvBay LvBay commented Aug 5, 2021

What this PR does:Support jaeger tracing

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


pkg/client/dubbo/dubbo.go Show resolved Hide resolved
pkg/filter/tracing/tracing.go Outdated Show resolved Hide resolved
pkg/context/http/util.go Outdated Show resolved Hide resolved
pkg/context/http/util.go Outdated Show resolved Hide resolved
pkg/context/http/util.go Outdated Show resolved Hide resolved
@cityiron cityiron changed the title traing tracing Aug 10, 2021
@cityiron cityiron changed the title tracing [Ftr] tracing Aug 10, 2021
"net/http"
"time"

"go.opentelemetry.io/otel/trace"
Copy link
Contributor

Choose a reason for hiding this comment

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

split it

@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2021

Codecov Report

Merging #236 (6defd6c) into develop (e88f221) will decrease coverage by 0.53%.
The diff coverage is 0.00%.

❗ Current head 6defd6c differs from pull request most recent head cf01aa9. Consider uploading reports for the commit cf01aa9 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #236      +/-   ##
===========================================
- Coverage    40.18%   39.65%   -0.54%     
===========================================
  Files           50       50              
  Lines         2541     2575      +34     
===========================================
  Hits          1021     1021              
- Misses        1410     1444      +34     
  Partials       110      110              
Impacted Files Coverage Δ
pkg/client/dubbo/dubbo.go 21.48% <0.00%> (-1.36%) ⬇️
pkg/client/http/http.go 43.20% <0.00%> (-4.74%) ⬇️
pkg/context/http/util.go 0.00% <0.00%> (ø)

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 e88f221...cf01aa9. Read the comment docs.


// Do execute tracerFilter filter logic.
func (f TraceFilter) Handle(hc *contexthttp.HttpContext) {
spanName := spanNamePrefix + hc.Request.Method + " " + hc.Request.URL.Path
Copy link
Member

Choose a reason for hiding this comment

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

一般情况下在网关层spanName直接到method就可以了,不需要把url path作为spanName。这是一个会无限膨胀的字段,会给后期聚合带来非常大的困难。

@@ -60,3 +64,31 @@ func HttpRouteActionMatch(c *HttpContext, ra model.RouteAction) bool {

return true
}

// ExtractRequestBody extract body of http request
func ExtractRequestBody(req *http.Request) []byte {
Copy link
Member

Choose a reason for hiding this comment

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

我还是建议不要采集body了,如果这是个上传文件的请求,或是敏感请求,不是很合适

@LvBay LvBay closed this Sep 11, 2021
@AlexStocks AlexStocks merged commit 59c6681 into apache:develop Sep 18, 2021
@ztelur ztelur added this to the v0.4.0 milestone Oct 20, 2021
mark4z pushed a commit that referenced this pull request Nov 7, 2021
[Ftr] tracing

Former-commit-id: 59c6681
tydhot pushed a commit to tydhot/dubbo-go-pixiu that referenced this pull request Nov 10, 2021
[Ftr] tracing

Former-commit-id: 6bb0a57 [formerly 59c6681]
Former-commit-id: 7b27b44
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.

9 participants