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

[ritalwar]

• 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]

💚 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]

🌐 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]

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.

[zmoog]

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]

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]

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]

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

[kaiyan-sheng]

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]

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]

@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]

• 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.

[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]

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]

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]

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]

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]

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