general tracing context interface by wbpcode · Pull Request #16793 · envoyproxy/envoy

wbpcode · 2021-06-03T07:19:32Z

Signed-off-by: wbpcode comems@msn.com

Sub-PR of #16049. Check #16049 get more background information.

This PR designed a new generic abstraction TraceContext to replace Http::RequestHeaderMap to provide tracing context to the tracer driver. Http::RequestHeaderMapImpl already inherits from TraceContext and implements the relevant interfaces.

Next, we just need simply replace Http::RequestHeaderMap with TraceContext in all tracer drivers implementations. After that, the main body of the entire general tracing system is completed. I'm not sure, whether the replacement step requires a separate PR. Because it does not involve any new logic, but there will be a lot of file changes.

Risk Level: Low.
Testing: Added.
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: wbpcode <comems@msn.com>

wbpcode · 2021-06-03T10:54:31Z

/retest

repokitteh-read-only · 2021-06-03T10:54:36Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16793 (comment) was created by @wbpcode.

see: more, trace.

wbpcode · 2021-06-03T13:06:07Z

@lizan @mattklein123 This PR is second sub-PR of #16049 and introduces the new TraceContext. All that remains is to replace the ReqeustHeaderMap with the TraceContext. 😃

wbpcode · 2021-06-03T13:11:21Z

@jmarantz Here I completed the new TraceContext without LowerCaseString. 😄

rojkov

Thank you! Looks very good. I've added a couple of question.

test/test_common/utility.h

include/envoy/tracing/trace_driver.h

…racing

Signed-off-by: wbpcode <comems@msn.com>

wbpcode · 2021-06-07T06:43:49Z

/retest

repokitteh-read-only · 2021-06-07T06:44:00Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16793 (comment) was created by @wbpcode.

see: more, trace.

source/common/http/header_map_impl.h

Signed-off-by: wbpcode <comems@msn.com>

rojkov

Thanks! LGTM. @lizan I think this is ready for a second pass.

source/common/http/header_map_impl.cc

test/test_common/utility.h

Signed-off-by: wbpcode <comems@msn.com>

wbpcode · 2021-06-14T00:45:25Z

/retest

repokitteh-read-only · 2021-06-14T00:45:29Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16793 (comment) was created by @wbpcode.

see: more, trace.

lizan

LGTM, just one nit

source/common/http/header_map_impl.cc

Signed-off-by: wbpcode <comems@msn.com>

…racing

wbpcode · 2021-06-22T05:08:46Z

@lizan ASSERT removed and main merged.

wbpcode · 2021-06-22T08:31:06Z

/retest

repokitteh-read-only · 2021-06-22T08:31:11Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16793 (comment) was created by @wbpcode.

see: more, trace.

source/common/http/header_map_impl.cc

Signed-off-by: wbpcode <comems@msn.com> Sub-PR of envoyproxy#16049. Check envoyproxy#16049 get more background information. This PR designed a new generic abstraction `TraceContext` to replace `Http::RequestHeaderMap` to provide tracing context to the tracer driver. `Http::RequestHeaderMapImpl` already inherits from TraceContext and implements the relevant interfaces. Next, we just need simply replace `Http::RequestHeaderMap` with `TraceContext` in all tracer drivers implementations. After that, the main body of the entire general tracing system is completed. I'm not sure, whether the replacement step requires a separate PR. Because it does not involve any new logic, but there will be a lot of file changes. Risk Level: Low. Testing: Added. Docs Changes: N/A Release Notes: N/A Signed-off-by: wbpcode <comems@msn.com> Signed-off-by: chris.xin <xinchuantao@qq.com>

Sub-PR of #16049. Check #16049 get more background information. This PR is last sub-PR of #16049. It simply replaces `Http::RequestHeaderMap` with general `Tracing::TraceContext` in all tracer drivers implementations. Check #16793 get more info about `Tracing::TraceContext`. After this PR, the main body of the entire general tracing system is completed. Next, we can try to use the new general tracing system in dubbo and thrift. Commit Message: replace RequestHeaderMap in tracers with general TraceContext Risk Level: Low. Testing: N/A. Docs Changes: N/A. Release Notes: N/A. Signed-off-by: wbpcode <comems@msn.com>

Signed-off-by: wbpcode <comems@msn.com> Sub-PR of envoyproxy#16049. Check envoyproxy#16049 get more background information. This PR designed a new generic abstraction `TraceContext` to replace `Http::RequestHeaderMap` to provide tracing context to the tracer driver. `Http::RequestHeaderMapImpl` already inherits from TraceContext and implements the relevant interfaces. Next, we just need simply replace `Http::RequestHeaderMap` with `TraceContext` in all tracer drivers implementations. After that, the main body of the entire general tracing system is completed. I'm not sure, whether the replacement step requires a separate PR. Because it does not involve any new logic, but there will be a lot of file changes. Risk Level: Low. Testing: Added. Docs Changes: N/A Release Notes: N/A Signed-off-by: wbpcode <comems@msn.com>

…roxy#17212) Sub-PR of envoyproxy#16049. Check envoyproxy#16049 get more background information. This PR is last sub-PR of envoyproxy#16049. It simply replaces `Http::RequestHeaderMap` with general `Tracing::TraceContext` in all tracer drivers implementations. Check envoyproxy#16793 get more info about `Tracing::TraceContext`. After this PR, the main body of the entire general tracing system is completed. Next, we can try to use the new general tracing system in dubbo and thrift. Commit Message: replace RequestHeaderMap in tracers with general TraceContext Risk Level: Low. Testing: N/A. Docs Changes: N/A. Release Notes: N/A. Signed-off-by: wbpcode <comems@msn.com>

new general tracing context interface

a4cef7d

Signed-off-by: wbpcode <comems@msn.com>

wbpcode mentioned this pull request Jun 3, 2021

tracing: general tracing support #16049

Closed

junr03 assigned rojkov and lizan Jun 3, 2021

rojkov reviewed Jun 4, 2021

View reviewed changes

test/test_common/utility.h Show resolved Hide resolved

include/envoy/tracing/trace_driver.h Outdated Show resolved Hide resolved

wbpcode added 3 commits June 7, 2021 09:44

Merge branch 'main' of https://github.com/envoyproxy/envoy into new-t…

b3fcd82

…racing

remove un-needed header file temporarily

0238438

Signed-off-by: wbpcode <comems@msn.com>

fix build file

f085453

Signed-off-by: wbpcode <comems@msn.com>

rojkov reviewed Jun 7, 2021

View reviewed changes

source/common/http/header_map_impl.h Show resolved Hide resolved

add comment

3f7e374

Signed-off-by: wbpcode <comems@msn.com>

rojkov previously approved these changes Jun 7, 2021

View reviewed changes

lizan reviewed Jun 11, 2021

View reviewed changes

source/common/http/header_map_impl.cc Show resolved Hide resolved

source/common/http/header_map_impl.cc Outdated Show resolved Hide resolved

test/test_common/utility.h Outdated Show resolved Hide resolved

lizan added the waiting label Jun 11, 2021

some minor update

f8ff416

Signed-off-by: wbpcode <comems@msn.com>

wbpcode dismissed rojkov’s stale review via f8ff416 June 12, 2021 14:59

repokitteh-read-only bot removed the waiting label Jun 12, 2021

remove ambiguous constructor of lowercasestring

23c1361

Signed-off-by: wbpcode <comems@msn.com>

wbpcode requested a review from lizan June 19, 2021 07:28

lizan reviewed Jun 21, 2021

View reviewed changes

source/common/http/header_map_impl.cc Outdated Show resolved Hide resolved

wbpcode added 2 commits June 22, 2021 09:58

remove ASSERT

e6cf25d

Signed-off-by: wbpcode <comems@msn.com>

Merge branch 'main' of https://github.com/envoyproxy/envoy into new-t…

3c61754

…racing

wbpcode requested a review from lizan June 24, 2021 01:40

lizan reviewed Jun 28, 2021

View reviewed changes

source/common/http/header_map_impl.cc Show resolved Hide resolved

lizan approved these changes Jun 29, 2021

View reviewed changes

lizan merged commit ad628c5 into envoyproxy:main Jun 29, 2021

wbpcode mentioned this pull request Jul 1, 2021

replace RequestHeaderMap in tracers with general TraceContext #17212

Merged

wbpcode mentioned this pull request Jan 11, 2023

tracing: make the tracer & tracer manager protocol independent #24866

Merged

Conversation

wbpcode commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbpcode commented Jun 3, 2021

Uh oh!

repokitteh-read-only bot commented Jun 3, 2021

Uh oh!

wbpcode commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbpcode commented Jun 3, 2021

Uh oh!

rojkov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wbpcode commented Jun 7, 2021

Uh oh!

repokitteh-read-only bot commented Jun 7, 2021

Uh oh!

Uh oh!

rojkov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wbpcode commented Jun 14, 2021

Uh oh!

repokitteh-read-only bot commented Jun 14, 2021

Uh oh!

lizan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wbpcode commented Jun 22, 2021

Uh oh!

wbpcode commented Jun 22, 2021

Uh oh!

repokitteh-read-only bot commented Jun 22, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wbpcode commented Jun 3, 2021 •

edited

Loading

wbpcode commented Jun 3, 2021 •

edited

Loading