Add kinesis output support #428
Conversation
|
Thank you @jimmystewpot! It seems like your unit test is assuming there will be some authentication credentials available? Is that true? We may not want to have a "test connect and write" for this particular output, as it is quite a bit different than the others, unless there happens to be a docker container somewhere that can Mock the kinesis write endpoint locally. |
| return errors.New("Error") | ||
| } | ||
|
|
||
| func formatmetric(k *KinesisOutput, point *client.Point) (string, error) { |
There was a problem hiding this comment.
you could write a unit test for this function at least
There was a problem hiding this comment.
As opposed to the full connect and write test
There was a problem hiding this comment.
Updating unit tests now. will add another commit soon.
| } | ||
|
|
||
| func init() { | ||
| outputs.Add("kinesis_output", func() outputs.Output { |
There was a problem hiding this comment.
One last nitpick 😅, call the output just kinesis for consistency and reduce redundancy
There was a problem hiding this comment.
I am planning to write a kinesis monitoring plugin which is why I named it output. Is this bad practise?
There was a problem hiding this comment.
@jimmystewpot, yes, it should just be named kinesis, in the config they will appear in two different sections outputs.kinesis and plugins.kinesis. We could also call the monitoring plugin kinesis_consumer which would be consistent with the kafka naming that we have now.
There was a problem hiding this comment.
I have updated the PR to include the rename.
|
Updated the name of the plugin as per your request. I have also removed the connection unit test so that the tests can pass. |
|
Awesome, thank you @jimmystewpot! I'll fixup the |
This is a PR to add an output plugin that writes to Kinesis.
I have attempted to keep it as simple and small as possible. The tests pass on my local system and on other amazon instances where there are provided credentials.