RUMM-1744 Add E2E tests for Kronos (NTP) #703
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
so it can be monitored in nightly runs.
ncreated
commented
Dec 30, 2021
Comment on lines
+99
to
+106
| // Inconsistent result may correspond to flaky execution, e.g. if network was unreachable or if **all** NTP calls received timeout. | ||
| // We track inconsistent result as WARN log that will be watched by E2E monitor. | ||
| logger.warn("KronosClock.sync() completed with inconsistent result for \(ddNTPPool)", attributes: [ | ||
| "serverDate_firstReceived": result.firstReceivedDate.flatMap { iso8601DateFormatter.string(from: $0) }, | ||
| "serverDate_lastReceived": result.lastReceivedDate.flatMap { iso8601DateFormatter.string(from: $0) }, | ||
| "serverOffset_firstReceived": result.firstReceivedOffset, | ||
| "serverOffset_lastReceived": result.lastReceivedOffset, | ||
| ]) |
There was a problem hiding this comment.
Example WARN log received from here:
ncreated
commented
Dec 30, 2021
Comment on lines
+181
to
+186
| logger.info( | ||
| "KronosNTPClient.query(pool:) completed with consistent result receiving \(result.numberOfCompletedSamples)/\(result.expectedNumberOfSamples) NTP packets", | ||
| attributes: [ | ||
| "offsets_received": receivedOffsets | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Example INFO log received from here:
buranmert
approved these changes
Dec 31, 2021
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What and why?
⚙️🧪 This PR adds E2E tests for Kronos and its NTP sync logic. It removes tests which perform real UDP from unit tests target and moves them to E2E target which is ran nightly.
This is to:
How?
I added tests for high level Kronos APIs:
KronosClock.sync()- to test it against all 4 Datadog NTP pools;KronosNTPClient.query(pool:)- to test it against all IPv4 and IPv6 addresses from2.datadog.pool.ntp.org;Each test sends either INFO or WARN log upon its completion. This will be later (
RUMM-1859) used to create E2E monitors in Mobile Integration org:Similar, each test is measured by performance span, so we can understand how fast Kronos performs its job, e.g.:
Later (next PRs), we will use this system to send more logs regarding internal telemetry from Kronos to better understand its performance on more data points from nightly runs. Hopefully this will led to more conclusion for #647. In parallel, we will send similar telemetry from our dogfood project to collect samples from Datadog iOS app in production.
Review checklist