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

Do not collect max, min and sum aggregates as they are the same as avg #5638

Merged
merged 2 commits into from
Feb 10, 2020

Conversation

FlorianVeaux
Copy link
Member

@FlorianVeaux FlorianVeaux commented Feb 4, 2020

For realtime resources, we have always been collecting max, min and raw rollup metrics as long as the avg one despite all of those having the exact same values. Indeed in this vsphere implementation, we collect only the latest point at the finest resolution. So rollup do not come into play.

This PR prevents those extra rollup types to be collected and updates the fixture file accordingly.

Note: We could have removed all the remaining suffixes (avg, sum and latest), but users are used to see them. Also having sum at the end helps to understand the metric is a "count" (I think). Additionally, the code-change would have been much bigger.

Fixture file was updated with:

f = open('<FILENAME>', 'r')
data = json.load(f)
data = [d for d in data if d['name'].split('.')[-1] not in ('raw', 'max', 'min')]
data = sorted(data, key=lambda x: (x['name'], x.get('hostname')))
f.close()
f = open('<FILENAME>', 'w')
f.write(json.dumps(data, indent=4))

@FlorianVeaux
Copy link
Member Author

Not adding changelog/Changed as this PR should be released with the PR here: #5251 already labeled changelog/Changed

ofek
ofek previously approved these changes Feb 4, 2020
@FlorianVeaux FlorianVeaux merged commit 2897819 into master Feb 10, 2020
@FlorianVeaux FlorianVeaux deleted the florian/simplify_vsphere_metrics branch February 10, 2020 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants