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
plugin/metrics: support HTTPS qType in requests count metric label #4934
Conversation
This needs a DCO sign-off. You can use |
Signed-off-by: Pavol Lieskovský <pavol.lieskovsky@wandera.com>
@SuperQ added, thank you for the info 👍 |
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
happy to add this, but isn't the HTTPS still in draft? Might be prudent to wait until RFC. yeah it still draft: https://datatracker.ietf.org/doc/draft-ietf-dnsop-svcb-https/ |
@miekg ah, that's interesting site/fact. Didn't know about it. But as you guys are the owners/maintainers of the repo, the decision is totally up to you :) |
[ Quoting ***@***.***> in "Re: [coredns/coredns] plugin/metric..." ]
***@***.*** ah, that's interesting site/fact. Didn't know about it.
On the other hand, iOS has been already using it for about 4 months. We can see
a non-trivial amount (~18%) of HTTPS requests sent to our CoreDNS instance that
faces the internet. So I guess we cannot go wrong with enabling the computation
of it into the metric label.
But as you guys are the owners/maintainers of the repo, the decision is totally
up to you :)
yeah, it's a label value so easily changed and I can see this RR being import, but it
might still see a name change. SVCB might be nice as well (same caveats)
/lgtm
/merge
|
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.
Approved by miekg
…oredns#4934) Automatically submitted. Signed-off-by: jinglinax@163.com <jinglinax@163.com>
1. Why is this pull request needed and what does it do?
Since iOS14, apple devices are standardly using the HTTPS query (type 65). It would be beneficial to be able to distinguish the amount of these requests based on the metrics. Therefore I would like to add the HTTPS query type support into the metric labels.
2. Which issues (if any) are related?
none
3. Which documentation changes (if any) need to be made?
none
4. Does this introduce a backward incompatible change or deprecation?
no