Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Azure Service Bus message generation with dependency ID #259

Closed
stijnmoreels opened this issue Mar 17, 2022 · 3 comments · Fixed by #312
Closed

Add Azure Service Bus message generation with dependency ID #259

stijnmoreels opened this issue Mar 17, 2022 · 3 comments · Fixed by #312
Assignees
Labels
area:correlation All issues related to correlation feature All issues related to new features
Milestone

Comments

@stijnmoreels
Copy link
Member

Is your feature request related to a problem? Please describe.
When creating an Azure Service Bus message to be placed on a queue or topic subscription, we need to add an dependency ID to the context properties so that the peaking message pump can track a request in Application Insights.

Describe the solution you'd like
Investigates in how this can be done. Should we extend our extension .AsServiceBusMessage to incorporate this? Should we instead extend the QueueClient and TopicClient to easier access? Keeping in mind that you usually start from a model that needs to be serialized as the message body of the Azure Service Bus message.

Additional context
Relates to https://github.com/arcus-azure/arcus-service-to-service-correlation-poc

@stijnmoreels stijnmoreels added feature All issues related to new features area:correlation All issues related to correlation labels Mar 17, 2022
@stijnmoreels stijnmoreels added this to the v1.2.0 milestone Mar 17, 2022
@fgheysels
Copy link
Member

The dependencyId should not be part of the actual (business) payload of the message. It should be a meta-property / context property.

The amqp protocol already provides a 'CorrelationId' property, but I feel we should not use this property for our purpose. This is rather a property which is used to correlate between a message and their reply messages.
https://docs.microsoft.com/en-us/azure/service-bus-messaging/service-bus-messages-payloads

I think we should add a custom property to the user-defined meta-properties of the message, and let the user specify how that property must be called. (We can also provide a default name).

@fgheysels
Copy link
Member

fgheysels commented Mar 17, 2022

I am currently wondering if setting the correlation identifier in the message that is posted to servicebus, is the responsability of Arcus.

I'm currently thinking that we should give the developer freedom to decide how / where the correlation identifier must be placed and how that identifier should look like.

For the consumer-side, we should provide some functionality in Arcus to easily retrieve that correlation-id and make sure that it is correctly used when writing the request to the logs (AppInsights). Also, in this case, the developer must be able to specify where the correlation-id can be found.

But, that is just my idea. It would be nice to have some thoughts / ideas / input from other people as well.

@stijnmoreels
Copy link
Member Author

Yes, that's a good point. We have already issues created on web API that will help adding this ID when sending out HTTP requests (arcus-azure/arcus.webapi#257 & arcus-azure/arcus.webapi#258). Those ones are of the same 'action' as this one. And so, are also susceptible to this argument?

Without doubt, service-to-service correlation is hard to understand and set up. I'm wondering, if we start documenting this, how we can easily help people if all dependency ID assignment is open and no functionality is available.

There are already lots of topics that could benefit from having input from other people. 😄 But those topics are sometimes on-hold and otherwise done without the feedback. We could definitely have a look how we would do this, ask feedback to the community, but this is maybe something to discuss with everyone involved with Arcus.

@stijnmoreels stijnmoreels modified the milestones: v1.2.0, v1.3.0 Mar 31, 2022
@stijnmoreels stijnmoreels self-assigned this Jul 12, 2022
@stijnmoreels stijnmoreels linked a pull request Jul 12, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:correlation All issues related to correlation feature All issues related to new features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants