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

[GCS] Adding new integration for Custom GCS Input #4692

Merged
merged 10 commits into from
Feb 7, 2023

Conversation

P1llus
Copy link
Member

@P1llus P1llus commented Nov 22, 2022

What does this PR do?

This adds an integration for the new Google Cloud Storage input that was added to filebeat in 8.5.0

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@P1llus
Copy link
Member Author

P1llus commented Nov 22, 2022

There is currently a few questions up for discussion here:

  1. Will we add system tests, and how we should do it.
  2. Which ECS fields should we use, continuing a discussion around if we should include at least fields related to common metadata processors.
  3. Needs a new icon.

@elasticmachine
Copy link

elasticmachine commented Nov 22, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-02-07T07:37:36.361+0000

  • Duration: 14 min 58 sec

Test stats 🧪

Test Results
Failed 0
Passed 3
Skipped 0
Total 3

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Nov 22, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (0/0) 💚
Files 100.0% (0/0) 💚
Classes 100.0% (0/0) 💚
Methods 100.0% (3/3) 💚
Lines 100.0% (0/0) 💚 11.296
Conditionals 100.0% (0/0) 💚

@P1llus
Copy link
Member Author

P1llus commented Nov 22, 2022

Integration has not been fully tested yet, but will be soon (before merging)

@P1llus
Copy link
Member Author

P1llus commented Nov 23, 2022

Waiting for this to merge to add system tests: elastic/stream#46

Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just minor change requested. Rest LGTM 👍🏼

{{#if service_account_file}}
auth.credentials_file.path: {{service_account_file}}
{{/if}}
{{#if buckets}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another field parse_json could be added as per the doc

@botelastic
Copy link

botelastic bot commented Dec 31, 2022

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic bot added the Stalled label Dec 31, 2022
@narph
Copy link
Contributor

narph commented Jan 9, 2023

@P1llus , can we get this in?

@botelastic botelastic bot removed the Stalled label Jan 9, 2023
@ShourieG
Copy link
Contributor

ShourieG commented Jan 9, 2023

@narph @P1llus I feel if we could wait till this PR: elastic/beats#34155 is merged, it would be awesome. This contains a lot of significant improvements to the GCS input which makes it more stable and behave better at scale. Having the integration live after the changes have gone through would be ideal.

@P1llus
Copy link
Member Author

P1llus commented Jan 16, 2023

This was waiting for elastic/stream#46 and elastic/elastic-package#1073
The latter was only merged a few days ago, so I will go ahead and finish the first one.

The reason behind it was so that we could implement the dynamic ECS template already from the initial version. It also seems that we should bump the minimum version as per @ShourieG comment, so we can ensure we have all the features needed for the input to work as intended.

@P1llus
Copy link
Member Author

P1llus commented Jan 19, 2023

All blocking features have now been resolved, will just have to update the PR with the system test and add dynamic ECS template before merging: #5055

@ShourieG
Copy link
Contributor

@narph @P1llus PR: elastic/beats#34155 is now merged, hence unblocking this PR.

@P1llus
Copy link
Member Author

P1llus commented Jan 23, 2023

This new input needs to be added to the filebeat spec file for elastic-agent first: https://github.com/elastic/elastic-agent/blob/main/specs/filebeat.spec.yml

@P1llus
Copy link
Member Author

P1llus commented Jan 23, 2023

Spec file updated as per: elastic/elastic-agent#2149
Awaiting new snapshot image to be generated.

@P1llus
Copy link
Member Author

P1llus commented Jan 25, 2023

There is currently no possible way to override the URL endpoint for the GCS client, we would need to add a alternative_host variable, similar to gcp-pubsub, so that we have the possibility to route the client to our local system test.

Copy link
Contributor

@ShourieG ShourieG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@P1llus P1llus merged commit 8607186 into elastic:main Feb 7, 2023
@elasticmachine
Copy link

Package google_cloud_storage - 0.1.0 containing this change is available at https://epr.elastic.co/search?package=google_cloud_storage

@andrewkroh andrewkroh added the New Integration Issue or pull request for creating a new integration package. label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Integration:google_cloud_storage Custom GCS (Google Cloud Storage) Input New Integration Issue or pull request for creating a new integration package.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom GCS input package
6 participants