Skip to content

dnsdist: Add feature to create Telemetry Spans from LuaAction#16961

Open
pieterlexis wants to merge 5 commits intoPowerDNS:masterfrom
pieterlexis:dnsdist-lua-opentelemetry
Open

dnsdist: Add feature to create Telemetry Spans from LuaAction#16961
pieterlexis wants to merge 5 commits intoPowerDNS:masterfrom
pieterlexis:dnsdist-lua-opentelemetry

Conversation

@pieterlexis
Copy link
Contributor

@pieterlexis pieterlexis commented Mar 9, 2026

Short description

This PR adds a withTraceSpan method to DNSQuestion and DNSResponse that takes the name of the Span and the function that will be instrumented.
It also adds a setSpanAttribute method that allows for adding attributes to the Span.

We can bikeshed on the naming and implementation.

I've added Otto as a reviewer, as he mentioned wanting to have similar functionality in the Recursor.

This PR also fixes a small over-sight when creating a minimal Protobuf message where we did not add the TraceID to the message.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • read and accepted the Developer Certificate of Origin document, including the AI Policy, and added a "Signed-off-by" to my commits
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@pieterlexis pieterlexis added this to the dnsdist-2.2.0 milestone Mar 9, 2026
@pieterlexis pieterlexis force-pushed the dnsdist-lua-opentelemetry branch from e2c1e34 to b7c9c2e Compare March 9, 2026 12:54
rgacogne
rgacogne previously approved these changes Mar 10, 2026
@pieterlexis
Copy link
Contributor Author

@rgacogne I was thinking that this could be made easier. I could add the setSpanAttribute method to DNSQuestion and remove the Closer from the closure. This would allow 2 things:

  1. Users could set attributes on the Rule's Span
  2. There is no need to capture anything

Rule code would look like this:

function (dq)
  dq:setSpanAttribute("attr-in-the-rulespan", "my-value")
  dq:withTraceSpan(
    "my-span",
    function ()
      -- Do work
      dq:setSpanAttribute("attr-in-my-subspan", "other-value")
    end
  )
  Return DNSAction.None
end

@rgacogne
Copy link
Member

That looks nice indeed, where would the closer live and how does dq reference the correct span?

@omoerbeek
Copy link
Member

I would love to see this a bit simpler indeed.

@pieterlexis
Copy link
Contributor Author

That looks nice indeed, where would the closer live and how does dq reference the correct span?

The closer would live outside of Lua and DNSQuestion:setSpanAttribute would use Tracer::getLastSpanID to get the correct SpanID to pass to Tracer::setSpanAttribute.

@pieterlexis
Copy link
Contributor Author

I've updated the PR with new, simpler method that no longer requires a closer.

rgacogne
rgacogne previously approved these changes Mar 12, 2026
Copy link
Member

@rgacogne rgacogne left a comment

Choose a reason for hiding this comment

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

I love the simplicity of this version, great work!

omoerbeek
omoerbeek previously approved these changes Mar 12, 2026
@pieterlexis pieterlexis force-pushed the dnsdist-lua-opentelemetry branch from a458381 to e82d14e Compare March 16, 2026 16:01
@coveralls
Copy link

coveralls commented Mar 16, 2026

Pull Request Test Coverage Report for Build 23153198208

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 10953 unchanged lines in 110 files lost coverage.
  • Overall coverage decreased (-4.7%) to 66.223%

Files with Coverage Reduction New Missed Lines %
modules/gpgsqlbackend/gpgsqlbackend.cc 1 84.78%
pdns/query-local-address.cc 1 93.62%
pdns/validate.cc 1 68.16%
pdns/burtle.hh 2 97.59%
pdns/channel.hh 2 47.48%
pdns/dns.cc 2 87.39%
pdns/dnsdistdist/dnsdist-lua-bindings-kvs.cc 2 0.0%
pdns/dnsdistdist/dnsdist-lua.hh 2 0.0%
pdns/dnsdistdist/dnsdist-opentelemetry.cc 2 94.85%
pdns/epollmplexer.cc 2 83.85%
Totals Coverage Status
Change from base Build 23152810648: -4.7%
Covered Lines: 107737
Relevant Lines: 150102

💛 - Coveralls

@pieterlexis pieterlexis force-pushed the dnsdist-lua-opentelemetry branch from e82d14e to 7646ccb Compare March 16, 2026 19:46
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