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

fix(tracing): should trace request stopped by extern plugin #6500

Merged
merged 4 commits into from
Mar 6, 2022

Conversation

spacewander
Copy link
Member

Signed-off-by: spacewander spacewanderlzx@gmail.com

What this PR does / why we need it:

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@spacewander
Copy link
Member Author

I don't change the skywalking/opentracing because increasing the priority will make the body_filter call earlier.

Can we remove the finish in the body_filter and only finish the span in the log phase?
@dmsolr @yangxikun
Please guide me the right way.

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
@spacewander spacewander marked this pull request as ready for review March 3, 2022 07:32
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
@spacewander
Copy link
Member Author

spacewander commented Mar 3, 2022

Or maybe we can switch to an onion model for the filter?(Not a good idea)

@dmsolr
Copy link
Member

dmsolr commented Mar 4, 2022

I think finish() should be after ext-plugin, but start() should be before ext-plugin.
Moving the finish() to the log phase represents increasing the response time in trace view.

the trace includes 2 spans in APISIX,
the first span, entry span, is over the whole request. It means from the request accepted to the response returned.
the 2nd span, exit span, is only from the sending request to accepting the response.
(Now, those spans start and stop at the same time.)

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
@spacewander
Copy link
Member Author

I will open another issue to trace #6500 (comment)

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.

4 participants