-
Notifications
You must be signed in to change notification settings - Fork 446
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
Add Cloudflare package #984
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
run tests |
c00e243
to
b00577c
Compare
7c56912
to
9afde7c
Compare
packages/cloudflare/data_stream/logpull/agent/stream/httpjson.yml.hbs
Outdated
Show resolved
Hide resolved
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
5379c47
to
686a8ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution :D!
Aside from the couple comments, would be nice to add in some of the latest improvements/practices. If you prefer I can push the changes myself or I can help you in doing them:
1- For the ecs fields file, now you can reference directly the ECS definitions from the original repository, take a look at https://github.com/elastic/integrations/pull/971/files#diff-ab0b8ba7ee65eeaec37ad049da3278b9e3f27eafc297ef39b4d97d5557f1e3d7R1 and https://github.com/elastic/integrations/pull/971/files#diff-e25525aa40a63de53545aad0489afe6dd123f5840076dfae30b3f0ad63da1edbR1 for an example, this saves a lot of boilerplate and makes for consistent descriptions across packages.
2- Instead of the _conf
field to decide to keep or not the event.original
, you can take https://github.com/elastic/integrations/pull/971/files#diff-3e0dc30780e24af54910c65b1217d914992e35cf87045ed76728285b3851ec9bR1 as an example of how we standardised it across all packages, in addition for a custom processors
field. You might also want to check how the option is handled in the config and in the pipeline
Also if you can add a couple screenshots of the dashboards that would be great.
Thanks!
packages/cloudflare/data_stream/logpull/elasticsearch/ingest_pipeline/http.yml
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/elasticsearch/ingest_pipeline/http.yml
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/elasticsearch/ingest_pipeline/http.yml
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/elasticsearch/ingest_pipeline/http.yml
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/_dev/test/pipeline/test-http-json.log-expected.json
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/_dev/test/pipeline/test-http-json.log-expected.json
Outdated
Show resolved
Hide resolved
@marc-gr I think i resolved all the issues you brought up in the first round :). |
@marc-gr I also found an issue that the |
They should be safe to add in those, according to https://www.elastic.co/guide/en/ecs/current/ecs-geo.html, note that AS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! after the last comments I think it should be good to go 👍
packages/cloudflare/data_stream/logpull/_dev/test/pipeline/test-http-json.log-expected.json
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/_dev/test/pipeline/test-http-json.log-expected.json
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/agent/stream/httpjson.yml.hbs
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/agent/stream/log.yml.hbs
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/_dev/test/system/test-default-config.yml
Outdated
Show resolved
Hide resolved
packages/cloudflare/data_stream/logpull/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
52f35f5
to
978a3aa
Compare
/test |
* Initial commit * add dashboards and visualizations * update dashboards * update dashboards * update visualizations * update * format package * update sample log file * update package * upodate pipeline * change datastream to logpull * update pipeline & vis * update the dashboards & pipeline * Updates from comments * Update fields mappings * Add httpjson system tests and sample event * Remove log input and do minor changes to pipeline and configuration * Update dashboards to use a core saved search * Export dashboards and add missing event fields Co-authored-by: Marc Guasch <[email protected]>
What does this PR do?
Add Cloudflare integration
Checklist
changelog.yml
file.How to test this PR locally
Related issues
Screenshots