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

[Metricbeat] Rename tags to tags_filter for cloudwatch metricset #16733

Merged
merged 9 commits into from
May 20, 2020
Merged

[Metricbeat] Rename tags to tags_filter for cloudwatch metricset #16733

merged 9 commits into from
May 20, 2020

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Mar 2, 2020 •
edited
Loading

This PR is to deprecate tags in cloudwatch metricset config. This is replaced by tags_filter, which is a metricset level config parameter for all metricsets under aws module.

@kaiyan-sheng kaiyan-sheng requested a review from a team as a code owner March 2, 2020 19:03
@kaiyan-sheng kaiyan-sheng self-assigned this Mar 2, 2020
@kaiyan-sheng kaiyan-sheng added discuss Issue needs further discussion. Metricbeat Metricbeat needs_backport PR is waiting to be backported to other branches. review in progress Pull request is currently in progress. and removed review labels Mar 2, 2020
exekias
exekias suggested changes Mar 3, 2020
View reviewed changes
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

this will need a changelog

exekias
exekias reviewed Mar 3, 2020
View reviewed changes
x-pack/metricbeat/module/aws/cloudwatch/cloudwatch.go Outdated
Comment on lines 69 to 71
Tags []aws.Tag `config:"tags"`
TagsFilter []aws.Tag `config:"tags_filter"`
Tags []aws.Tag `config:"tags"` // Deprecated.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to doing this change! great work at keeping backwards compatibility

@kaiyan-sheng
Copy link
Contributor Author

kaiyan-sheng commented Mar 3, 2020 •
edited
Loading

Will come back to work on this after #16605 is merged. In #16605 I moved tags_filter to a MetricSet level config for aws module, which means all metricset under aws module should be able to use tags_filter. This means I have to remove tags under cloudwatch and instead to use tags_filter. All the light modules need to be tested as well with the new tags_filter.

@andresrc andresrc added [zube]: Inbox [zube]: In Progress Team:Platforms Label for the Integrations - Platforms team and removed [zube]: Inbox labels Mar 9, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-platforms (Team:Platforms)

sayden
sayden reviewed Mar 9, 2020
View reviewed changes
Copy link
Contributor

@sayden sayden 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 until now :)

@elasticmachine
Copy link
Collaborator

elasticmachine commented May 13, 2020 •
edited
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

Test stats 🧪

Test Results
Failed 0
Passed 1310
Skipped 214
Total 1524

Steps errors

Expand to view the steps failures

  • Name: Report to Codecov

    • Description: curl -sSLo codecov https://codecov.io/bash for i in auditbeat filebeat heartbeat libbeat metricbeat packetbeat winlogbeat journalbeat do FILE="${i}/build/coverage/full.cov" if [ -f "${FILE}" ]; then bash codecov -f "${FILE}" fi done

    • Result: FAILURE

    • Duration: 2 min 22 sec

    • Start Time: 2020-05-19T17:35:56.884+0000

    • log

@kaiyan-sheng kaiyan-sheng mentioned this pull request May 18, 2020
Merged
6 tasks
@kaiyan-sheng kaiyan-sheng removed discuss Issue needs further discussion. in progress Pull request is currently in progress. labels May 18, 2020
@kaiyan-sheng kaiyan-sheng requested a review from exekias May 19, 2020 20:49
@kaiyan-sheng kaiyan-sheng merged commit 7e5e98c into elastic:master May 20, 2020
@kaiyan-sheng kaiyan-sheng deleted the rename_tags branch May 20, 2020 12:56
@kaiyan-sheng kaiyan-sheng mentioned this pull request May 20, 2020
Merged
@kaiyan-sheng kaiyan-sheng added v7.9.0 and removed needs_backport PR is waiting to be backported to other branches. labels May 20, 2020
kaiyan-sheng added a commit that referenced this pull request May 20, 2020
@kaiyan-sheng
[Metricbeat] Rename tags to tags_filter for cloudwatch metricset (#16733
311ab02 
) (#18660)

* Deprecate tags config parameter to use tags_filter instead for cloudwatch metricset

(cherry picked from commit 7e5e98c)
This was referenced Jul 8, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sayden sayden sayden left review comments

@ChrsMark ChrsMark ChrsMark approved these changes

@exekias exekias exekias approved these changes

Assignees

@kaiyan-sheng kaiyan-sheng

Labels
Metricbeat Metricbeat review Team:Platforms Label for the Integrations - Platforms team v7.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kaiyan-sheng @elasticmachine @exekias @sayden @ChrsMark @andresrc