Skip to content

Simplify dns.question.name note to remove custom encoding requirements#2952

Merged
lmolkova merged 5 commits intomainfrom
copilot/fix-dns-question-name
Oct 22, 2025
Merged

Simplify dns.question.name note to remove custom encoding requirements#2952
lmolkova merged 5 commits intomainfrom
copilot/fix-dns-question-name

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 21, 2025

  • Review current dns.question.name note in model/dns/registry.yaml
  • Update the note to reference Punycode (RFC 3492) instead of custom encoding scheme
  • Run make table-generation attribute-registry-generation to regenerate markdown files
  • Verify the generated docs/registry/attributes/dns.md reflects the changes
  • Run linting and validation checks to ensure changes are correct (markdownlint, table-check, schema-check, misspell)
  • Add changelog entry for the fix
  • Simplify note text per review feedback to focus on capturing values as-is
  • Update changelog entry to reflect the simplified approach
Original prompt

This section details on the original issue you should resolve

<issue_title>Fix dns.question.name note for non-ascii, refer to Punycode instead of custom conversion</issue_title>
<issue_description>

**[1] `dns.question.name`:** If the name field contains non-printable characters (below 32 or above 126), those characters should be represented as escaped base 10 integers (\DDD). Back slashes and quotes should be escaped. Tabs, carriage returns, and line feeds should be converted to \t, \r, and \n respectively.

If the name field contains non-printable characters (below 32 or above 126), those characters should be represented as escaped base 10 integers (\DDD). Back slashes and quotes should be escaped. Tabs, carriage returns, and line feeds should be converted to \t, \r, and \n respectively.

Seems to be incorrect. DNS supports ASCII only and there is rfc3492 on how to record unicode in domain names which looks like xn--s1ab.xn--80ab0c:80 - this is called Punycode.

I'd like to revisit the need to do any type of normalization or escaping - instrumentation on the DNS level is likely to deal with IP address primitive provided by the standard library which is already encoded.

Knowing that unencoded (invalid) address is submitted for resolution is important, and there are no constraints in OTel that would require special encoding. We can mention that it's likely in Punycode, but no normalization is needed.
</issue_description>

<agent_instructions>The source file that needs to be changed is model/dns/registry.yaml on the attribute dns.question.name. After that, we need to run the make target make table-generation attribute-registry-generation to re-generate the markdown files. </agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes #2143


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix dns.question.name note to reference Punycode Fix dns.question.name note to refer to Punycode instead of custom encoding Oct 21, 2025
Copilot AI requested a review from joaopgrassi October 21, 2025 11:29
@joaopgrassi
Copy link
Copy Markdown
Member

@lmolkova did this achieve what you wanted initially? I think it did a pretty good job 👍

@joaopgrassi
Copy link
Copy Markdown
Member

@copilot mark as ready for review

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 21, 2025

@copilot mark as ready for review

I don't have the ability to mark the PR as ready for review - that needs to be done through the GitHub UI. The changes have been completed and all validation checks have passed (markdownlint, table-check, schema-check, and misspell).

@joaopgrassi
Copy link
Copy Markdown
Member

joaopgrassi commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 21, 2025

@copilot add a changelog. See here: https://github.com/open-telemetry/semantic-conventions/blob/main/CONTRIBUTING.md#5-changelog

Added changelog entry in commit 6b0e31f.

@joaopgrassi joaopgrassi marked this pull request as ready for review October 21, 2025 11:39
@joaopgrassi joaopgrassi requested review from a team as code owners October 21, 2025 11:39
@joaopgrassi joaopgrassi moved this from Untriaged to Needs More Approval in Semantic Conventions Triage Oct 21, 2025
Comment thread model/dns/registry.yaml Outdated
Copilot AI changed the title Fix dns.question.name note to refer to Punycode instead of custom encoding Simplify dns.question.name note to remove custom encoding requirements Oct 21, 2025
Copilot AI requested a review from lmolkova October 21, 2025 15:45
Comment thread .chloggen/copilot_fix-dns-question-name.yaml Outdated
@joaopgrassi
Copy link
Copy Markdown
Member

@copilot update the changelog note and subtext after the latest requested changes

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 22, 2025

@copilot update the changelog note and subtext after the latest requested changes

Updated the changelog note and subtext in commit d4d1b2a to reflect the simplified approach of capturing domain names as-is without normalization.

@lmolkova lmolkova added this pull request to the merge queue Oct 22, 2025
Merged via the queue into main with commit d1ad914 Oct 22, 2025
35 of 36 checks passed
@lmolkova lmolkova deleted the copilot/fix-dns-question-name branch October 22, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Fix dns.question.name note for non-ascii, refer to Punycode instead of custom conversion

5 participants