remove trace drivers' dependency on HttpTracerImpl#16244
Merged
lizan merged 5 commits intoenvoyproxy:mainfrom May 12, 2021
Merged
remove trace drivers' dependency on HttpTracerImpl#16244lizan merged 5 commits intoenvoyproxy:mainfrom
lizan merged 5 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: wbpcode <comems@msn.com>
wbpcode
requested review from
Shikugawa,
dio,
lizan and
mattklein123
as code owners
Member
Author
|
Here is first sub PR. @lizan There are some points my be can help reviewers:
|
Signed-off-by: wbpcode <comems@msn.com>
dio
reviewed
May 6, 2021
| class TracingTagValues { | ||
| public: | ||
| // OpenTracing standard tag names. | ||
| const std::string Component = "component"; |
Member
There was a problem hiding this comment.
Not sure if we can make all of these as constexpr. WDYT?
Member
Author
There was a problem hiding this comment.
@dio These values are separated from http_tracer_impl.h derectly and no any change. I prefer to add a TODO, and then complete the const to constexpr migration in a new PR.
added 2 commits
May 7, 2021 18:22
Signed-off-by: wbpcode <comems@msn.com>
Member
Author
|
/retest |
|
Retrying Azure Pipelines: |
Member
Author
|
/retest |
|
Retrying Azure Pipelines: |
Member
Author
|
/retest |
|
Retrying Azure Pipelines: |
lizan
approved these changes
May 12, 2021
| class TracingTagValues { | ||
| public: | ||
| // OpenTracing standard tag names. | ||
| const std::string Component = "component"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signed-off-by: wbpcode comems@msn.com
This PR is part of #16049 to support general tracing. Please check #16049 get more details.
Commit Message: remove trace drivers' dependency on HttpTracerImpl
Additional Description:
Now all tracers (zipkin, skywalking, etc.) will depend on HttpTracerImpl, making it difficult for Tracers to be reused by other protocols (Dubbo, Thrift, etc.). The purpose of this PR is to change this dependency.
Risk Level: Low (No new logic).
Testing: Add.
Docs Changes: N/A
Release Notes: N/A