-
Notifications
You must be signed in to change notification settings - Fork 361
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: match serviceName in tracing with mergedGateways topology #3214
Conversation
ec27985
to
70be6a5
Compare
/retest |
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.
LGTM, can you add a translator test?
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.
lgtm
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
7082daa
to
34c471f
Compare
/retest |
just thinking if we should use |
it would be better to see a xds translator test, but LGTM now. |
What type of PR is this?
What this PR does / why we need it:
When mergeGateways is enabled, tracing should set its service name to GatewayClass name, as there will be only one service for every gateway listener.
Which issue(s) this PR fixes:
Fixes #3210