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.

Sign up for GitHub

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

Jump to bottom

fix(zipkin): global plugin always executes in rewrite phase #9877

Merged
merged 2 commits into from
Dec 5, 2022
Merged

fix(zipkin): global plugin always executes in rewrite phase #9877

merged 2 commits into from
Dec 5, 2022

Conversation

mayocream
Copy link
Contributor

@mayocream mayocream commented Dec 5, 2022 •
edited by fffonion
Loading

Summary

The global Zipkin plugin always executes in the rewrite phase whenever a route/service-specific plugin enables, since the router only executes after the rewrite phase.
This cause an issue where route/service-specific plugin inherits the context written by the global plugin in the access phase.

According to plugin precedence, a plugin will always be run once and only once per request, and route/service-specific plugins have higher priority than global plugins.

This PR moves the logic inside the rewrite phase handler to the log phase, and it sets the span annotation lazily with no side-effects.

Checklist

Issue reference

Fix #9812

@mayocream mayocream force-pushed the fix/zipkin-context-overwritten branch from 0ec7833 to a2192d8 Compare December 5, 2022 08:53
@mayocream mayocream force-pushed the fix/zipkin-context-overwritten branch from a2192d8 to 2c761de Compare December 5, 2022 08:53
fffonion
fffonion approved these changes Dec 5, 2022
View reviewed changes
Copy link
Contributor

@fffonion fffonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, missing changelog, add after 3.1 tagged and rebase

@fffonion fffonion merged commit eedadc2 into master Dec 5, 2022
@fffonion fffonion deleted the fix/zipkin-context-overwritten branch December 5, 2022 09:25
oowl pushed a commit that referenced this pull request Aug 15, 2024
@ADD-SP
chore(build): make sure the .so was compiled for the `json-threat-p…
7668a58 
…rotection` plugin (#9877)

Make sure the `liblua_resty_json_threat_protection.so` was generated by compiler and it is right.

KAG-5109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fffonion fffonion fffonion approved these changes

Assignees
No one assigned
Labels
changelog plugins/zipkin size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Zipkin: Global sample ratio overrides route-specific
2 participants
@mayocream @fffonion