New Resource: azurerm_log_analytics_datasource_windows_performance_counter

[magodo]

This is one of the datasource requested by #3182.

There is a trade-off of interface design, is that for this specific ds, it has several different counters, and user specify one by setting the object_name. So it means user can only set one counter each time. While there are acutally kinds of counters out there.
So maybe we can give user a list to specify them at once. But later I feel it has several problems:

1. we need to iterate the list to call API one by one, unless using some concurrency (which is rare in terraform provider)
2. we need to define different names for those counters, since each counter has its own resource name/id. This is weired and maybe breaking some contract in terraform.

Another point is that currently the name, object_name, counter_name, instance_name allow arbitrary string, hence I'm using the StringIsNotEmpty validation for them.

Test Result:

💤  terraform-provider-azurerm [log_analytics_workspace_datasource] ⚡  make testacc TEST=./azurerm/internal/services/loganalytics/tests TESTARGS="-run='TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_'"
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test ./azurerm/internal/services/loganalytics/tests -v -run='TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_' -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_basic
=== PAUSE TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_basic
=== RUN   TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_complete
=== PAUSE TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_complete
=== RUN   TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_update
=== PAUSE TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_update
=== RUN   TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_requiresImport
=== PAUSE TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_requiresImport
=== CONT  TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_basic
=== CONT  TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_requiresImport
=== CONT  TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_update
=== CONT  TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_complete
--- PASS: TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_complete (180.08s)
--- PASS: TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_basic (180.31s)
--- PASS: TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_requiresImport (193.07s)
--- PASS: TestAccAzureRMLogAnalyticsDataSourceWindowsPerformanceCounter_update (238.72s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/loganalytics/tests  238.765s

[mbfrahry]

Hey @magodo, this is definitely a weird resource to craft from what we're given on the api/sdk side but I think you've nailed it. It is weird that we have to do each counter separately but I don't think there is a way to do it in bulk that doesn't feel hacky so let's stick with the way you've done it here.

My main issue is just how we're presenting the attributes to the user which I've pointed out in more detail below.

If this model is also universal across other datasources, it might be worthwhile to tease this out into it's own separate package but that work can be done once more start coming in.

Thanks for the effort here!

[magodo]

@mbfrahry Thank you for review, I have made the change accordingly, please have a check.

Regarding to:

If this model is also universal across other datasources, it might be worthwhile to tease this out into it's own separate package but that work can be done once more start coming in.

That is exactly what I was thinking about at first. While after a closely looking into the property definition of each DS(s), I find they has slightly difference among others. I listed some points which will affect code structure below which might hint we may have to implement each DS separately with different schema definitions and undertaking code redundency in some degree:

Kind	Sub-kind	Sub-sub-kind	has switch?
Windows Event	eventLogName	eventType(static)	no
Windows Performance Counter	objectName	N/A	no
IIS	N/A	N/A	yes
Linux Performance Counter	objectName	performanceCounters(dynamic)	yes
Linux Syslog	syslogName	syslogSeverities(static)	yes
Custom Log	TBD

[mbfrahry]

LGTM!

[mbfrahry]

Ahhh, that make sense. Thanks for pulling that table together.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!