dapr / components-contrib

Community driven, reusable components for distributed apps
Apache License 2.0
544 stars 473 forks source link

Dapr pubsub service bus topics: Add support for including `ApplicationProperties` in subscriber #3428

Closed vbuonagura closed 2 months ago

vbuonagura commented 4 months ago

Hello, we are trying to integrate Dapr within our microservices architectures but we are experiencing some issues in accessing custom properties for messaged published through Service Bus Topics.

We are not using cloudEvents and to make events traceable, we will leverage ElasticAPM with the default Azure Service Bus capabilities based on Diagnostic-Id field. See more here.

We are publishing the message from a .NET 8 API in the following way: ` public async Task CreateNotification([FromBody] Notification notification) { if (notification is not null) { ITransaction transaction = Agent.Tracer.CurrentTransaction;

    var asyncResult = await transaction.CaptureSpan("SendEvent", ApiConstants.TypeMessaging, async(s) =>
    {
        //metadata needed to remove the cloudevent envelop
        var metadata = new Dictionary<string, string>
        {
            { "rawPayload", "true" },
            { "Diagnostic-Id", "00-"+ Agent.Tracer.CurrentTransaction.TraceId + "-" + Agent.Tracer.CurrentSpan.Id + "-" +"01"}
        };

        await _daprClient.PublishEventAsync("pubsub", "notification-topic", notification, metadata);

        return true;
    });

    return Ok("Notification Created");
}

return BadRequest();

}`

The rawPayload and Diagnostic-Id metadata are correctly set into the message custom properties, but when we receive the data in the following way, we cannot see those metadata into the http request header `Dapr.Topic("pubsub", "notification-topic")] [HttpPost("notificationreceived")] public async Task NotificationReceived([FromBody] Notification notification) { foreach (var header in Request.Headers) { _logger.LogInformation(header.Key + " = " + header.Value); }

if (notification is not null)
{
    await _notificationsService.CreateAsync(notification);

    return Ok("Notification Received");
}

return BadRequest();

}`

the only metadata available are:

Is there a way to get also custom properties with metadata?

vbuonagura commented 4 months ago

Looking at the implementation of the component here: https://github.com/dapr/components-contrib/blob/93cf5cb2c96e91940d7d8c2da4376a09062df0b8/common/component/azure/servicebus/message_pubsub.go#L56

it looks like the metadata is missing from the mapping when receiving a message from the bus

berndverst commented 3 months ago

Dapr supports pass through of non-cloud event properties if and only if are these sent as additional properties within a cloud event body. That is referred to as cloud event extension. It does not work with raw events, but instead a payload of type application/cloudevent+json must be sent, with a manual json dictionary body which then contains both legitimate cloud event properties and your custom properties. These properties are indeed retained and delivered to all components.

https://github.com/dapr/dapr/blob/199bcf4e198b247bf0f259562e02c97064cb378e/pkg/api/grpc/grpc.go#L182-L190

What you are requesting is not inherently Service Bus specific and this request occasionally comes up with other components.

The Dapr PubSub component interface does not currently specify that metadata properties are to be retained. These were only created with the intentions to control component specific behavior but not to be delivered to the consumer application.

Therefore, a larger proposal would be required to design an appropriate solution to store and deliver metadata in all Dapr pubsub components, even when cloud event envelopes are not used. I am not a fan of creating a component specific solution / approach here, especially given that we already support passing through custom data via cloud event extensions.

berndverst commented 3 months ago

Perhaps there is some confusion here based on wording of the request.

The code you pointed out will take any unknown metadata properties and add them to:

ApplicationProperties on the message. This is the place to store custom metadata in service bus (at least when using the Go SDK): https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/messaging/azservicebus#Message

https://github.com/dapr/components-contrib/blob/f0be1a2d28edbec685633e6c3b69b950daf4206f/common/component/azure/servicebus/message.go#L172-L174

I think you are seeing the ApplicationProperties set correctly.

When this was designed, the intention there was never for those custom properties to be delivered back to the subscriber.

We can certainly add support for this however.

berndverst commented 3 months ago

What needs to be done is for this function be updated to also look at the ApplicationProperties property (which itself is a map) and for that map to be merged into the resulting metadata map.

https://github.com/dapr/components-contrib/blob/f0be1a2d28edbec685633e6c3b69b950daf4206f/common/component/azure/servicebus/message_pubsub.go#L56-L109

That should give you what you want @vbuonagura and this change would apply to all Service Bus components in Dapr.

berndverst commented 3 months ago

I implemented this and you will see it in Dapr 1.14

vbuonagura commented 3 months ago

Thanks @berndverst for taking the point. I will check it with Dapr 1.14

vbuonagura commented 3 months ago

@berndverst is there a way to test the code change in preview mode?

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

github-actions[bot] commented 2 months ago

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as pinned, good first issue, help wanted or triaged/resolved. Thank you for your contributions.