Skip to content

Add latency calculation tests & logs #103

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

Merged
merged 2 commits into from
Apr 17, 2025
Merged

Add latency calculation tests & logs #103

merged 2 commits into from
Apr 17, 2025

Conversation

cesher
Copy link
Contributor

@cesher cesher commented Apr 17, 2025

Added witness latency calculation tests. Specifically the following scenarios are now being tested:

  • the request and the response have 8 ms of latency
  • the request and the response have 0 ms of latency
  • the request and the response overlap by -1 ms of latency
  • the request and the response fully overlap each other have 0 ms of latency
  • the request and the response are in the wrong order have -4 ms of latency
  • the request and the response timestamps are the zero value have 0 ms of latency
  • a stream with two interleaved request and response each with 3 ms of latency
  • two pipelined requests and responses have 0 ms of latency

Added 2 validations in the stream processing layer:

  • if the first or last packet timestamps are the default zero value, override to time.Now().
  • if the last packet timestamp is before the first packet timestamp, override last packet timestamp to first packet timestamp.

^These are being logged and printed as part of the PrintWarnings report at the end of a session.

Added 3 validations in the backend collector layer:

  • if the collector matches 2 requests or 2 responses together (due to HTTP/1.1 pipelining and matching on ACKs), log it and leave latency default value of 0.0.
  • if the collector sees timestamp zero values, log it and leave latency default value of 0.0
  • if the collector sees negative latency calculation, log and report it.

Copy link
Contributor

@mgritter mgritter left a comment

Choose a reason for hiding this comment

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

Looks OK -- do you plan on enabling debugging in beta when you deploy this? (Or do we already have debug log level set?)

pcap.CountBadAssemblerContextType > 0 ||
pcap.CountZeroValuePacketTimestamp > 0 ||
pcap.CountLastPacketBeforeFirstPacket > 0 {
printer.Stderr.Infof("Detected packet assembly context problems during capture: %v empty, %v bad type, %v empty after parse, %v zero timestamp, %v last before first timestamp. ",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good but we should figure out some more K8s-friendly way of showing our counters, for example including them in telemetry or dumping them out periodically to the log.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do this in a follow up PR, I have to first update the schema in the shared libs repo before I can push these values through the telemetry worker.

@cesher cesher merged commit 4240f82 into main Apr 17, 2025
4 checks passed
@cesher cesher deleted the POA-3321 branch April 17, 2025 22:44
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.

2 participants