Skip to content

[GCP] Migrate GCP package to ecs@mappings #10225

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
Merged
merged 10 commits into from
Jul 5, 2024
Merged

[GCP] Migrate GCP package to ecs@mappings #10225

merged 10 commits into from
Jul 5, 2024

Conversation

@harnish-crest-data
Copy link
Contributor

@harnish-crest-data harnish-crest-data commented Jun 24, 2024

  • Enhancement

Proposed commit message

Migration performed using ecs-update. Minor manual changes are performed.

  • event.category or event.type is expecting value as an array instead of string. Hence provided the value as an array.
  • generated pipeline and system tests.

Command

go run github.com/andrewkroh/go-examples/ecs-update@014b35dfe4c9832b51e7c909a39a48257d6a005d \
  -ecs-version=8.11.0 \
  -ecs-git-ref=v8.11.0 \
  -fields-yml-drop-ecs \
  -kibana-version=^8.13.0 \
  -drop-import-mappings \
  -owner=elastic/obs-ds-hosted-services \
  packages/gcp

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.

@harnish-crest-data
[gcp] - change to ECS version git@v8.11.0
5359e6c
@harnish-crest-data
update event.type and event.category fields values from string to arr…
8c79fb0 
…ay and generate pipeline and system tests for dns and laodbalancing_logs data streams
@harnish-crest-data harnish-crest-data self-assigned this Jun 24, 2024
@elasticmachine
Copy link

elasticmachine commented Jun 24, 2024

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@harnish-crest-data harnish-crest-data changed the title [GCP] Migrate ECS version to git@v8.11.0 [GCP] Migrate GCP package to ecs@mappings Jun 25, 2024
@harnish-crest-data harnish-crest-data marked this pull request as ready for review June 25, 2024 07:29
@harnish-crest-data harnish-crest-data requested review from a team as code owners June 25, 2024 07:29
niraj-elastic
niraj-elastic suggested changes Jul 2, 2024
View reviewed changes
@harnish-crest-data
address review comments
2499cd8
@harnish-crest-data
Merge branch 'main' of github.com:harnish-elastic/integrations into c…
c05b009 
…loud_package_ecs_mapping_migration

Conflicts:
	packages/gcp/changelog.yml
	packages/gcp/manifest.yml
ishleenk17
ishleenk17 reviewed Jul 5, 2024
View reviewed changes
packages/gcp/data_stream/firestore/fields/ecs.yml Outdated
- external: ecs
name: ecs.version
- external: ecs
name: error
Copy link
Member

@ishleenk17 ishleenk17 Jul 5, 2024

Choose a reason for hiding this comment

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

Is this valid ?

Copy link
Contributor Author

@harnish-crest-data harnish-crest-data Jul 5, 2024

Choose a reason for hiding this comment

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

Nope, let me remove these fields as well from all the PRs!

@ishleenk17
Copy link
Member

ishleenk17 commented Jul 5, 2024

@mrodm : As discussed over slack, I see these failures after just the last commit. What has changed here ?
These coverage checks are useful, but it would hinder these bulk merges? Please suggest

@mrodm
Copy link
Collaborator

mrodm commented Jul 5, 2024

@mrodm : As discussed over slack, I see these failures after just the last commit. What has changed here ? These coverage checks are useful, but it would hinder these bulk merges? Please suggest

@ishleenk17 there were some changes related to how elastic-package creates the coverage reports in this PR: elastic/elastic-package#1915

Now there are more files marked as covered/uncovered, and it is likely that the coverage is decreased (see elastic/elastic-package#1915 (comment)).
There is still pending to add more files as part of the coverage in this other PR #10385 (to be checked if that would decreased the coverage (%) too).

I think this PR could be merged even if that coverage analysis from sonar step is failing.

cc @jsoriano

ishleenk17
ishleenk17 approved these changes Jul 5, 2024
View reviewed changes
Copy link
Member

@ishleenk17 ishleenk17 left a comment

Choose a reason for hiding this comment

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

Looks good!

kcreddy
kcreddy approved these changes Jul 5, 2024
View reviewed changes
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.

Minor suggestion.
LGTM 👍🏼

niraj-elastic
niraj-elastic approved these changes Jul 5, 2024
View reviewed changes
Copy link
Contributor

@niraj-elastic niraj-elastic left a comment

Choose a reason for hiding this comment

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

LGTM

@harnish-crest-data harnish-crest-data mentioned this pull request Jul 5, 2024
Merged
4 tasks
@elasticmachine
Copy link

elasticmachine commented Jul 5, 2024

💚 Build Succeeded

History

cc @harnish-elastic

@elastic-sonarqube
Copy link

elastic-sonarqube bot commented Jul 5, 2024

Quality Gate failed Quality Gate failed

Failed conditions
56.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

@ishleenk17
Copy link
Member

ishleenk17 commented Jul 5, 2024

As per the comment here, we are ignoring the sonarqube error and merging the code

@ishleenk17 ishleenk17 merged commit 73d57d7 into elastic:main Jul 5, 2024
@elasticmachine
Copy link

elasticmachine commented Jul 5, 2024

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

This was referenced Jul 16, 2024
Merged
Merged
@andrewkroh andrewkroh added the Integration:gcp Google Cloud Platform label Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zmoog zmoog zmoog approved these changes

@kcreddy kcreddy kcreddy approved these changes

@ishleenk17 ishleenk17 ishleenk17 approved these changes

@ShourieG ShourieG ShourieG approved these changes

@milan-elastic milan-elastic Awaiting requested review from milan-elastic

+1 more reviewer

@niraj-elastic niraj-elastic niraj-elastic approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@harnish-crest-data harnish-crest-data

Labels

Integration:gcp Google Cloud Platform

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@harnish-crest-data @elasticmachine @ishleenk17 @mrodm @zmoog @kcreddy @ShourieG @niraj-elastic @andrewkroh