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.

Sign up for GitHub

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

Jump to bottom

Thread id, discarded #8

Merged
merged 1 commit into from
May 30, 2024
Merged

Thread id, discarded #8

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions dns-metrics.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
"type": "object",
"properties": {
"runid": {
"description": "The unique id for a run",
"description": "The unique id for a run, used to correlate all objects to the same run",
"type": "string"
},
"threadid": {
"description": "The unique thread id within a run",
"subid": {
"description": "The unique id within a run, used to differentiate for example individual threads or streams within the run",
"type": "string"
}
},
Expand Down Expand Up @@ -99,7 +99,7 @@
},
"stats": {
"title": "DNS Metric Stats",
"description": "A stats object with the metrics",
"description": "A stats object with the metrics around the DNS that was sent and received during the run",
"type": "object",
"properties": {
"since": {
Expand Down Expand Up @@ -130,6 +130,10 @@
"description": "The number of DNS queries and/or responses that was unexpected",
"type": "integer"
},
"discarded": {
"description": "The number of DNS queries and/or responses that was discarded",
Copy link

@pspacek pspacek May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, this is an excellent point. I think we need counter for queries "not sent because it's garbage" and "not received because it's garbage". I would go for discarded_dns_request discarded_dns_response or something like that.

Note the dns - as mentioned somewhere in @libor-peltan-cznic's notes there are two types of "discarded" - DNS and non-DNS:

  • DNS type is that something does not parse as DNS payload even though it should be a DNS payload - e.g. during PCAP replay a packet is too short to be a DNS message
  • non-DNS type is that e.g. network stack (or driver if we are in AF_XDP land) refused to send stuff - e.g. because buffers were full

In general both counters should be zero all the time, but if they are not it's very important to distinguish the two cases - one indicates problem in test data and the other with test environment itself.

Copy link

Choose a reason for hiding this comment

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

I got confused by "counting counters", so here's what I meant:

  • discarded_dns_request
  • discarded_dns_response
  • discarded_other_request
  • discarded_other_response

Of course it can be somehow put into sub-objects etc. I think you get the idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that garbage in input shouldn't be represented in these stats, since if your input has garbage you would always add those stats into this then. So, you should clean your input! Or we need an object about the input.

But splitting queries/responses for discarded, timeouts, interrupted, unexpected sure, but can I merge this and do that in another PR?

"type": "integer"
},
"response_rcodes": {
"description": "The number of different RCODEs as an object with the RCODE as a property",
"type": "object",
Expand Down