Usage of EnableDeadLetteringOnFilterEvaluationExceptions in ASB transport

mrichards3 · August 31, 2020, 12:29pm

I am using Terraform to provision my application resources in Azure, including an ASB namespace and the messaging topology for NServiceBus.

The NServiceBus ASB operational scripting utilities set EnableDeadLetteringOnFilterEvaluationExceptions=false, as shown here:

github.com

Particular/NServiceBus.Transport.AzureServiceBus/blob/master/src/CommandLine/Subscription.cs#L20


      
          public static Task Create(ServiceBusAdministrationClient client, CommandArgument endpointName, CommandOption topicName, CommandOption subscriptionName)
          {
              var topicNameToUse = topicName.HasValue() ? topicName.Value() : Topic.DefaultTopicName;
              var subscriptionNameToUse = subscriptionName.HasValue() ? subscriptionName.Value() : endpointName.Value;
          
              var options = new CreateSubscriptionOptions(topicNameToUse, subscriptionNameToUse)
              {
                  LockDuration = TimeSpan.FromMinutes(5),
                  ForwardTo = endpointName.Value,
                  EnableDeadLetteringOnFilterEvaluationExceptions = false,
                  MaxDeliveryCount = int.MaxValue,
                  EnableBatchedOperations = true,
                  UserMetadata = endpointName.Value
              };
          
              return client.CreateSubscriptionAsync(options, new CreateRuleOptions("$default", new FalseRuleFilter()));
          }
          
          public static Task CreateForwarding(ServiceBusAdministrationClient client, CommandOption topicToPublishTo, CommandOption topicToSubscribeTo, string subscriptionName)
          {
              var options = new CreateSubscriptionOptions(topicToPublishTo.Value(), subscriptionName)

I am currently unable to set this property the same way using Terraform. While investigating, I discovered that the attribute that would control this was removed in version 2.0 of the Terraform azurerm provider:
https://www.terraform.io/docs/providers/azurerm/guides/2.0-upgrade-guide.html

The deprecated dead_lettering_on_filter_evaluation_exceptions field will be removed, since this is no longer used.

I am not sure why this attribute was removed from Terraform – I cannot find any reference in the Azure documentation that this setting is or will be deprecated. When I create a subscription with Terraform, this property defaults to EnableDeadLetteringOnFilterEvaluationExceptions=true.

Is there any adverse behavior we would expect to encounter with NServiceBus if this property is left as “true” instead of “false”? It would seem that properly configured subscription filters in an NServiceBus topology would never be expected to throw an exception. Are there some edge-cases that I am overlooking?

SeanFeldman · August 31, 2020, 6:15pm

Hi Mike,

The upgrade guide deprecation note is puzzling to me as well.

I’ve looked into this and something here doesn’t seem to be right. Subscription’s EnableDeadLetteringOnFilterEvaluationExceptions is not deprecated. You can see it in the current and the future (preview) .NET clients. Moreover, the property deadLetteringOnFilterEvaluationExceptions is still part of the latest version of the ARM template for subscriptions. It’s optional but not deprecated. And the default should never be set to true. On the contrary, it should be set to false.

Is there any adverse behavior we would expect to encounter with NServiceBus if this property is left as “true” instead of “false”? It would seem that properly configured subscription filters in an NServiceBus topology would never be expected to throw an exception. Are there some edge-cases that I am overlooking?

The feature is intended for testing (lower environments) and verification, not for production. You don’t want to throw an exception on the publisher’s side in production if a filter for a specific message doesn’t exist for subscribers.

I would suggest opening a support case with Terraform to get more details. I’ve also sent this question to the Azure Service Bus team to confirm my statements as well as to see if they could reach out to Terraform for clarification.

mrichards3 · September 1, 2020, 12:14pm

Sean, thanks for the quick response and digging into this.

I will open an issue in the Terraform GitHub repository to investigate further.

It appears from my initial review of the source code that the Terraform provider is not setting this attribute one way or the other when a subscription resource is created.
https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/internal/services/servicebus/resource_arm_servicebus_subscription.go#L154

If that is the case, it would seem that the default value of EnableDeadLetteringOnFilterEvaluationExceptions=true is coming from the Azure service itself.

mrichards3 · September 1, 2020, 1:55pm

Here is the issue I opened in the relevant Terraform GitHub repository:

github.com/hashicorp/terraform-provider-azurerm

Service Bus: Support for dead_lettering_on_filter_evaluation_exceptions attribute on azurerm_servicebus_subscription

opened 01:54PM - 01 Sep 20 UTC

mjr5749

service/service-bus upstream/microsoft

### Community Note * Please vote on this issue by adding a 👍 [reaction](https…://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to the original issue to help the community and maintainers prioritize this request * Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request * If you are interested in working on this issue or have submitted a pull request, please leave a comment ### Description Add support for dead_lettering_on_filter_evaluation_exceptions attribute on azurerm_servicebus_subscription. This was removed in #5718, but I can find no reference in the Azure Service Bus documentation or APIs that this attribute is deprecated, or plans to be deprecated. This was discussed on the NServiceBus community forum here: https://discuss.particular.net/t/usage-of-enabledeadletteringonfilterevaluationexceptions-in-asb-transport/2059 SeanFeldman wrote: > I’ve looked into this and something here doesn’t seem to be right. Subscription’s EnableDeadLetteringOnFilterEvaluationExceptions is not deprecated. You can see it in the current and the future (preview) .NET clients. Moreover, the property deadLetteringOnFilterEvaluationExceptions is still part of the latest version of the ARM template for subscriptions. It’s optional but not deprecated. ### New or Affected Resource(s) * azurerm_servicebus_subscription ### Potential Terraform Configuration ```hcl resource "azurerm_servicebus_subscription" "subscription" { ... dead_lettering_on_filter_evaluation_exceptions = false ... } ``` ### References  * Attribute was removed in #5718, may have been deprecated earlier than that.

SeanFeldman · September 1, 2020, 7:41pm

Hi Mike,

I’ve left a comment on the issue you’ve raised, suggesting to raise an issue with the ASB broker as well. The default value of true for EnableDeadLetteringOnFilterEvaluationExceptions that ARM is applying when the parameter is not specified feels off. Especially for production environments as it goes against SB team recommendations to use this feature for testing purposes only. A recommendation that should be documented here.

SeanFeldman · September 3, 2020, 3:17am

~~Here’s the document about use of this specific property and why it is not recommended for production:~~

~~Service Bus has one feature, used specifically for development, which should never be used in production configurations : TopicDescription.EnableFilteringMessagesBeforePublishing .~~

~~When new rules or filters are added to the topic, you can use TopicDescription.EnableFilteringMessagesBeforePublishing to verify that the new filter expression is working as expected.~~

Disregard the comment.

mrichards3 · September 3, 2020, 1:59pm

Hi Sean,

I’m confused about your last comment – aren’t these two different properties (one on a Topic, and one on a Subscription)?

If there is a relationship between these properties, I am not clear about what the relationship is.

Thanks,
Mike

SeanFeldman · September 3, 2020, 3:56pm

Hi Mike,
I’m sorry, got confused by those two. Please disregard that comment.

mrichards3 · September 11, 2020, 12:14pm

I also received confirmation from our MS support team that this property is not deprecated:

I found out that the EnableDeadLetteringOnFilterEvaluationExceptions is not deprecated, so looks like Terraform needs to fix it.

Second, what this means is, lets say I have a filter that is
x < 1/y
The subscription will look for messages that have ‘x’ and ‘y’ in it and accept the message, but then, if it finds out that ‘y’ is zero, its going to throw an exception.
It will then put the message in the dead letter queue.

mrichards3 · November 16, 2020, 5:53pm

The missing Terraform property has now been added by completion of this issue:

github.com/hashicorp/terraform-provider-azurerm

Support for azurerm_servicebus_subscription.dead_lettering_on_filter_evaluation_error

opened 06:48AM - 09 Sep 20 UTC

closed 04:39AM - 10 Sep 20 UTC

springcomp

enhancement documentation service/service-bus

### Community Note * Please vote on this issue by adding a 👍 [reaction](https…://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to the original issue to help the community and maintainers prioritize this request * Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request * If you are interested in working on this issue or have submitted a pull request, please leave a comment ### Description * azurerm_servicebus_subscription Azure Service Bus subscriptions created using `azurerm_servicebus_subscription` have their `Enable Deadlettering on Filter Evaluation Error` property set to `true`. I don’t find a corresponding `dead_lettering_on_filter_evaluation_error` property on the Terraform resource. I think this property must be surfaced to the user.