Skip to content

Conversation

@mwillbanks
Copy link
Contributor

This commit is two part:

  1. getActiveValueAttribute should always return the active attribute
    without any further calculation here. My guess is that this should also happen with sum but I was not going to modify it due to implications on the PR itself.

  2. The query to create the average was incorrect and was taking the
    count value * the value which we are really looking for the average of
    the value.

This commit is two part:

1. getActiveValueAttribute should always return the active attribute
without providing an average since the active is the current value.
2. The query to create the average was incorrect and was taking the
count value * the value which we are really looking for the average of
the value.
@jbrooksuk
Copy link
Member

Hmm, is this correct? The counter is used as a "how many times has this value been receieved in x time?" so surely we'd still need that, even for averages.

@jbrooksuk
Copy link
Member

The query to create the average was incorrect and was taking the
count value * the value which we are really looking for the average of
the value.

No it wasn't? It was value * counter, then averaged that out.

@jbrooksuk jbrooksuk added this to the V2.4.0 milestone Dec 8, 2016
@mwillbanks
Copy link
Contributor Author

@jbrooksuk so while it looks like that was the case, then there are other issues. When I was pushing in values once per second, the value on the chart would increment, say an average of 12 would be quickly be 200+.

So in this case, you would sum the value and sum the counter and then take the division over a specific time period.

@jbrooksuk
Copy link
Member

Do you need to change the threshold then?

@mwillbanks
Copy link
Contributor Author

Aha, this looks to be the bug that is mentioned in: #1922

@jbrooksuk jbrooksuk merged commit 8b5a4fc into cachethq:2.4 Jan 2, 2017
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