Fix: allow probers to provide a duration value #898
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
Some probers need to override the duration that is reported, mostly because we use probe_duration_seconds as the "latency" of the probe, which isn't exactly correct. `probe_duration_seconds` was meant to be used as the duration of the probe: how long did it take for this probe to run. Capture that value in the logs for debugging purposes, and use `probe_duration_seconds` as if it meant the time it takes the target to produce a response, including *some* setup, but not e.g. the overhead introduced by retries or the time it takes for external programs to spin up. This is OK to change since these are all internal packages. Signed-off-by: Marcelo E. Magallon <[email protected]>
|
This is the second time I have needed this functionality. The first time I didn't want the big noisy change, so I hacked around it. For the second time around, I think it's better to do it right. This will benefit all k6-based checks, too. Maybe traceroute, too? |
mem
commented
Sep 19, 2024
| @@ -90,7 +91,9 @@ func probeICMP(ctx context.Context, target string, module Module, registry *prom | |||
|
|
|||
| pinger.OnSetup = func() { | |||
| if !setupDone { | |||
| durationGaugeVec.WithLabelValues("setup").Add(time.Since(setupStart).Seconds()) | |||
| setupDuration := time.Since(setupStart).Seconds() | |||
There was a problem hiding this comment.
This is the actual change I need. :-P
mem
added a commit
that referenced
this pull request
Sep 19, 2024
* Feature: add retries to ICMP prober (#896) * Fix: allow probers to provide a duration value (#898) Signed-off-by: Marcelo E. Magallon <[email protected]>
Merged
mem
added a commit
that referenced
this pull request
Sep 19, 2024
* Feature: add retries to ICMP prober (#896) * Fix: allow probers to provide a duration value (#898) Signed-off-by: Marcelo E. Magallon <[email protected]>
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.
Some probers need to override the duration that is reported, mostly because we use probe_duration_seconds as the "latency" of the probe, which isn't exactly correct.
probe_duration_secondswas meant to be used as the duration of the probe: how long did it take for this probe to run. Capture that value in the logs for debugging purposes, and useprobe_duration_secondsas if it meant the time it takes the target to produce a response, including some setup, but not e.g. the overhead introduced by retries or the time it takes for external programs to spin up.This is OK to change since these are all internal packages.