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

Thread id, discarded #8

Merged
merged 1 commit into from
May 30, 2024
Merged

Thread id, discarded #8

merged 1 commit into from
May 30, 2024

Conversation

jelu
Copy link
Member

@jelu jelu commented May 29, 2024

  • Rename threadid to subid and clarified that it can be used to differentiate between threads or streams
  • stats: Add discarded

- Rename `threadid` to `subid` and clarified that it can be used to differentiate between threads or streams
- `stats`: Add `discarded`
@@ -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

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?

@jelu jelu requested a review from a team May 30, 2024 07:55
@jelu jelu merged commit e7027f7 into DNS-OARC:main May 30, 2024
1 check passed
@jelu jelu deleted the threadid branch May 30, 2024 08:26
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.

3 participants