Hey All, So I’ve been testing the new addition of ...
# generalf
fancy-waitress-35577
10/25/2021, 1:08 PMHey All, So I’ve been testing the new addition of --sync-usage-file and run into a small issue when testing it with atlantis_diff.
Namely if I add the option for it to the diff script (will send a PR after I get the test working) then it creates the usage file from scratch fine but it seems that it breaks the summary json file by not grepping out the below line before outputting to json.
Copy code
└─ Synced 0 of 6 resourcesfancy-waitress-35577
10/25/2021, 1:10 PMDebug :
Copy code
time="2021-10-25T13:09:38Z" level=info msg="starting: Syncing usage data from cloud"
time="2021-10-25T13:09:38Z" level=info msg="starting: Calculating monthly cost estimate"
Running infracost output using:
$ infracost output --no-color --format diff --path infracost_breakdown.json
Error: Error parsing JSON file: invalid character 'â' looking for beginning of valuel
little-author-61621
10/25/2021, 2:12 PM
Thanks @fancy-waitress-35577, yeah this looks like a bug. I've managed to reproduce it.
The fix will be to change this line https://github.com/infracost/infracost/blob/master/cmd/infracost/run.go#L109 to
cmd.PrintErrlnlittle-author-61621
10/25/2021, 2:14 PM
Let me know if you want to send a PR for this. Otherwise, I can do it. We'll get this fix out in the next release (Wednesday).
f
fancy-waitress-35577
10/25/2021, 2:14 PMAh stdout vs stderr. Ok
fancy-waitress-35577
10/25/2021, 2:14 PMI can add it to my branch since it already has the addition of new var for the --sync-usage-file to atlantis
l
little-author-61621
10/25/2021, 2:17 PM
🙌 thanks!
w
white-airport-8778
10/25/2021, 2:35 PM
@fancy-waitress-35577 out of interest, why are you syncing the usage file in atlantis? because that has the creds to call cloudwatch?
f
fancy-waitress-35577
10/25/2021, 2:39 PMSo I think the answer is twofold -
1. Cause I didn’t actually think about syncing it on my local machine before messing with atlantis already 😉
2. Still figuring out the process / testing everything but my original idea was that if atlantis could do it automatically this would cut down on the manual step
fancy-waitress-35577
10/25/2021, 2:40 PMBut longterm I think it would be good if this was automatic - out of sight out of mind type of thing where this will just work automatically to supplement the already great estimates.
w
white-airport-8778
10/25/2021, 2:41 PM
That makes sense! Recently I’ve also been thinking whether “sync from cloudwatch” should happen automatically, as it provides a more accurate estimate, so it could be tried and we can fall-back to just showing prices if it fails to connect with the cloud (there could be an option to disable this for ppl who do not want it).
f
fancy-waitress-35577
10/25/2021, 2:42 PMYep - so for now my current testing was focused on adding a new env var switch in atlantis-diff which when set to true will try to do the sync
fancy-waitress-35577
10/25/2021, 2:42 PMI guess looking at the synced 0 out of 6 it already fails open so if it doesn’t work no big deal
👍 1
w
white-airport-8778
10/25/2021, 2:45 PM
I summarized the above learning in https://github.com/infracost/infracost/discussions/985, feel free to comment further - this was very helpful 🙂
👍 1
f
fancy-waitress-35577
10/25/2021, 2:57 PMSent a PR with my hacky addition of the new env var / and the fix mentioned by @little-author-61621 https://github.com/infracost/infracost/pull/1075
🙌 1
b
brave-fireman-95738
10/26/2021, 11:10 AMRe: auto-sync from CloudWatch, I am being very paranoid, but wondered: how aggressive are CloudWatch rate limits? Could Infracost exhaust them in a sufficiently large deployment? (which could have negative impact across wider orgs, obviously). But again, a bit paranoid here.
w
white-airport-8778
10/26/2021, 11:13 AM
Good point, the PR only added an option so users can enable it if they want (it’s off by default), but if we move towards having that on by default then some sort of caching is probably needed, specially in CI/CD
36 Views