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.

Sign up for GitHub

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

Jump to bottom

[GCP] Add firestore data_stream #2704

Merged
merged 28 commits into from
Mar 30, 2022
Merged

[GCP] Add firestore data_stream #2704

merged 28 commits into from
Mar 30, 2022

Conversation

gpop63
Copy link
Contributor

@gpop63 gpop63 commented Feb 16, 2022

What does this PR do?

Adds firestore datastream.

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

Screenshots

@elasticmachine
Copy link

elasticmachine commented Feb 16, 2022 •
edited
Loading

💚 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: 2022-03-30T13:58:01.738+0000

  • Duration: 20 min 49 sec

Test stats 🧪

Test Results
Failed 0
Passed 50
Skipped 0
Total 50

🤖 GitHub comments

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

  • /test : Re-trigger the build.

@gpop63 gpop63 marked this pull request as ready for review March 16, 2022 17:35
endorama
endorama requested changes Mar 17, 2022
View reviewed changes
Copy link
Member

@endorama endorama left a comment

Choose a reason for hiding this comment

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

Please ensure elastic-package check does not error. I added some minor changes but overall looks good to me and we can merge this as soon as CI is green

packages/gcp/data_stream/firestore/manifest.yml Outdated Show resolved Hide resolved
packages/gcp/data_stream/firestore/_dev/deploy/tf/main.tf Outdated Show resolved Hide resolved
packages/gcp/data_stream/firestore/_dev/deploy/tf/main.tf Outdated Show resolved Hide resolved
packages/gcp/data_stream/firestore/_dev/deploy/tf/main.tf Outdated Show resolved Hide resolved
packages/gcp/data_stream/firestore/_dev/test/system/test-default-config.yml Outdated Show resolved Hide resolved
@endorama endorama mentioned this pull request Mar 17, 2022
Closed
11 tasks
@gpop63
Copy link
Contributor Author

gpop63 commented Mar 17, 2022

/test

@gpop63 gpop63 requested review from a team as code owners March 17, 2022 17:01
mtojek
mtojek reviewed Mar 18, 2022
View reviewed changes
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

If you need any help from me, let me know, but please resolve first all conflicts :)

@gpop63 gpop63 force-pushed the gcp-firestore-datastream branch from 0dad745 to cdbcea0 Compare March 21, 2022 22:26
@gpop63 gpop63 removed request for a team March 21, 2022 22:27
endorama
endorama reviewed Mar 22, 2022
View reviewed changes
packages/gcp/data_stream/firestore/_dev/deploy/tf/main.tf Outdated
default = "detached"
}

variable "project_id" {
Copy link
Member

Choose a reason for hiding this comment

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

This has been changed and should be:

Suggested change
variable "project_id" {
variable "gcp_project_id" {

Copy link
Member

Choose a reason for hiding this comment

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

I noticed this change was not included in 081c2ce, is that done on purpose?

@endorama endorama force-pushed the gcp-metrics branch 2 times, most recently from 8cf80ec to fe54af3 Compare March 22, 2022 15:14
@gpop63 gpop63 force-pushed the gcp-firestore-datastream branch from 081c2ce to 21e114f Compare March 23, 2022 13:00
@gpop63
Copy link
Contributor Author

gpop63 commented Mar 23, 2022

/test

2 similar comments
@gpop63
Copy link
Contributor Author

gpop63 commented Mar 23, 2022

/test

@gpop63
Copy link
Contributor Author

gpop63 commented Mar 23, 2022

/test

@gpop63 gpop63 merged commit aaab6bd into elastic:gcp-metrics Mar 30, 2022
@endorama endorama mentioned this pull request May 11, 2022
Merged
4 tasks
endorama pushed a commit that referenced this pull request Jun 7, 2022
@gpop63 @endorama
[GCP] Add firestore data_stream (#2704)
471f0fc
endorama pushed a commit that referenced this pull request Jun 9, 2022
@gpop63 @endorama
[GCP] Add firestore data_stream (#2704)
d547fca
endorama pushed a commit that referenced this pull request Jul 12, 2022
@gpop63 @endorama
[GCP] Add firestore data_stream (#2704)
c8f00a8
@andrewkroh andrewkroh mentioned this pull request Jul 18, 2022
Closed
@andrewkroh andrewkroh mentioned this pull request Jul 26, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtojek mtojek mtojek left review comments

@endorama endorama endorama requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gpop63 @elasticmachine @endorama @mtojek @legoguy1000