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): patch dns query function earlier #11996

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Conversation

samugi
Copy link
Member

@samugi samugi commented Nov 10, 2023

Summary

The dns query lazy patch was only effective for cosockets, not for the upstream dns queries, because the patch happened too late when the toip function had already been cached in some modules (i.e. balancer)

This change moves the patch to globalpatches.lua so that dns spans are correctly generated both for cosocket and upstream dns queries.

Checklist

Full changelog

  • moved dns query patch away from instrumentation, into globalpatches

Issue reference

KAG-3057

The dns query lazy patch was only effective for cosockets, not for the
upstream dns queries, because the patch happened too late when the
`toip` function had already been cached in some modules (i.e. balancer)

This change moves the patch to `globalpatches.lua` so that dns spans
are correctly generated both for cosocket and upstream dns queries.
@samugi samugi force-pushed the fix/tracing-dns-instrum branch from 269c013 to 2bf03c5 Compare November 10, 2023 23:58
@samugi samugi changed the title fix(tracing): move dns query patch to globalpatches fix(tracing): patch dns query function earlier Nov 11, 2023
Copy link
Contributor

@StarlightIbuki StarlightIbuki left a comment

Choose a reason for hiding this comment

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

LGTM.

dndx
dndx previously approved these changes Nov 14, 2023
@dndx dndx dismissed their stale review November 14, 2023 09:55

testing

@gszr gszr merged commit 2b8c69e into master Nov 14, 2023
@gszr gszr deleted the fix/tracing-dns-instrum branch November 14, 2023 15:18
samugi added a commit that referenced this pull request Nov 14, 2023
* add test that covers a scenario that was missing from
  #11996 where a span is created for
  the upstream dns query
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants