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

(cloudwatch) support high resolution query #10925

Merged
merged 2 commits into from
Mar 8, 2018

Conversation

mtanda
Copy link
Collaborator

@mtanda mtanda commented Feb 15, 2018

@mtanda mtanda force-pushed the cw_high_resolution branch from 999eaf7 to 18c54a9 Compare February 15, 2018 14:35
@mtanda
Copy link
Collaborator Author

mtanda commented Feb 15, 2018

I want to more good option name. "highResolution" is not good...

@codecov-io
Copy link

codecov-io commented Feb 15, 2018

Codecov Report

Merging #10925 into master will decrease coverage by 0.02%.
The diff coverage is 9.09%.

@@            Coverage Diff             @@
##           master   #10925      +/-   ##
==========================================
- Coverage   51.47%   51.45%   -0.03%     
==========================================
  Files         343      343              
  Lines       24244    24258      +14     
  Branches     1417     1417              
==========================================
+ Hits        12480    12482       +2     
- Misses      11069    11081      +12     
  Partials      695      695

@mauve
Copy link
Contributor

mauve commented Feb 19, 2018

LGTM, regarding name I also cannot come up with a good name for the setting.

@mauve
Copy link
Contributor

mauve commented Feb 19, 2018

@mtanda also thanks for looking into this I tried it locally but ran out of time

@@ -274,6 +293,11 @@ func parseQuery(model *simplejson.Json) (*CloudWatchQuery, error) {
alias = "{{metric}}_{{stat}}"
}

highResolution, err := model.Get("highResolution").Bool()
Copy link
Contributor

Choose a reason for hiding this comment

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

When I test this PR I get an error.

This should be .MustBool(false)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for check!
Fix it.

@bergquist
Copy link
Contributor

Awesome! :) Great work

@bergquist bergquist merged commit a589f70 into grafana:master Mar 8, 2018
bergquist added a commit that referenced this pull request Mar 8, 2018
@mtanda
Copy link
Collaborator Author

mtanda commented Mar 9, 2018

Thanks!!

@rehevkor5
Copy link

I can't find info in the docs about the "HighRes" checkbox. What does it actually do?

@ying-jeanne ying-jeanne added the pr/external This PR is from external contributor label Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/external This PR is from external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Cloudwatch undesired time aggregation
6 participants