Rename azure/storage metricset to storage_account for consistency

[narph]

What does this PR do?

Rename azure/storage metricset to storage_account for consistency

Why is it important?

Rename azure/storage metricset to storage_account for consistency

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files
• [x I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

[mergify]

This pull request does not have a backport label. Could you fix it @narph? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

[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

• Duration: 114 min 56 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

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

• /test : Re-trigger the build.

• /package : Generate the packages and run the E2E tests.

• /beats-tester : Run the installation tests with beats-tester.

[elasticmachine]

Pinging @elastic/integrations (Team:Integrations)

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b rename-azure-storage upstream/rename-azure-storage
git merge upstream/master
git push upstream rename-azure-storage

[narph]

Will block this PR till we decide what to do:

Potential solutions:
1.

- register both names and add a deprecate message asking users to move to the new naming for 7.16, add the new naming in the docs
- remove the deprecate message and the old name in the docs but keep both names of the metricset registered for 8.0

What do we do with the dashboards containing the azure.storage.* fields in this case ?

no rename

cc @jsoriano , @mtojek

[mtojek]

This isn't a perfect solution, but let's analyze it.

• register both names and add a deprecate message asking users to move to the new naming for 7.16, add the new naming in the docs

I'm for this option, even though we can't guarantee a smooth/automatic update (it's a change of metricset name).

Regarding fields and dashboards, I can speak only in the context of integration. Think about the situation when you have a fleet with 1000 agents and you need to update them. It means that for some time there will two kinds of agents:

1. Sending data to old namespace.
2. Sending data to new namespace.

Solution: we need a compatibility ingest pipeline in the integration, which will rewrite old fields to the new namespace. When will this pipeline be removed in the future, TBD (sorry about that, no clear decision).

• remove the deprecate message and the old name in the docs but keep both names of the metricset registered for 8.0

I think that if you have the compatibility pipeline in place, you create another revision of the package which will update the metricset reference in the integration and get rid of the old one.

Any opinion on this, @jsoriano @ruflin ?

[narph]

@mtojek , thanks, just a clarification here , on the package side the data stream has the right naming storage_account and we are running a pipeline to rename the fields already https://github.com/elastic/integrations/blob/master/packages/azure_metrics/data_stream/storage_account/elasticsearch/ingest_pipeline/default.yml.

I am more concerned about: if the effort it takes to make all these changes and the disruption on the user side (metricbeat users switching between the 2 names) outweighs the reason for the renaming. The reason is consistency to the other azure metricsets names and also to the azure resource type which is a storage account and not storage.

[jsoriano]

Rename azure/storage metricset to storage_account for consistency

I am more concerned about: if the effort it takes to make all these changes and the disruption on the user side (metricbeat users switching between the 2 names) outweighs the reason for the renaming. The reason is consistency to the other azure metricsets names and also to the azure resource type which is a storage account and not storage.

I would also consider the option of doing nothing. Specially if we are only doing this change for consistency. Do we expect problems if we don't rename the metricset? We can still title the dataset "Storage Account", even when under the hood this is called "storage".

If we still want to do the rename, I agree with the discussed strategies:

• Register the metricset with both names (what could be already done in Beats 7.x), in both cases it keeps the old field naming.
• In Beats 8.0 only, rename the fields.
• In the integration targeting 8.x: add the pipeline with the rename for old agents.

When will this pipeline be removed in the future, TBD (sorry about that, no clear decision).

I don't think we ever need to remove these compatibility layers, unless they are causing maintenance burden, or they conflict with future features.

[mtojek]

I don't think we ever need to remove these compatibility layers, unless they are causing maintenance burden, or they conflict with future features.

Yes, I was thinking exactly about this risk. So that we'll end up with dozen of old compatibility pipelines.

EDIT:

I would also consider the option of doing nothing. Specially if we are only doing this change for consistency. Do we expect problems if we don't rename the metricset? We can still title the dataset "Storage Account", even when under the hood this is called "storage".

Maybe you're right that in this case there is a low gain. If we don't have any guidelines for proper metricset naming, I would also support this option (less work to do).

[ruflin]

I'm leaning in the direction of not doing anything right now even though looking at the name I understand why we should rename. But this is a very valuable discussion we should continue. Even if we don't rename this right now, the next case will pop up where we either need to deprecate a dataset or even deprecate a package. We need a solution for it. I would start with dataset and packages and work backwards from there on what it means for Beats. I expect the to have a simpler solution for packages as each dataset has its own data stream and we likely could use aliases or similar but lets see.

[jsoriano]

So that we'll end up with dozen of old compatibility pipelines.

This would mean that a package would have needed dozens of breaking changes, I wouldn't like to see this situation 🙂 But if this happens, maintaining this dozen of compatibility pipelines will reduce the pain caused to users by these changes.

Even if we don't rename the metricset itself, we can still use the consistent naming in titles, descriptions and other texts in the integration, this is actually the only thing users are going to see in the new experience, and this doesn't require a breaking change.

[narph]

Thanks everyone for the feedback, I have been doing some tests and I am leaning towards the following:

• we have always made it clear in the docs that although the metricset is called storage it will collect storage account related metrics and we have not received any user feedback that the naming has been confusing so far.
• we were able to name the corresponding data stream from the start storage_account in the azure_metrics package

This is why I have been leaning towards not renaming. I did test registering the new name #28447 to the metricset and collected some metrics and my conclusion would be:

• register the 2nd name but do not make any additional changes like update docs/field definition/dashboards, users should not be aware of the change - this can come in 7.16.
• in the future replace the metricset name to storage_account in the package https://github.com/elastic/integrations/blob/master/packages/azure_metrics/data_stream/storage_account/agent/stream/stream.yml.hbs and remove the rename pipeline

Should we expect any unforeseen issues with the plan above, I feel like I am missing something?

[jsoriano]

@narph sounds good. Specially seeing that we already have the pipeline in the integration to handle compatibility between both field schemas, with the rename from azure.storage to azure.storage_account.

Do we still need to do the changes in Beats to register the new name? I guess that the only reason would be to be able to remove the rename in the future?

[narph]

Do we still need to do the changes in Beats to register the new name? I guess that the only reason would be to be able to remove the rename in the future?

Yes, the reason I would still add is to remove the rename pipeline in the future for the data stream

[narph]

closing in favor of #28447