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

contrib/miekg/dns: fix span kind for dns server traces #1696

Merged
merged 4 commits into from
Jan 27, 2023

Conversation

rarguelloF
Copy link
Contributor

@rarguelloF rarguelloF commented Jan 26, 2023

What does this PR do?

Fixes span kind for dns server traces - this library is used both for DNS client and server and we were setting type client for both.

Also refactored tests and increased test coverage for some functions not covered by existing tests.

Motivation

We want to set correct span kind for DNS server traces.

Describe how to test/QA your changes

Updated existing tests.

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Changed code has unit tests for its functionality.
  • If this interacts with the agent in a new way, a system test has been added.

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
mikededo Miquel de Domingo i Giralt

Verified

This commit was signed with the committer’s verified signature.
mikededo Miquel de Domingo i Giralt
@rarguelloF rarguelloF added this to the Triage milestone Jan 26, 2023
@rarguelloF rarguelloF marked this pull request as ready for review January 26, 2023 16:56
@rarguelloF rarguelloF requested a review from a team January 26, 2023 16:56
@rarguelloF rarguelloF marked this pull request as draft January 26, 2023 17:15
@pr-commenter
Copy link

pr-commenter bot commented Jan 26, 2023

Benchmarks

Comparing candidate commit d17e6fe in PR branch rarguelloF/fix-dns-span-kind with baseline commit 3f8f234 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 6 cases.

@rarguelloF rarguelloF marked this pull request as ready for review January 26, 2023 19:30
Copy link
Contributor

@ajgajg1134 ajgajg1134 left a comment

Choose a reason for hiding this comment

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

Overall looks good! Just some whitespace comments here

@ajgajg1134 ajgajg1134 modified the milestones: Triage, v1.48.0 Jan 26, 2023

Verified

This commit was signed with the committer’s verified signature.
mikededo Miquel de Domingo i Giralt
…rver ones

By separating them, we don't depend on the order of the received spans when making
assertions.
@rarguelloF rarguelloF force-pushed the rarguelloF/fix-dns-span-kind branch from 25987d8 to 1def817 Compare January 27, 2023 08:56
@rarguelloF rarguelloF requested a review from ajgajg1134 January 27, 2023 08:56

Verified

This commit was signed with the committer’s verified signature.
mikededo Miquel de Domingo i Giralt
@ajgajg1134 ajgajg1134 merged commit 8b3c776 into main Jan 27, 2023
@ajgajg1134 ajgajg1134 deleted the rarguelloF/fix-dns-span-kind branch January 27, 2023 20:11
dianashevchenko pushed a commit that referenced this pull request Feb 9, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

None yet

2 participants