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] Add dimensions metadata #6827

Merged
merged 7 commits into from
Jul 18, 2023

Conversation

tetianakravchenko
Copy link
Contributor

@tetianakravchenko tetianakravchenko commented Jul 5, 2023

What does this PR do?

  • Add dimension fields to the cloudwatch data_stream
  • Add ingest pipeline to generate aws.dimensions.fingerprint field
  • remove aws.s3.bucket.name field

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

Screenshots

Mapping:
Screenshot 2023-07-05 at 18 07 01

Testing with TSDB-migration-test-kit

python3.10 main.py
You're testing with version 8.8.0.

Testing data stream metrics-aws.cloudwatch_metrics-default.
Index being used for the documents is .ds-metrics-aws.cloudwatch_metrics-default-2023.07.05-000001.
Index being used for the settings and mappings is .ds-metrics-aws.cloudwatch_metrics-default-2023.07.05-000001.

The time series fields for the TSDB index are: 
        - dimension (5 fields):
                - agent.id
                - aws.dimensions.fingerprint
                - aws.s3.bucket.name
                - cloud.account.id
                - cloud.region
        - routing_path (5 fields):
                - agent.id
                - aws.dimensions.fingerprint
                - aws.s3.bucket.name
                - cloud.account.id
                - cloud.region

Index tsdb-index-enabled successfully created.

Copying documents from .ds-metrics-aws.cloudwatch_metrics-default-2023.07.05-000001 to tsdb-index-enabled...
All 204 documents taken from index .ds-metrics-aws.cloudwatch_metrics-default-2023.07.05-000001 were successfully placed to index tsdb-index-enabled.

Signed-off-by: Tetiana Kravchenko <[email protected]>
@tetianakravchenko tetianakravchenko requested a review from a team as a code owner July 5, 2023 16:18
Signed-off-by: Tetiana Kravchenko <[email protected]>
@elasticmachine
Copy link

elasticmachine commented Jul 5, 2023

💚 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: 2023-07-18T08:04:45.271+0000

  • Duration: 54 min 39 sec

Test stats 🧪

Test Results
Failed 0
Passed 194
Skipped 4
Total 198

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Jul 5, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (15/15) 💚
Files 93.75% (15/16) 👎 -1.25
Classes 93.75% (15/16) 👎 -1.25
Methods 85.714% (240/280) 👎 -0.505
Lines 85.925% (7387/8597) 👎 -3.475
Conditionals 100.0% (0/0) 💚

@constanca-m
Copy link
Contributor

Quick question: why is aws.s3.bucketname a dimension? Isn't a dimension fingerprint enough?

description: |
Name of a S3 bucket.
- name: dimensions.*
type: object
description: |
Metric dimensions.
- name: dimensions.fingerprint
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: naming is simil to other data_strems - #6749

@tetianakravchenko
Copy link
Contributor Author

tetianakravchenko commented Jul 5, 2023

Quick question: why is aws.s3.bucketname a dimension? Isn't a dimension fingerprint enough?

@constanca-m with the default configuration for ec2:instance I couldn't actually get this field in document. It is as well not available in the sample_event

I assumed it is only used for the S3 metrics. As for the S3 bucket name is not always part of aws.dimentions.*, example:

    "aws": {
      "s3": {
        "bucket": {
          "name": "tkravchenko-test"
        }
      },
      "cloudwatch": {
        "namespace": "AWS/S3"
      },
      "s3_request": {
        "latency": {
          "total_request": {
            "ms": 2.5
          }
        },
        "requests": {
          "head": 2,
          "total": 2
        },
        "downloaded": {
          "bytes_per_period": 730,
          "bytes": 365
        },
        "errors": {
          "4xx": 1,
          "5xx": 0
        }
      },
      "dimensions": {
        "FilterId": "Screenshot"
      }

I assumed it was added explicitly for s3 use case, @elastic/obs-cloud-monitoring could you please clarify it? why this field is declared in cloudwatch data_stream explicitly? Is there any specific reason? Or it should be removed from the fields definition?

Copy link
Contributor

@constanca-m constanca-m left a comment

Choose a reason for hiding this comment

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

I assumed it was added explicitly for s3 use case, https://github.com/orgs/elastic/teams/obs-cloud-monitoring could you please clarify it? why this field is declared in cloudwatch data_stream explicitly? Is there any specific reason? Or it should be removed from the fields definition?

Doesn't sound like a blocker to TSDB then. Even if it is an extra field, that is something that can be changed along the way.

@tetianakravchenko tetianakravchenko requested a review from a team July 6, 2023 08:03
Signed-off-by: Tetiana Kravchenko <[email protected]>
Signed-off-by: Tetiana Kravchenko <[email protected]>
Signed-off-by: Tetiana Kravchenko <[email protected]>
@tetianakravchenko
Copy link
Contributor Author

/test

Signed-off-by: Tetiana Kravchenko <[email protected]>
@tetianakravchenko tetianakravchenko merged commit 3ce066e into elastic:main Jul 18, 2023
@elasticmachine
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants