Stream analytics output to CosmosDB #16441

jiaweitao001 · 2022-04-19T07:45:59Z

Added azurerm_stream_analytics_output_cosmosdb resource and corresponding tests.

stephybun

Hi @jiaweitao001, thanks for your effort on this PR. Overall this is off to a good start, I left some questions and comments in-line, once those are fixed up we can take another look through this.
Thanks!

internal/services/streamanalytics/stream_analytics_output_cosmosdb_resource.go

stephybun · 2022-04-19T09:14:30Z

website/docs/r/stream_analytics_output_cosmosdb.html.markdown

+
+* `database` - (Required) The name or id of the CosmosDB database.
+
+* `collection_name_pattern` - (Required) The collection name pattern for the collection to be used. It can be the name of the container, or a collection name format which contains an optional {partition} token.


As mentioned above I think it makes more sense for this to be named container_id - I also haven't seen an a test that provides this property with an optional partition token. Could you find an example of what that looks like? This might need to be added as an additional/separate field to the resource.

I'll add another test for the scenario with an optional partition token.

stephybun · 2022-04-19T09:18:42Z

website/docs/r/stream_analytics_output_cosmosdb.html.markdown

+
+* `document_id` - (Optional) The name of the field in output events used to specify the primary key which insert or update operations are based on.
+
+* `partition_key` - (Optional) The name of the field in output events used to specify the key for partitioning output across collections. If 'collection_name_pattern' contains the {partition} token, this property is required to be specified.


If this is required in specific circumstances should we be checking this in the create function?

If the partition_key does not exist under the described circumstance, an error message will show up to complain about the missing field, test proven.

website/docs/r/stream_analytics_output_cosmosdb.html.markdown

stephybun

Thanks for making those changes @jiaweitao001! There are still a few things to fix up before we can merge this though.

In addition to the comments left in-line I would like to come back to the comments left in my initial review on the collection_name_pattern. As mentioned this property is referred to as Container Name in the portal. In addition the field partition_key isn't exposed at all and as far as I can tell doesn't actually do anything.

Despite the fact that the API allows the creation of a name pattern with a partition key e.g. container-name{partition}, this results in an unusable connection with the following error.

After some digging I found this comment which says that this is deprecated and no longer supported - the portal reflects this behaviour.

As a result I think we should rename the property collection_name_pattern to container_name and remove the partition_key property. WDYT?

stephybun · 2022-04-25T09:42:17Z

internal/services/streamanalytics/parse/output.go

+func (id OutputId) StreamingJobID() string {
+	fmtString := "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.StreamAnalytics/streamingjobs/%s"
+	return fmt.Sprintf(fmtString, id.SubscriptionId, id.ResourceGroup, id.StreamingjobName)
+}
+


These files are generated so this will be overwritten when the provider is built

Suggested change

func (id OutputId) StreamingJobID() string {

fmtString := "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.StreamAnalytics/streamingjobs/%s"

return fmt.Sprintf(fmtString, id.SubscriptionId, id.ResourceGroup, id.StreamingjobName)

}

Will remove.

stephybun · 2022-04-25T09:44:18Z

internal/services/streamanalytics/stream_analytics_output_cosmosdb_resource.go

+
+				state := OutputCosmosDBResourceModel{
+					Name:               id.Name,
+					StreamAnalyticsJob: id.StreamingJobID(),


We should generate the streaming job ID here

Suggested change

state := OutputCosmosDBResourceModel{

Name: id.Name,

StreamAnalyticsJob: id.StreamingJobID(),

streamingJobId := parse.NewStreamingJobID(id.SubscriptionId, id.ResourceGroup, id.StreamingjobName)

state := OutputCosmosDBResourceModel{

Name: id.Name,

StreamAnalyticsJob: streamingJobId.ID(),

Sure. Will fix.

stephybun · 2022-04-25T10:19:38Z

internal/services/streamanalytics/stream_analytics_output_cosmosdb_resource.go

+			client := metadata.Client.StreamAnalytics.OutputsClient
+			subscriptionId := metadata.Client.Account.SubscriptionId
+
+			streamingJobStruct, err := parse.StreamingJobID(model.StreamAnalyticsJob)


The variable type Struct in the name is redundant information, we usually append these with Id to help with readability

Suggested change

streamingJobStruct, err := parse.StreamingJobID(model.StreamAnalyticsJob)

streamingJobId err := parse.StreamingJobID(model.StreamAnalyticsJob)

Will rename.

stephybun · 2022-04-25T10:20:03Z

internal/services/streamanalytics/stream_analytics_output_cosmosdb_resource.go

+				return metadata.ResourceRequiresImport(r.ResourceType(), id)
+			}
+
+			databaseIdStruct, err := cosmosParse.SqlDatabaseID(model.Database)


Suggested change

databaseIdStruct, err := cosmosParse.SqlDatabaseID(model.Database)

databaseId, err := cosmosParse.SqlDatabaseID(model.Database)

Will rename.

stephybun · 2022-04-25T11:58:42Z

internal/services/streamanalytics/stream_analytics_output_cosmosdb_resource.go

+			documentDbOutputProps := &streamanalytics.DocumentDbOutputDataSourceProperties{
+				AccountID:             utils.String(databaseIdStruct.DatabaseAccountName),
+				AccountKey:            utils.String(model.AccountKey),
+				Database:              utils.String(model.Database),


This should actually be the database name and not the ID. Even though you can create the SA output with the ID, the connectivity test will fail.

Suggested change

Database: utils.String(model.Database),

Database: utils.String(databaseId.Name

stephybun · 2022-04-25T12:01:42Z

internal/services/streamanalytics/stream_analytics_output_cosmosdb_resource.go

+
+			if metadata.ResourceData.HasChangesExcept("name", "stream_analytics_job_id") {
+				props := streamanalytics.Output{
+					OutputProperties: &streamanalytics.OutputProperties{
+						Datasource: streamanalytics.DocumentDbOutputDataSource{
+							Type: streamanalytics.TypeBasicOutputDataSourceTypeMicrosoftStorageDocumentDB,
+							DocumentDbOutputDataSourceProperties: &streamanalytics.DocumentDbOutputDataSourceProperties{
+								AccountKey:            &state.AccountKey,
+								Database:              &state.Database,


Same here, we need to parse this value as an ID and send the name of the database

Suggested change

if metadata.ResourceData.HasChangesExcept("name", "stream_analytics_job_id") {

props := streamanalytics.Output{

OutputProperties: &streamanalytics.OutputProperties{

Datasource: streamanalytics.DocumentDbOutputDataSource{

Type: streamanalytics.TypeBasicOutputDataSourceTypeMicrosoftStorageDocumentDB,

DocumentDbOutputDataSourceProperties: &streamanalytics.DocumentDbOutputDataSourceProperties{

AccountKey: &state.AccountKey,

Database: &state.Database,

databaseId, err := cosmosParse.SqlDatabaseID(state.Database)

if err != nil {

return err

}

if metadata.ResourceData.HasChangesExcept("name", "stream_analytics_job_id") {

props := streamanalytics.Output{

OutputProperties: &streamanalytics.OutputProperties{

Datasource: streamanalytics.DocumentDbOutputDataSource{

Type: streamanalytics.TypeBasicOutputDataSourceTypeMicrosoftStorageDocumentDB,

DocumentDbOutputDataSourceProperties: &streamanalytics.DocumentDbOutputDataSourceProperties{

AccountKey: &state.AccountKey,

Database: &databaseId.Name,

stephybun · 2022-04-25T12:02:33Z

internal/services/streamanalytics/stream_analytics_output_cosmosdb_resource_test.go

+  resource_group_name = azurerm_cosmosdb_account.test.resource_group_name
+  account_name        = azurerm_cosmosdb_account.test.name
+  database_name       = azurerm_cosmosdb_sql_database.updated.name
+  partition_key_path  = "/definition/id"


This is a nested partition key which isn't supported by Stream Analytics, an error will be thrown when testing the connectivity

Suggested change

partition_key_path = "/definition/id"

partition_key_path = "/definition"

stephybun · 2022-04-25T12:02:51Z

internal/services/streamanalytics/stream_analytics_output_cosmosdb_resource_test.go

+  resource_group_name = azurerm_cosmosdb_account.test.resource_group_name
+  account_name        = azurerm_cosmosdb_account.test.name
+  database_name       = azurerm_cosmosdb_sql_database.test.name
+  partition_key_path  = "/definition/id"


Suggested change

partition_key_path = "/definition/id"

partition_key_path = "/definition"

jiaweitao001 · 2022-04-26T06:08:12Z

After testing on this param, I agree that this CollectionNamePattern can only be used as container name now. I'll rename it and remove the partition key param. Thanks for the effort on looking into this.

stephybun

Thanks for @jiaweitao001! LGTM 🍰

github-actions · 2022-04-29T03:17:33Z

This functionality has been released in v3.4.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2022-05-30T02:36:30Z

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.

Stream analytics output to CosmosDB

303eb02

github-actions bot added documentation service/stream-analytics size/XL labels Apr 19, 2022

stephybun requested changes Apr 19, 2022

View reviewed changes

stephybun added the new-resource label Apr 19, 2022

jiaweitao001 added 3 commits April 20, 2022 17:26

Addressing comments, removing resource group, etc

84c1a53

Fixing typo

1a621eb

fix typo

24d4585

jiaweitao001 requested a review from stephybun April 20, 2022 09:32

fix go imports

e7cfcec

stephybun requested changes Apr 25, 2022

View reviewed changes

Address comments

5fd51a6

jiaweitao001 requested a review from stephybun April 26, 2022 06:09

renamed CollectionNamePattern in model to ContainerName

a22266b

stephybun approved these changes Apr 26, 2022

View reviewed changes

stephybun merged commit 88fc6bc into hashicorp:main Apr 26, 2022

stephybun added a commit that referenced this pull request Apr 26, 2022

Update for #16441

1c9d64c

github-actions bot added this to the v3.4.0 milestone Apr 26, 2022

stephybun mentioned this pull request Apr 26, 2022

CosmosDB Output for Azure Stream Analytics #3809

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stream analytics output to CosmosDB #16441

Stream analytics output to CosmosDB #16441

jiaweitao001 commented Apr 19, 2022

stephybun left a comment

stephybun Apr 19, 2022

jiaweitao001 Apr 20, 2022

stephybun Apr 19, 2022

jiaweitao001 Apr 20, 2022

stephybun left a comment •

edited

Loading

stephybun Apr 25, 2022

jiaweitao001 Apr 26, 2022

stephybun Apr 25, 2022

jiaweitao001 Apr 26, 2022

stephybun Apr 25, 2022

jiaweitao001 Apr 26, 2022

stephybun Apr 25, 2022

jiaweitao001 Apr 26, 2022

stephybun Apr 25, 2022

jiaweitao001 Apr 26, 2022

stephybun Apr 25, 2022

jiaweitao001 Apr 26, 2022

stephybun Apr 25, 2022

jiaweitao001 Apr 26, 2022

stephybun Apr 25, 2022

jiaweitao001 Apr 26, 2022

jiaweitao001 commented Apr 26, 2022

stephybun left a comment

github-actions bot commented Apr 29, 2022

github-actions bot commented May 30, 2022


		* `database` - (Required) The name or id of the CosmosDB database.

		* `collection_name_pattern` - (Required) The collection name pattern for the collection to be used. It can be the name of the container, or a collection name format which contains an optional {partition} token.


		* `document_id` - (Optional) The name of the field in output events used to specify the primary key which insert or update operations are based on.

		* `partition_key` - (Optional) The name of the field in output events used to specify the key for partitioning output across collections. If 'collection_name_pattern' contains the {partition} token, this property is required to be specified.

	streamingJobStruct, err := parse.StreamingJobID(model.StreamAnalyticsJob)
	streamingJobId err := parse.StreamingJobID(model.StreamAnalyticsJob)

	databaseIdStruct, err := cosmosParse.SqlDatabaseID(model.Database)
	databaseId, err := cosmosParse.SqlDatabaseID(model.Database)

	Database: utils.String(model.Database),
	Database: utils.String(databaseId.Name

	partition_key_path = "/definition/id"
	partition_key_path = "/definition"

Stream analytics output to CosmosDB #16441

Stream analytics output to CosmosDB #16441

Conversation

jiaweitao001 commented Apr 19, 2022

stephybun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephybun left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiaweitao001 commented Apr 26, 2022

stephybun left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 29, 2022

github-actions bot commented May 30, 2022

stephybun left a comment •

edited

Loading