`azurerm_log_analytics_data_export_rule` - support for the `destination_metadata` property #17019

neil-yechenwei · 2022-05-31T04:00:41Z

This PR is to support new property destination_metadata.

Note: I think service team would expand the sub properties of destination_metadata in the future so that I make it as block.

--- PASS: TestAccLogAnalyticsDataExportRule_basic (295.37s)
--- PASS: TestAccLogAnalyticsDataExportRule_complete (303.93s)
--- PASS: TestAccLogAnalyticsDataExportRule_requiresImport (334.64s)
--- PASS: TestAccLogAnalyticsDataExportRule_destinationMetadata (405.26s)
--- PASS: TestAccLogAnalyticsDataExportRule_update (435.80s)

magodo · 2022-06-07T02:47:10Z

@neil-yechenwei LGTM 👍

website/docs/r/log_analytics_data_export_rule.html.markdown

WodansSon

@neil-yechenwei LGTM as well, just one minor comment about the documentation. 🚀

WodansSon

@neil-yechenwei Thanks for pushing that change, LGTM! 🚀

internal/services/loganalytics/log_analytics_data_export_resource.go

website/docs/r/log_analytics_data_export_rule.html.markdown

katbyte · 2022-06-08T03:33:08Z

website/docs/r/log_analytics_data_export_rule.html.markdown

+
+The `metadata` block supports the following attributes:
+
+* `eventhub_name` - (Optional) The name of the EventHub that is defined for the Log Analytics Data Export Rule.


as above, what is this actually doing? if this is setting anything? just a place to put the destination event hub name? should it just be

Suggested change

* `eventhub_name` - (Optional) The name of the EventHub that is defined for the Log Analytics Data Export Rule.

* `destination_eventhub_name` - (Optional) The name of the EventHub that Log Analytics Data Export Rule exports too.

also given we are giving the id above? what is the point of this? could we not just grab it from the ID and set it ourselves so the user doesn't have to?

It means "Select an Event Hub name to send data from all tables in the rule to a specific Event Hub. If nothing is selected, an Event Hub is created for each table in the selected namespace".

When this is setting anything, it would fail.

I think it's better to update metadata to destination_metadata and keep eventhub_name since maybe service team would introduce other destinations.

As this azure doc indicates "Select an Event Hub name to send data from all tables in the rule to a specific Event Hub. If nothing is selected, an Event Hub is created for each table in the selected namespace.", so I think supporting this property is meaningful.

As setting it or not setting it represents different meaning, so I think it should be set by user. And destination_resource_id should be set with eventhub namespace resource id or storage account id not eventhub resource id. Below data export rule is created on Azure Portal and the destination resource id is set with eventhub namespace resource id.

Well i think that's pretty self explanatory, we should match what the portal is doing. and really. going by these images, maybe have two resources 1 for storage account, 1 for eventhubs - there no metadata in those pages, and the property names are much easier to decipher when correctly named vs being overloaded in the api

Azure portal also allows to set or not set event hub name under destination metadata. So I assume my PR is aligned with Azure Portal. If I am wrong, please correct me.

Currently the destination metadata only supports event hub name.

I already updated the property name and the name description. Seems you're still referencing old commit. Could you refresh your PR to get my latest update?Thanks.

And suggest go thourgh this azure doc (https://docs.microsoft.com/en-us/azure/azure-monitor/logs/logs-data-export?tabs=portal#create-or-update-data-export-rule) and play this log analytics data export rule resource on Azure Portal first.

Or could you provide the suggested code/commit for them? Thanks in advance.

What i'm thinking is we model the two twps you can select in the portal:

resource "azurerm_log_analytics_data_export_rule_storage_account" "test" { name = "acctest-DER-%d" resource_group_name = azurerm_resource_group.test.name workspace_resource_id = azurerm_log_analytics_workspace.test.id table_names = ["Heartbeat"] destination_storage_account_id = } resource "azurerm_log_analytics_data_export_rule_evenhub" "test" { name = "acctest-DER-%d" resource_group_name = azurerm_resource_group.test.name workspace_resource_id = azurerm_log_analytics_workspace.test.id table_names = ["Heartbeat"] destination_evenhub_id = azurerm_eventhub.test.id }

or even

resource "azurerm_log_analytics_data_export_rule" "test" { name = "acctest-DER-%d" resource_group_name = azurerm_resource_group.test.name workspace_resource_id = azurerm_log_analytics_workspace.test.id table_names = ["Heartbeat"] destination_evenhub_id = azurerm_eventhub.test.id destination_storage_account_id = }

@katbyte , sorry, seems I cannot get your point.

This PR is to support new property destination_metadata not destination_resource_id. May I ask why we have to split the existing property destination_resource_id in this PR because I think they're different things? Does it make sense to implement it in another PR if we intend to split it?

As destination_resource_id already supports Storage Account and EventHub, is there any impact if we don't split destination_resource_id while introducing destination_metadata in this PR?

Do you agree with the part of destination_metadata in this PR?

website/docs/r/log_analytics_data_export_rule.html.markdown

neil-yechenwei · 2022-06-09T09:20:24Z

@katbyte , thanks for your comments. I've updated code and shared my understanding. Please take another look. Thanks in advance.

katbyte · 2022-06-08T03:30:45Z

internal/services/loganalytics/log_analytics_data_export_resource.go

+	}
+
+	var eventHubName string
+	if input.DestinationMetaData.EventHubName != nil {


the name is destination metadata? is this metadata added to the destination OR is it the final destination for the data?

Yes. It's destination metadata. The whole block means the data from all tables in the rule are sent to a specific Event Hub when an Event Hub name is selected. See more details from https://docs.microsoft.com/en-us/azure/azure-monitor/logs/logs-data-export?tabs=portal#create-or-update-data-export-rule.

katbyte · 2022-06-27T23:53:29Z

website/docs/r/log_analytics_data_export_rule.html.markdown

+
+The `metadata` block supports the following attributes:
+
+* `eventhub_name` - (Optional) The name of the EventHub that is defined for the Log Analytics Data Export Rule.


Well i think that's pretty self explanatory, we should match what the portal is doing. and really. going by these images, maybe have two resources 1 for storage account, 1 for eventhubs - there no metadata in those pages, and the property names are much easier to decipher when correctly named vs being overloaded in the api

neil-yechenwei · 2022-06-28T02:01:16Z

@katbyte , Azure portal also allows to set or not set event hub name under destination metadata. So I assume my PR is aligned with Azure Portal. If I am wrong, please correct me.

Currently the destination metadata only supports event hub name. Maybe it would support more metdatas when service team introduces other new destination type in the future.

I already updated the property name and the name description. Seems you're still referencing old commit. Could you refresh the PR to get my latest update?Thanks.

And suggest go thourgh this azure doc (https://docs.microsoft.com/en-us/azure/azure-monitor/logs/logs-data-export?tabs=portal#create-or-update-data-export-rule) and play this log analytics data export rule resource on Azure Portal to better understand the background.

Or if it's possible, could you provide the suggested code/commit for them? Thanks in advance.

neil-yechenwei · 2022-08-22T05:32:56Z

As the destination_metadata property has been implemented by another PR and it has been merged, so close this PR.

github-actions · 2022-09-22T02:42:46Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

neil-yechenwei added 5 commits May 31, 2022 11:51

azurerm_log_analytics_data_export_rule: support new property metadata

95d8837

update code

fa3f13b

update code

2bebc92

update code

ccfa9ae

update code

989e8d9

github-actions bot added documentation service/log-analytics size/M labels May 31, 2022

neil-yechenwei added 3 commits June 6, 2022 19:38

update code

f314d42

update code

1ecf7e6

update code

6e13ecc

WodansSon reviewed Jun 7, 2022

View reviewed changes

website/docs/r/log_analytics_data_export_rule.html.markdown Outdated Show resolved Hide resolved

WodansSon approved these changes Jun 7, 2022

View reviewed changes

update code

948ed4e

neil-yechenwei requested a review from WodansSon June 7, 2022 03:38

WodansSon approved these changes Jun 7, 2022

View reviewed changes

katbyte requested changes Jun 8, 2022

View reviewed changes

update code

c6512e4

neil-yechenwei requested a review from katbyte June 9, 2022 09:20

update code

c869bf5

github-actions bot added service/dns size/XL and removed size/M labels Jun 9, 2022

update code

3e09c8e

github-actions bot added size/M and removed service/dns size/XL labels Jun 9, 2022

update code

5ccdcc9

katbyte self-assigned this Jun 27, 2022

katbyte requested changes Jun 27, 2022

View reviewed changes

neil-yechenwei requested a review from katbyte June 28, 2022 02:02

neil-yechenwei changed the title ~~azurerm_log_analytics_data_export_rule: support new property metadata~~ azurerm_log_analytics_data_export_rule - support for the destination_metadata property Aug 15, 2022

neil-yechenwei added 2 commits August 22, 2022 11:28

update code

1650b07

update code

2703c2c

neil-yechenwei closed this Aug 22, 2022

github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_log_analytics_data_export_rule` - support for the `destination_metadata` property #17019

`azurerm_log_analytics_data_export_rule` - support for the `destination_metadata` property #17019

neil-yechenwei commented May 31, 2022 •

edited

Loading

magodo commented Jun 7, 2022

WodansSon left a comment

WodansSon left a comment

katbyte Jun 8, 2022

neil-yechenwei Jun 9, 2022 •

edited

Loading

katbyte Jun 27, 2022 •

edited

Loading

neil-yechenwei Jun 28, 2022

katbyte Jul 6, 2022

neil-yechenwei Jul 6, 2022 •

edited

Loading

neil-yechenwei commented Jun 9, 2022

katbyte Jun 8, 2022

neil-yechenwei Jun 28, 2022 •

edited

Loading

katbyte Jun 27, 2022 •

edited

Loading

neil-yechenwei commented Jun 28, 2022 •

edited

Loading

neil-yechenwei commented Aug 22, 2022

github-actions bot commented Sep 22, 2022


		The `metadata` block supports the following attributes:

		* `eventhub_name` - (Optional) The name of the EventHub that is defined for the Log Analytics Data Export Rule.

	* `eventhub_name` - (Optional) The name of the EventHub that is defined for the Log Analytics Data Export Rule.
	* `destination_eventhub_name` - (Optional) The name of the EventHub that Log Analytics Data Export Rule exports too.

azurerm_log_analytics_data_export_rule - support for the destination_metadata property #17019

azurerm_log_analytics_data_export_rule - support for the destination_metadata property #17019

Conversation

neil-yechenwei commented May 31, 2022 • edited Loading

magodo commented Jun 7, 2022

WodansSon left a comment

Choose a reason for hiding this comment

WodansSon left a comment

Choose a reason for hiding this comment

katbyte Jun 8, 2022

Choose a reason for hiding this comment

neil-yechenwei Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

katbyte Jun 27, 2022 • edited Loading

Choose a reason for hiding this comment

neil-yechenwei Jun 28, 2022

Choose a reason for hiding this comment

katbyte Jul 6, 2022

Choose a reason for hiding this comment

neil-yechenwei Jul 6, 2022 • edited Loading

Choose a reason for hiding this comment

neil-yechenwei commented Jun 9, 2022

katbyte Jun 8, 2022

Choose a reason for hiding this comment

neil-yechenwei Jun 28, 2022 • edited Loading

Choose a reason for hiding this comment

katbyte Jun 27, 2022 • edited Loading

Choose a reason for hiding this comment

neil-yechenwei commented Jun 28, 2022 • edited Loading

neil-yechenwei commented Aug 22, 2022

github-actions bot commented Sep 22, 2022

`azurerm_log_analytics_data_export_rule` - support for the `destination_metadata` property #17019

`azurerm_log_analytics_data_export_rule` - support for the `destination_metadata` property #17019

neil-yechenwei commented May 31, 2022 •

edited

Loading

neil-yechenwei Jun 9, 2022 •

edited

Loading

katbyte Jun 27, 2022 •

edited

Loading

neil-yechenwei Jul 6, 2022 •

edited

Loading

neil-yechenwei Jun 28, 2022 •

edited

Loading

katbyte Jun 27, 2022 •

edited

Loading

neil-yechenwei commented Jun 28, 2022 •

edited

Loading