Skip to content

Fix time_adjusted calculations #40

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 1 commit into from
May 29, 2019
Merged

Fix time_adjusted calculations #40

merged 1 commit into from
May 29, 2019

Conversation

tombruijn
Copy link
Member

Some context about the time_adjusted function

The time_adjusted function returns a value calculated for a minute. It
received two values, for which it calculates the delta first. Then,
using the time_difference_ns value (the time in which these values
were measured), it calculates the value as it would be for a full
minute.

When the time_difference_ns is 59 seconds or 61 seconds it returns the
value as it would be for 60 seconds instead. This helps account for
small drift within the time at which the metrics were measured.

The problem

The previous implementation of the time_adjusted function would multiple
the delta value by the real time (30 seconds) and divide that value by
60 seconds.

The problem here was that it would not return the value for 60 seconds,
but instead that for 15 seconds, half of 30 seconds.

delta = 10.0
time_difference = 30.0
expected_time = 60.0

delta * real_time / expected_time
=> 5.0

5.0 being half of 10.0 (the delta), half of 30 seconds.

The fix

Instead of multiplying the delta by time_difference_ns we should
divide the delta by the time_difference_ns to get the value per
nanosecond (ns). Then we multiply that 1 nanosecond value by 60 seconds.
This will return the average value for 60 seconds.

delta = 10.0
time_difference = 30.0
expected_time = 60.0

delta / real_time * expected_time
=> 20.0

20.0 being double of 10.0 (the delta), double of 30 seconds.

## Some context about the `time_adjusted` function

The `time_adjusted` function returns a value calculated for a minute. It
received two values, for which it calculates the delta first. Then,
using the `time_difference_ns` value (the time in which these values
were measured), it calculates the value as it would be for a full
minute.

When the `time_difference_ns` is 59 seconds or 61 seconds it returns the
value as it would be for 60 seconds instead. This helps account for
small drift within the time at which the metrics were measured.

## The problem

The previous implementation of the time_adjusted function would multiple
the delta value by the real time (30 seconds) and divide that value by
60 seconds.

The problem here was that it would not return the value for 60 seconds,
but instead return that for around 15 seconds, half of 30 seconds.

```
delta = 10.0
time_difference = 30.0
expected_time = 60.0

delta * real_time / expected_time
=> 5.0
```

5.0 being half of 10.0 (the delta), half of 30 seconds.

## The fix

Instead of multiplying the delta by `time_difference_ns` we should
divide the delta by the `time_difference_ns` to get the value per
nanosecond (ns). Then we multiply that 1 nanosecond value by 60 seconds.
This will return the average value for 60 seconds.

```
delta = 10.0
time_difference = 30.0
expected_time = 60.0

delta / real_time * expected_time
=> 20.0
```

20.0 being double of 10.0 (the delta), double of 30 seconds.
@tombruijn tombruijn merged commit e2ac041 into master May 29, 2019
@tombruijn tombruijn deleted the time_adjusted-fix branch May 29, 2019 08:23
tombruijn added a commit to appsignal/appsignal-ruby that referenced this pull request Jul 22, 2019
- Fix container CPU runtime metrics.
  See appsignal/probes-rs#38 for more
  information.
- Improve host metrics calculations accuracy for counter metrics.
  See appsignal/probes-rs#40 for more
  information.
- Support Kernel 4.18+ format of /proc/diskstats file parsing.
  See appsignal/probes-rs#39 for more
  information.
tombruijn added a commit to appsignal/appsignal-elixir that referenced this pull request Jul 22, 2019
- Fix container CPU runtime metrics.
  See appsignal/probes-rs#38 for more
  information.
- Improve host metrics calculations accuracy for counter metrics.
  See appsignal/probes-rs#40 for more
  information.
- Support Kernel 4.18+ format of /proc/diskstats file parsing.
  See appsignal/probes-rs#39 for more
  information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants