search

sensu / sensu-influxdb-handler

Sensu Go InfluxDB Metrics Handler
https://sensu.io
MIT License
14 stars 16 forks source link

Closes #44 adding enterprise parsing flags #45

Closed hurrycaine closed 3 years ago

hurrycaine commented 3 years ago

Signed-off-by: Hurrycaine jason.e.joyner@gmail.com

hurrycaine commented 3 years ago

There are no changes to functionality if the -e or --enterprise flags are not provided. I did break out some of the logic in the for loop in func sendMetrics.

The more conditionals we added the more that loop was getting harder to follow. I broke out each value into it's own function; setName, setTime, setFields and setTags. This will also allow fore easier testing. I did add some tests to ensure nothing broke and to test the functionality but did not touch any existing tests.

I avoided having a function do two things; the setName and setFields would be a good one to merge and return two values. They both require a split of the measurement name and if it was combined the --host stripping could be put in there and would greatly simplify it's tests.

I also avoided changing the error handling but question on erroring immediately out on a bad timestamp. See func setTime. I think continuing the loop on an error would allowing indexing of all good measuments in a batch and then a total error could error out at the end of sendMetrics.

nixwiz commented 3 years ago

For the option, how about -l and --legacy-format? Otherwise @jspaleta and I think it looks okay.

hurrycaine commented 3 years ago

I like legacy-format as well. I updated the PR and because legacy-format (or enterprise) were not as self explanatory as --strip-host. I added more info to the readme.

jspaleta commented 3 years ago

hey! thanks for the update on the PR. I'm going to test these changes locally in my dev environment and look to get this merged next week.

I'm still looking to build up a github action env with running services to be able to add tests for changes like this as part of normal CI/CD. Just hitting a couple of snags with with github actions.