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

Aws cloudwatch msk #13389

Merged
merged 5 commits into from Aug 30, 2019
Merged

Aws cloudwatch msk #13389

merged 5 commits into from Aug 30, 2019

Conversation

thenom
Copy link
Contributor

@thenom thenom commented Aug 29, 2019

Added label seperator and split the labels based on that instead of spaces. Some dimensions include spaces so was breaking the generation of the event, for example:

This label:
KafkaAppLogsDiskUsed AWS/Kafka Cluster Name,Broker ID test-kafka,1

was generating:

    "aws": {
      "cloudwatch": {
        "namespace": "AWS/Kafka",
        "dimensions": {
          "Cluster": "Name"
        },
        "metrics": {
          "ZooKeeperRequestLatencyMsMean": 1.1137819147251762
        }
      }
    },

instead of:

    "aws": {
      "cloudwatch": {
        "namespace": "AWS/Kafka",
        "dimensions": {
          "Cluster Name": "test-kafka",
          "Broker ID": "1"
        },
        "metrics": {
          "CpuIdle": 98.540024,
          "MemoryCached": 1297193200,
          ...

And a side affect of this is it was creating an event for every metric.

@thenom thenom requested a review from a team as a code owner August 29, 2019 08:43
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@kaiyan-sheng
Copy link
Contributor

jenkins, test this please

@kaiyan-sheng
Copy link
Contributor

@thenom Thank you for creating this PR and making this great change in cloudwatch metricset! I see that you are making the change based on 7.3 branch. Could you create a PR based on master please? Then we can backport it to 7.3 and 7.4 after it gets merged.

@kaiyan-sheng kaiyan-sheng added Team:Integrations Label for the Integrations team Metricbeat Metricbeat labels Aug 29, 2019
@thenom thenom requested review from a team as code owners August 29, 2019 15:53
@thenom thenom changed the base branch from 7.3 to master August 29, 2019 15:54
@kaiyan-sheng kaiyan-sheng added needs_backport PR is waiting to be backported to other branches. review labels Aug 29, 2019
@thenom
Copy link
Contributor Author

thenom commented Aug 29, 2019

Hi @kaiyan-sheng , thanks for taking a look. I have rebased this on master and local tests are passing.

@kaiyan-sheng
Copy link
Contributor

jenkins, test this please

@kaiyan-sheng
Copy link
Contributor

Thank you! Could you please add a changelog entry in CHANGELOG.next.asciidoc under Bugfixes Metricbeat for this please?

@thenom
Copy link
Contributor Author

thenom commented Aug 30, 2019

Yep no problem, entry added:

  • Fix issue with aws cloudwatch module where dimensions and/or namespaces that contain space are not being parsed correctly {pull}13389[13389]

@kaiyan-sheng
Copy link
Contributor

jenkins, test this please

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

Tested locally and it works. Thanks again for your contribution @thenom!

@kaiyan-sheng kaiyan-sheng merged commit c8f0879 into elastic:master Aug 30, 2019
@thenom thenom deleted the aws-cloudwatch-msk branch August 30, 2019 17:45
@thenom
Copy link
Contributor Author

thenom commented Aug 30, 2019

Brilliant, @kaiyan-sheng thanks again 👍

@urso urso added the v7.5.0 label Oct 22, 2019
@kaiyan-sheng kaiyan-sheng removed the needs_backport PR is waiting to be backported to other branches. label Jan 14, 2020
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Metricbeat Metricbeat review Team:Integrations Label for the Integrations team v7.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants