-
Notifications
You must be signed in to change notification settings - Fork 827
Jav 163 zuul tracing #86
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
Conversation
8e2408a
to
fc2d684
Compare
Changes Unknown when pulling fc2d684 on JAV-163_zuul_tracing into ** on master**. |
fc2d684
to
2465e7c
Compare
255b06b
to
4720888
Compare
93bd261
to
650283e
Compare
Changes Unknown when pulling 650283e on JAV-163_zuul_tracing into ** on master**. |
<dependencies> | ||
<dependency> | ||
<groupId>io.servicecomb.tests</groupId> | ||
<artifactId>test-common</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the test common is used by other test, it could be better we just put the code into main/src.
Because we normally don't release the test jar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
users don't need to use test common, right? it's to share common test scaffolding in our integration tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that's fine.
@Override | ||
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) | ||
throws IOException, ServletException { | ||
logger.info("logged tracing filter"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be better if we can log more information about request and response here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just for testing to grab the tracing logs
650283e
to
7361c54
Compare
No description provided.