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

[Metricbeat] gcp: respect Beats naming conventions #27974

Merged
merged 55 commits into from
Oct 18, 2021

Conversation

endorama
Copy link
Member

@endorama endorama commented Sep 16, 2021

What does this PR do?

Changes fiels exported by all GCP metricset to adhere to ECS and Beats naming conventions.

This PR flattens all fields definition, to help correlating the mapping between GCP Monitoring and ECS fields.

The remapping operation is performed as a map[string]string lookup on a manually built map mapping the GCP Monitoring metric name to the appropriate ECS field.

Why is it important?

We want our metricset to adhere to ECS and Beats naming convention, so the data exported is easily consumed in Elasticsearch.

As 7.16 is the last minor before 8.0 and must be compatible with 8.0 is the last chance to introduce these breaking changes.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Sorry, something went wrong.

@endorama endorama added Team: Ingest Team:Integrations Label for the Integrations team labels Sep 16, 2021
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Sep 16, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 16, 2021

💚 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: 2021-10-18T09:46:39.839+0000

  • Duration: 105 min 13 sec

  • Commit: e9cf813

Test stats 🧪

Test Results
Failed 0
Passed 9538
Skipped 2537
Total 12075

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

@endorama endorama added the backport-v7.16.0 Automated backport with mergify label Sep 20, 2021
@endorama endorama changed the title [Metricbeat] gcp: respect ECS and Beats naming conventions [Metricbeat] gcp: respect Beats naming conventions Sep 22, 2021
@endorama
Copy link
Member Author

@kaiyan-sheng I was expecting tests to fail due to the rename. As the are all green I'm confused.

@endorama endorama marked this pull request as ready for review September 23, 2021 10:17
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@kaiyan-sheng
Copy link
Contributor

@endorama I think it's because only billing metricset has the check TestMetricsetFieldsDocumented in integration test enabled: https://github.com/elastic/beats/blob/master/x-pack/metricbeat/module/gcp/billing/billing_integration_test.go#L31. This should also be added to all the other metricsets.

@endorama endorama requested review from a team as code owners October 5, 2021 08:16
@botelastic botelastic bot added the Team:Automation Label for the Observability productivity team label Oct 5, 2021
@mergify
Copy link
Contributor

mergify bot commented Oct 5, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b master upstream/master
git merge upstream/master
git push upstream master

@endorama endorama self-assigned this Oct 6, 2021
@endorama endorama closed this Oct 6, 2021
@endorama endorama reopened this Oct 6, 2021
@kaiyan-sheng
Copy link
Contributor

@endorama Are you planning to add TestMetricsetFieldsDocumented for GCP integration tests as well in this PR?

@v1v v1v removed the request for review from a team October 7, 2021 11:05
@endorama
Copy link
Member Author

endorama commented Oct 7, 2021

@kaiyan-sheng I was planning to at least try to do that, so I was able to test these changes. But it's taking a lot of time and I'm still facing some strange errors.

@endorama
Copy link
Member Author

@kaiyan-sheng dashboards updated

@endorama endorama merged commit 5375f0a into elastic:master Oct 18, 2021
mergify bot pushed a commit that referenced this pull request Oct 18, 2021
(cherry picked from commit 5375f0a)

# Conflicts:
#	metricbeat/docs/fields.asciidoc
endorama added a commit that referenced this pull request Oct 19, 2021
(cherry picked from commit 5375f0a)

# Conflicts:
#	metricbeat/docs/fields.asciidoc
endorama added a commit that referenced this pull request Oct 19, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
(cherry picked from commit 5375f0a)

# Conflicts:
#	metricbeat/docs/fields.asciidoc

Co-authored-by: endorama <[email protected]>
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
endorama added a commit to endorama/beats that referenced this pull request Sep 7, 2022
After we aligned metrics with Beats naming convention (elastic#27974)
the docs has not been adequately updated.
endorama added a commit that referenced this pull request Sep 14, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-authored-by: DeDe Morton <[email protected]>
mergify bot pushed a commit that referenced this pull request Sep 14, 2022
Co-authored-by: DeDe Morton <[email protected]>
(cherry picked from commit 923e11f)

# Conflicts:
#	x-pack/metricbeat/module/gcp/dataproc/_meta/docs.asciidoc
#	x-pack/metricbeat/module/gcp/firestore/_meta/docs.asciidoc
mergify bot pushed a commit that referenced this pull request Sep 14, 2022
endorama added a commit that referenced this pull request Sep 15, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…es after #27974 (#33080)

Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: Edoardo Tenani <[email protected]>
endorama added a commit that referenced this pull request Sep 19, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ies after #27974 (#33079)

Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: Edoardo Tenani <[email protected]>
Co-authored-by: Edoardo Tenani <[email protected]>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.16.0 Automated backport with mergify breaking change Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Metricbeat] gcp: fields do not respect naming best practices
3 participants