[Azure][Storage_account] Add dimension to the storage account datastream #7356

ritalwar · 2023-08-11T05:29:21Z

Enhancement

What does this PR do?

This PR add dimension for azure storage_account datastream.
Specified dimension fields are as follows:
- agent.id
- azure.dimensions.api_name
- azure.dimensions.response_type
- azure.namespace
- azure.resource.id
- azure.timegrain
- cloud.region
Link for azure defined dimensions: https://learn.microsoft.com/en-us/azure/azure-monitor/reference/supported-metrics/microsoft-classicstorage-storageaccounts-metrics

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.

How to test this PR locally

Related issues

Relates Azure TSDB Enablement: storage_account datastream #7215

Screenshots

elasticmachine · 2023-08-11T07:46:50Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-10-03T04:52:11.114+0000
Duration: 15 min 49 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 · 2023-08-11T07:47:00Z

🌐 Coverage report

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

ritalwar · 2023-08-11T08:14:59Z

Azure has additional dimensions for storage accounts, such as GeoType, Authentication, FileShare, BlobType , and tier. However, these dimensions are not currently included in this PR. As in the current Beats implementation, only two dimensions, namely ResponseType and ApiName, are allowed. I've used these two dimensions for enabling TSDB for now.

Although the current set of dimensions doesn't discard any documents, I'm in the process of using Azure Rest APIs to explore the values for the other dimensions defined by Azure.

…integrations into azure_tsdb_storageaccount_7215

zmoog · 2023-08-24T07:00:23Z

However, these dimensions are not currently included in this PR. As in the current Beats implementation, only two dimensions, namely ResponseType and ApiName, are allowed.

Oh, I see the dimensions list is pretty limited here.

Expanding the list is not in the scope of this PR, so we should not block this PR. We should open an issue to plan this activity.

We should also drop the storage metricset and switch the agent to use the generic monitor metricset. WDYT @kaiyan-sheng ?

ritalwar · 2023-08-24T07:36:36Z

Expanding the list is not in the scope of this PR, so we should not block this PR. We should open an issue to plan this activity.

I merged this PR with the remaining dimensions included. However, I've come to realize that the TransactionType dimension is still pending and may require a separate PR to incorporate it.

We should also drop the storage metricset and switch the agent to use the generic monitor metricset.

@zmoog However, if this is under consideration, should we wait before migrating this datastream to the TSDB?
Alternatively, can I proceed with creating another PR in beats to include the TransactionType dimension?
We intend to merge this PR after the beats changes are available for use in the integration.

zmoog · 2023-08-24T08:30:54Z

I merged this elastic/beats#36331 with the remaining dimensions included. However, I've come to realize that the TransactionType dimension is still pending and may require a separate PR to incorporate it.

Oh, I missed this one! Thank you for pointing it out.

However, if this is under consideration, should we wait before migrating this datastream to the TSDB?

No, switching from the storage metricset to monitor metricset is (IMO) not urgent, and we can do it after the transition to TSDB is complete.

Alternatively, can I proceed with creating another PR in beats to include the TransactionType dimension?
We intend to merge this PR after the beats changes are available for use in the integration.

We can tackle the switch storage > monitor after completing all the in-flight activities.

@kaiyan-sheng, please chime in with your perspective 🙇

kaiyan-sheng · 2023-08-25T15:16:50Z

packages/azure_metrics/changelog.yml

@@ -1,3 +1,8 @@
+- version: "1.0.24"


I think we can bump this one to 1.1.0 since this is an enhancement not a bug fix.

kaiyan-sheng · 2023-08-25T15:22:58Z

I agree with @zmoog, we are not in a hurry to change storage to use the generic monitor. I was looking at the code in Beats and we are also not using the generic monitor there: https://github.com/elastic/beats/tree/main/x-pack/metricbeat/module/azure/storage

Let's make sure we are not collecting extra data in storage that can't be done by monitor first 🙂

botelastic · 2023-09-24T16:14:19Z

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

zmoog · 2023-09-26T17:59:22Z

@ritalwar I was checking the pending issues/PRs, and I want to recap this one to double-check I'm not overlooking something:

We were waiting for Add more dimensions to allowedDimensions for azure storage account to… beats#36331 to be available in a Beats release.
Do we need to bump the required Kibana version to 8.10?
Any other dependency?

ritalwar · 2023-09-27T07:48:04Z

We were waiting for Add more dimensions to allowedDimensions for azure storage account to… beats#36331 to be available in a Beats release.

Yes, now that this is available in release, I've been testing it and now waiting for the TSDB team to provide their feedback.

Do we need to bump the required Kibana version to 8.10?

I've observed that for all other similar changes, such as those related to dimensions and metric_type PRs, the version update was done at the patch level rather than the minor version. Should I follow the same approach and update the patch version for consistency?

Edit: My apologies! I overlooked the Kibana version and was referring to the package version instead.

Any other dependency?

I checked if this datastream is affected by the grouping issue seen in other Azure and GCP datastreams. However, it seems unaffected because the implementation for this datastream is separate in the Beats module, and it already groups data based on allowed dimensions. If you think there's anything I missed, please let me know, and I'll test further.

packages/azure_metrics/data_stream/storage_account/fields/agent.yml

packages/azure_metrics/data_stream/storage_account/elasticsearch/ingest_pipeline/default.yml

…nt processor.

tetianakravchenko

kibana version should be changed to 8.10 I think, since this change is relying on the 8.10 beats release

kibana.version: "^8.9.0"

I also agree that the package version should be changed to 1.1.0 when the new kibana version restriction will be added.

ritalwar · 2023-09-27T11:56:46Z

kibana version should be changed to 8.10 I think, since this change is relying on the 8.10 beats release
kibana.version: "^8.9.0"

Should we consider updating the Kibana version in the TSDB enablement PR, as we've done for some other packages?

I also agree that the package version should be changed to 1.1.0 when the new kibana version restriction will be added.

I'll follow this in the PR where I update the Kibana version, and we can decide whether it's in this one or the enablement PR.

tetianakravchenko · 2023-09-27T12:06:59Z

Should we consider updating the Kibana version in the TSDB enablement PR, as we've done for some other packages?

adding additional fields is not related to the tsdb enablement. Here I've seen similar discussion regarding the handling of kibana constrains - #6821 (comment) (this is a conclusion, but see the whole discussion) for adding new fields, based on that my understanding was that it is a proper way of handling it.

Do we need to bump the required Kibana version to 8.10?

@zmoog I've seen your comment regarding the kibana version #7356 (comment). Is your understanding the same as in discussion linked above?

tetianakravchenko · 2023-09-27T12:25:04Z

Should we consider updating the Kibana version in the TSDB enablement PR, as we've done for some other packages?

for other packages usually we were introducing the Kibana version constrain together with tsdb enablement, because tsdb is supported starting from ^8.8.0

in this case if TSDB enablement PR would include kibana constrain ^8.10.0, it will not be correct since TSDB enablement itself does not require bumping kibana version to 8.10

ritalwar · 2023-09-27T13:56:02Z

in this case if TSDB enablement PR would include kibana constrain ^8.10.0, it will not be correct since TSDB enablement itself does not require bumping kibana version to 8.10

That makes sense. I've made the update and updated both Kibana and package version.

elasticmachine · 2023-10-03T05:36:10Z

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

[Azure][Storage_account] Add dimension to the storage account datastream

8860463

ritalwar requested a review from a team as a code owner August 11, 2023 05:29

Update changelog.yml

80f72d4

ritalwar requested review from tetianakravchenko and agithomas August 11, 2023 05:32

ritalwar marked this pull request as draft August 11, 2023 07:00

Merge branch 'main' into azure_tsdb_storageaccount_7215

3c3b86e

ritalwar self-assigned this Aug 11, 2023

ritalwar mentioned this pull request Aug 11, 2023

Azure TSDB Enablement: storage_account datastream #7215

Closed

6 tasks

ritalwar and others added 3 commits August 17, 2023 11:30

Update dimensions.

6822d93

Merge branch 'azure_tsdb_storageaccount_7215' of github.com:ritalwar/…

58b6cee

…integrations into azure_tsdb_storageaccount_7215

Merge branch 'main' into azure_tsdb_storageaccount_7215

7418543

ritalwar marked this pull request as ready for review August 22, 2023 08:18

zmoog approved these changes Aug 24, 2023

View reviewed changes

kaiyan-sheng reviewed Aug 25, 2023

View reviewed changes

botelastic bot added the Stalled label Sep 24, 2023

botelastic bot removed the Stalled label Sep 26, 2023

Merge branch 'main' into azure_tsdb_storageaccount_7215

4835539

ritalwar requested a review from gpop63 September 27, 2023 07:51

tetianakravchenko requested changes Sep 27, 2023

View reviewed changes

packages/azure_metrics/data_stream/storage_account/fields/agent.yml Outdated Show resolved Hide resolved

packages/azure_metrics/data_stream/storage_account/elasticsearch/ingest_pipeline/default.yml Outdated Show resolved Hide resolved

ritalwar added 2 commits September 27, 2023 13:48

Address review comments

4a55325

Extend dimensions list to specify them individually, remove fingerpri…

a225cd2

…nt processor.

tetianakravchenko reviewed Sep 27, 2023

View reviewed changes

Update kibana and package version

e8a5d96

tetianakravchenko approved these changes Sep 27, 2023

View reviewed changes

Update kibana version to 8.10.2

bceab51

ritalwar merged commit 39190a9 into elastic:main Oct 3, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Azure][Storage_account] Add dimension to the storage account datastream #7356

[Azure][Storage_account] Add dimension to the storage account datastream #7356

ritalwar commented Aug 11, 2023 •

edited

Loading

elasticmachine commented Aug 11, 2023 •

edited

Loading

Build stats

Test stats 🧪

elasticmachine commented Aug 11, 2023 •

edited

Loading

ritalwar commented Aug 11, 2023 •

edited

Loading

zmoog commented Aug 24, 2023

ritalwar commented Aug 24, 2023 •

edited

Loading

zmoog commented Aug 24, 2023

kaiyan-sheng Aug 25, 2023

kaiyan-sheng commented Aug 25, 2023

botelastic bot commented Sep 24, 2023

zmoog commented Sep 26, 2023 •

edited

Loading

ritalwar commented Sep 27, 2023 •

edited

Loading

tetianakravchenko left a comment •

edited

Loading

ritalwar commented Sep 27, 2023

tetianakravchenko commented Sep 27, 2023

tetianakravchenko commented Sep 27, 2023

ritalwar commented Sep 27, 2023

elasticmachine commented Oct 3, 2023

[Azure][Storage_account] Add dimension to the storage account datastream #7356

[Azure][Storage_account] Add dimension to the storage account datastream #7356

Conversation

ritalwar commented Aug 11, 2023 • edited Loading

What does this PR do?

Checklist

How to test this PR locally

Related issues

Screenshots

elasticmachine commented Aug 11, 2023 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

elasticmachine commented Aug 11, 2023 • edited Loading

🌐 Coverage report

ritalwar commented Aug 11, 2023 • edited Loading

zmoog commented Aug 24, 2023

ritalwar commented Aug 24, 2023 • edited Loading

zmoog commented Aug 24, 2023

kaiyan-sheng Aug 25, 2023

Choose a reason for hiding this comment

kaiyan-sheng commented Aug 25, 2023

botelastic bot commented Sep 24, 2023

zmoog commented Sep 26, 2023 • edited Loading

ritalwar commented Sep 27, 2023 • edited Loading

tetianakravchenko left a comment • edited Loading

Choose a reason for hiding this comment

ritalwar commented Sep 27, 2023

tetianakravchenko commented Sep 27, 2023

tetianakravchenko commented Sep 27, 2023

ritalwar commented Sep 27, 2023

elasticmachine commented Oct 3, 2023

ritalwar commented Aug 11, 2023 •

edited

Loading

elasticmachine commented Aug 11, 2023 •

edited

Loading

elasticmachine commented Aug 11, 2023 •

edited

Loading

ritalwar commented Aug 11, 2023 •

edited

Loading

ritalwar commented Aug 24, 2023 •

edited

Loading

zmoog commented Sep 26, 2023 •

edited

Loading

ritalwar commented Sep 27, 2023 •

edited

Loading

tetianakravchenko left a comment •

edited

Loading