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

[Azure Storage Account] Add metric_type metadata to the storage_account datastream #7488

Merged

Conversation

ritalwar
Copy link
Contributor

@ritalwar ritalwar commented Aug 22, 2023

  • Enhancement

What does this PR do?

This PR use predefined metrics and adds metric_type metadata to the storage_account datastream for all of these services, descriptions are also taken from here:
StorageAccount
BlobServices
FileServices
QueueServices
TableServices

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.

Related issues

Screenshots

@ritalwar ritalwar requested a review from a team as a code owner August 22, 2023 08:16
@elasticmachine
Copy link

elasticmachine commented Aug 22, 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-10-03T05:14:36.339+0000

  • Duration: 15 min 44 sec

Test stats 🧪

Test Results
Failed 0
Passed 34
Skipped 0
Total 34

🤖 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 Aug 22, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (4/4) 💚
Files 100.0% (4/4) 💚 2.621
Classes 100.0% (4/4) 💚 2.621
Methods 75.758% (25/33) 👎 -16.698
Lines 100.0% (32/32) 💚 11.573
Conditionals 100.0% (0/0) 💚


- name: azure.storage_account
type: group
fields:
Copy link
Contributor

@zmoog zmoog Aug 22, 2023

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, please forget the comment about IndexCapacity, I could not find it but it's there. My Bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this is surprising: how can FileShareQuota field from the docs become file_share_capacity_quota field in ES?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is being taken from here or sdk.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks.

I see we have two documents for Azure Files:

Since the first one mentions the "Microsoft.ClassicStorage/storageAccounts" namespace, this may mean this for the classic version of the Files service, the the second for the current / modern version.

Are we supporting metrics for the classic namespace?

We should also check if we need add some *IOPS* related field in the current namespace doc, like:

  • FileShareMaxUsedIOPS
  • FileShareProvisionedIOPS
  • FileShareMaxUsedBandwidthMiBps

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, my mistake! I referred to the classicStorage link instead of the correct one, which should be Microsoft.Storage/storageAccounts as this is the namespace we are supporting. To address this, I will update the metric_type mapping using the object type instead of the group format like this

- name: azure.storage_account.*.*
  type: object
  object_type: float
  metric_type: gauge
  object_type_mapping_type: "*"
  description: >
    storage account

This will ensure that all the fields within this namespace are correctly accounted for.

I didn't use this format before due to some issues associated with it. However, it seems those issues have been resolved, as I tested it successfully.

Screenshot 2023-08-22 at 9 34 39 PM Screenshot 2023-08-22 at 9 34 09 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it! Thanks for the check!

I'm not an expert in TSDB, but this makes sense.

I updated the link in the PR description with the non-classic one; please take a look to double-check if it's the correct one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @zmoog!

@ritalwar ritalwar added the enhancement New feature or request label Aug 28, 2023
@ritalwar
Copy link
Contributor Author

/test

@ritalwar ritalwar merged commit 7142b33 into elastic:main Oct 3, 2023
4 checks passed
@ritalwar ritalwar changed the title [Azure Storage Account] Add metric_type metadata to the datastream [Azure Storage Account] Add metric_type metadata to the storage_account datastream Oct 3, 2023
@elasticmachine
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants