You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/10/22 09:46:06 UTC
[GitHub] [apisix] gy09535 opened a new pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
gy09535 opened a new pull request #2499:
URL: https://github.com/apache/apisix/pull/2499
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
fix: https://github.com/apache/apisix/issues/2377
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [] Have you modified the corresponding document?
* [x] Is this PR backward compatible?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis merged pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #2499:
URL: https://github.com/apache/apisix/pull/2499
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2499:
URL: https://github.com/apache/apisix/pull/2499#issuecomment-717812067
@nic-chen @moonming @spacewander do you have time to look at this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #2499:
URL: https://github.com/apache/apisix/pull/2499#issuecomment-719086701
It's a difficult multiple choice question.
I think we could fix it in zipkin plugin, add some judge code to make sure the span is exits. And let user custom its priority.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
Posted by GitBox <gi...@apache.org>.
gy09535 commented on pull request #2499:
URL: https://github.com/apache/apisix/pull/2499#issuecomment-717777569
Is this PR can merge?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2499:
URL: https://github.com/apache/apisix/pull/2499#issuecomment-718390987
I don't think it is a good idea to modify the priority. I will take a look again
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 edited a comment on pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
Posted by GitBox <gi...@apache.org>.
gy09535 edited a comment on pull request #2499:
URL: https://github.com/apache/apisix/pull/2499#issuecomment-719263049
yeah, change default priority and also add some judge code is better. Because If we just add judge code when zipkin plugin executed after limit-count then the access span will be lost, we should avoid this condition, so we can set the default priority to reduce risk .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gy09535 commented on pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
Posted by GitBox <gi...@apache.org>.
gy09535 commented on pull request #2499:
URL: https://github.com/apache/apisix/pull/2499#issuecomment-719263049
yeah, change default priority and also add some judge code is better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2499: bugfix: fix zipkin plugin error when used with limit count plugin
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2499:
URL: https://github.com/apache/apisix/pull/2499#issuecomment-719087839
Yes, we should fix it in zipkin plugin
nic-chen <no...@github.com>于2020年10月30日 周五上午7:46写道：
> It's a difficult multiple choice question.
> I think we could fix it in zipkin plugin, add some judge code to make sure
> the span is exits. And let user custom its priority.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/apisix/pull/2499#issuecomment-719086701>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGJZBK5LEVYRRWRVS7BI6STSNH5HDANCNFSM4S26MBAA>
> .
>
--
Thanks,
Ming Wen
Twitter: _WenMing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org