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

Added custom decode function to gunzip cloudwatch kinesis messages #12119

Closed
wants to merge 7 commits into from

Conversation

thenom
Copy link
Contributor

@thenom thenom commented May 9, 2019

Apologies for this in advance... This is my first attempt at Go and not much of a coder but needed to get a solution in place. My build is currently running in our test environment without issue. I have been advised by our support contact to submit this for review. I am assuming testing would need to be put in place for this and for this feature to be switchable.

This is to gzip -d the messages that come from a kinesis stream that has flowed through the following solution:

https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/CrossAccountSubscriptions.html

Remote account cloudwatch -> local account cloudwatch logs destination -> kinesis stream.

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@ph
Copy link
Contributor

ph commented May 9, 2019

Thanks for that, changes looks good but I wonder if we should instead make it a processors, WDYT @kvch?

@thenom
Copy link
Contributor Author

thenom commented May 9, 2019

Created a test to check for both compressed and uncompressed data messages and all now pass. Also changed the function so it can be passed both but will return the original message if not gzip compressed. This should hopefully cover the switchable problem.

Like i say, i am still new at Go so you lost me at 'processors' (will look it up though) but let me know if you want anything changing.

@ph ph self-assigned this May 23, 2019
@ph
Copy link
Contributor

ph commented May 23, 2019

@thenom What I think this functionality should be a processors instead of part of kinesis, this will allow it to be used by other inputs defined in filebeat.

We could call it gzip_field_decoder and people could use it like this.

processors:
 - gzip_field_decoder: ~

@thenom
Copy link
Contributor Author

thenom commented May 23, 2019

Ah, sorry, i see what you mean now, i thought it was a Go thing. I can have a look into implementing this, not sure how far i will get but interesting in having a go.

@ph
Copy link
Contributor

ph commented May 23, 2019

Thanks @thenom for looking into this, I will help you move it forward just ping me directly on the PR.

@thenom
Copy link
Contributor Author

thenom commented May 25, 2019

Hi @ph,

I have created another branch to work on the dedicated processor. I have blatantly stolen the source code for decode_csv_fields as a base and reworked it for this processor. I have also done the tests for this processor and they pass. Can you give it a once over and let me know anything else that needs doing. I have not compiled and run this in our AWS environment yet as i have been on holiday so if you think this is OK then i will rework our dev environment when i get back in.

I have had a look into doing the documentation for this but it looks like it is some sort of auto generated page but let me know if i am wrong.

https://github.com/thenom/beats/tree/gzip_decompress

@kvch
Copy link
Contributor

kvch commented May 27, 2019

@ph I agree it would be better if the a base64 and a gunzip processor were added to libbeat. But in case of Kinesis I would like to avoid having the user configure it. I mean when a Kinesis function is configured, we should hide how the message is parsed.

To illustrate what I mean there is a configuration snippet which I would like to avoid:

functionbeat.provider.aws.functions: 
   - name: kinesis
     enabled: true
     type: kinesis
     description: "lambda function for Kinesis events"
     processors:
      - base64
      - gunzip

I would rather introduce an option named decompress. By default it could be true, but if someone needs the compressed events, he/she can set it to false.

functionbeat.provider.aws.functions: 
   - name: kinesis
     enabled: true
     type: kinesis
     description: "lambda function for Kinesis events"
     decompress: true

WDYT?

@thenom
Copy link
Contributor Author

thenom commented May 28, 2019

So if this was the route taken, would just adding this switch to my current function be acceptable?

@ph
Copy link
Contributor

ph commented Jun 4, 2019

@kvch I still believe composition is better in the specific case, because we can reuse directly for SQS and kinesis or anything else without adding new options.

@kvch
Copy link
Contributor

kvch commented Jun 4, 2019

@thenom I have looked at your other branch. It seems fine, but I might add a few notes. Do you mind opening a PR for that branch, so we can discuss your changes?

@thenom
Copy link
Contributor Author

thenom commented Jun 5, 2019

@ph i have submitted:
#12438

Thanks

@kvch
Copy link
Contributor

kvch commented Jul 1, 2019

Closing as #12733 is merged

@kvch kvch closed this Jul 1, 2019
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.

None yet

5 participants