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

Implement live internal latency tracker #651

Merged
merged 9 commits into from
Dec 9, 2024
Merged

Conversation

gnawf
Copy link
Collaborator

@gnawf gnawf commented Dec 6, 2024

Adds a live internal latency tracker so we can emit internal latency metrics.

Copy link

github-actions bot commented Dec 6, 2024

Test Results

  135 files  + 2  135 suites  +2   57s ⏱️ +2s
1 010 tests +12  945 ✅ +12  65 💤 ±0  0 ❌ ±0 
1 018 runs  +12  953 ✅ +12  65 💤 ±0  0 ❌ ±0 

Results for commit c395dc4. ± Comparison against base commit 0025f15.

This pull request removes 1 and adds 13 tests. Note that renamed tests count towards both.
graphql.nadel.engine.instrumentation.NadelInstrumentationTimerTest ‑ combines times together
graphql.nadel.NadelStopwatchTest ‑ invoking start running stopwatch does nothing()
graphql.nadel.NadelStopwatchTest ‑ invoking stop on stopped stopwatch does nothing()
graphql.nadel.NadelStopwatchTest ‑ returns elapsed time if stopwatch is still running()
graphql.nadel.NadelStopwatchTest ‑ returns zero if stopwatch is never started()
graphql.nadel.NadelStopwatchTest ‑ stopwatch can be resumed()
graphql.nadel.NadelStopwatchTest ‑ stopwatch elapsed is frozen when stopped()
graphql.nadel.engine.instrumentation.NadelInstrumentationTimerTest ‑ takes the highest time in a batch
graphql.nadel.time.NadelInternalLatencyTrackerImplTest ‑ keeps internal latency paused if there are still external calls pending()
graphql.nadel.time.NadelInternalLatencyTrackerImplTest ‑ stops time as code is running()
graphql.nadel.time.NadelInternalLatencyTrackerImplTest ‑ stops time as future is running()
…

♻️ This comment has been updated with latest results.

@@ -13,7 +14,8 @@ data class NadelExecutionInput private constructor(
val variables: Map<String, Any?>,
val extensions: Map<String, Any?>,
val executionId: ExecutionId?,
val nadelExecutionHints: NadelExecutionHints,
val executionHints: NadelExecutionHints,
val latencyTracker: NadelInternalLatencyTracker,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Latency tracker is per request & will be passed in by the caller.

// When
assertTrue(stopwatch.elapsed().toNanos() == 0L)
time = 100
assertTrue(stopwatch.elapsed().toNanos() == 0L)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this elapsed not 100?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahh I see ignore me

@gnawf gnawf merged commit 0d71072 into master Dec 9, 2024
3 checks passed
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