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

Establish conventions on where we define configuration for a protocol using bindings for consistency #62

Closed
iancooper opened this issue May 24, 2021 · 16 comments
Assignees
Labels
enhancement New feature or request stale

Comments

@iancooper
Copy link
Collaborator

iancooper commented May 24, 2021

Documenting Producers and Consumers

Proposal

We should agree conventions around bindings so that different protocols are as consistent as possible as to how we use bindings on objects that support them. Today, implementers of protocols have not been consistent in application of bindings. This makes it difficult to create tooling that works with different bindings.

Discussion

Let's assume that I want to document the API exposed by a microservice, Foo, and I want to consume it from a microservice, Bar, that wants to receive via RMQ (AMQP 0-9-1), and another microservies, Wibble, that wants to receive receive via Kafka.

It is a little contrived, but the scenario let's us do a comparison of binding needs.

For reference then:

Producer Consumer
AMQP 0-9-1 Foo Bar
Kafka Foo Wibble

Now let's assume we have a channel quux which we communicate via. Foo sends a message to quux and Bar and Wibble receive a message from quux.

How would we use AsyncAPI to document this setup?

First, I would assume that we have an AsyncAPI files for the producer, Foo, and separate files for the consumers, Bar and Wibble.

Let's assume we define the Info and Server properties for both, and that the Server properties for both include a range of environments. Of course, I might use $ref to avoid a repeated definition of the object, and instead refer to a common definition.

Instead let's think about the channel.

channels:
    quux:
        subscribe:
            summary: Messages about quux
            operationId: sendQuux
            $ref: "#/components/messages/quuxEvent"

On the producer, I want to define the channel and a subscribe operation on it - to indicate that Foo produces this topic. As the producer, I need to set up this channel. What do I need to define so that I could use the Generator to create Infrastructure as Code (IAC) such as Terraform templates or an Ansible playbook to be able to send messages to this channel.

AMQP 0-9-1

For RMQ I need an exchange, this governs how messages are routed. I also need a routing key for this channel. We can assume that the channel name is the routing key name. But we need to define the exchange, so I need a binding. From the amqp binding, I'll pull the binding for an exchange. (see https://github.com/asyncapi/bindings/tree/master/amqp)

Logically, where should this be defined? Does it belong to the channel or the subscribe operation on the channel?

It's not uncommon to have a pattern where a consumer creates the channel and binds its queue to it, ahead of a producer sending to it. So we might also want to define the exchange on the consumer AsyncAPI file. It makes sense then that we do not define this on the subscribe operation, as we will not have that on the consumer API file, and adding it to the consumer API file will just increase the confusion around publish-consumer, subcribe-producer.

So we can make an initial assumption: by convention, bindings to set up the channel we communicate via live at the channel level as they may appear on both sides.

Let's look at Foo's AsyncAPI file:

channels:
    quux:
        subscribe:
            summary: Messages about quux
            operationId: sendQuux
            $ref: "#/components/messages/quuxEvent"
    bindings:
        amqp:
            exchange:
                name: myExchange
                type: topic
                durable: true
                autoDelete: false
                vhost: /
                bindingVersion: 0.1.0

Now, https://github.com/asyncapi/bindings/tree/master/amqp defines the queue here as well.

But, a queue belongs to a consumer, not a producer. I may have many consuming services, each will have their own queue if they want to obtain their own copy of the message. Logically then, I don't want my producer's AsyncAPI file to contain the definition of my consumers. The pub-sub model desires to prevent a producer having to know its consumers, so as to avoid that coupling. (Of course we may want to take the information from both sides and build a graph to document the flow).

I don't want to define a consumer's queue ijn the producer's file but in the consumer then. So where should that binding go. The channel or the operation? I would argue that the consumption belongs with the operation, because we don't possess a queue without that. If I add it to the channel I also risk confusing users of AsyncAPI. If I define a queue against the channel should it appear in both producer and consumer documents? If I define it against the publish operation, this becomes clearer.

So we can make a second assumption: by convention, bindings to configure how we consume a channel live with the publish operation on the consumer

Let's look at what Bar's AsyncAPI file looks like if we do this (note that we show the exchange definition again, in case we opt for consumer creates, and of course a $ref could optimize this):

channels:
    quux:
        publish:
            summary: Messages about quux
            operationId: sendQuux
            $ref: "#/components/messages/quuxEvent"
            bindings:
                amqp:
                    queue:
                        name: myQueue
                        type: topic
                        durable: true
                        autoDelete: false
                        bindingVersion: 0.1.0
    bindings:
        amqp:
            exchange:
                name: myExchange
                type: topic
                durable: true
                autoDelete: false
                vhost: /
                bindingVersion: 0.1.0

Finally, there are some things that impact how the producer operates that are independent of the channel. If we want to generate the producer code, we would need to configure these for the subscribe operation and not for the channel, as they make no sense on the consumer side, as we won't be generating producer code there. The main one for RMQ is enabling publisher confirms.

So we can make a third assumption: by convention, bindings to configure how we produce to a channel live with the subscribe operation on the producer

If we add something that does this (not in the existing specification so pseudo), our final producer file looks more like

channels:
    quux:
        subscribe:
            summary: Messages about quux
            operationId: sendQuux
            $ref: "#/components/messages/quuxEvent"
            bindings:
                amqp:
                    producer:
                        confirmSelect: true
    bindings:
        amqp:
            exchange:
                name: myExchange
                type: topic
                durable: true
                autoDelete: false
                vhost: /
                bindingVersion: 0.1.0

KAFKA

Kafka does not have an exchange. It's routing is uncomplicated, just subscribe to the topic.

But, for IAC there are properties we might want to configure for a given channel that do not appear in the existing specification. We'll add a couple of examples so we can see how it would look. The Admin client gives some clues as to what is needed here: https://kafka.apache.org/23/javadoc/index.html?org/apache/kafka/clients/admin/AdminClient.html

We already have amqp bindings against the channel and by following the convention above, these will be defined at the same level, which makes it easy to reason about, and write a generator for.

This leads to a meta-convention: to facilitate multiple-protocols binding authors should strive to use equivalent binding targets for their protocol, where possible

The flexibility of AsyncAPI is powerful, but if we are to create tooling we need some predictability for protocols that can conform.

In addition, how we send messages the topic has a lot of options as to how we consume. Now a lot of those may be client-specific. So the Confluent client does not necessarily have the same set of properties as other client SDKs for Kafka, and languages may vary. This is an interesting question because we may wonder if the binding here is not Kafka but Confluent? I'm not going to resolve that one in this issue, but it's worth bearing in mind for folks working on a binding.

Anyway, following our convention, if it affects how we would generate code for a producer, it ought to bind to the subscribe operation:

Here is Foo's AsyncAPI file now we have added those bindings:

channels:
    quux:
        subscribe:
            summary: Messages about quux
            operationId: sendQuux
            $ref: "#/components/messages/quuxEvent"
            bindings:
                amqp:
                    producer:
                        confirmSelect: true
                kafka:
                    producer:
                        transactionalId: Foo
                        replication: Acks.All
                        batchMessages: 10
                        retries: 3
                        queue_strategy: fifo
                        max_in_flight: 5
                     
    bindings:
        amqp:
            exchange:
                name: myExchange
                type: topic
                durable: true
                autoDelete: false
                vhost: /
                bindingVersion: 0.1.0
        kafka:
            topic:
                partitioner: ConsistentRandom
                numOfPartitions: 6
                replicationFactor: 3

How about the consumer? Of course Wibble has it's own file.

What are the key properties we need to create a client?

I would argue for the consumer group. Whilst we might consider this an implementation detail, having the group defined at the level of the AsyncAPI binding may help us with understanding what consumer a group relates to in any telemetry. We might make the same argument for consumer ids, but I would not tend to find how many consumers there were in our group in an AsyncAPI file.

There are other properties that we might want for code generation though such as those that govern how we deal with offsets. Again, there is an open question if the binding here is Kafka, or Confluent for the client SDK. I'll park that as well, as I don't think it changes the overall discussion either way it ought to be bound to the publish operation on the channel.

When we follow the publish route as above Wibble's file looks like this (it's less common to have the consumer create the topic than with RMQ, but let's show that anyway for completeness):

channels:
    quux:
        publish:
            summary: Messages about quux
            operationId: sendQuux
            $ref: "#/components/messages/quuxEvent"
            bindings:
                kafka:
                    consumer:
                        group: myGroup
                        maxPollIntervalInMs: 100
                        enableAutoOffsetStore: true
                        enableAutoCommit: true
    bindings:
        kafka:
            topic:
                partitioner: ConsistentRandom
                numOfPartitions: 6
                replicationFactor: 3

Conclusion

By contrast to the laissez-faire approach to bindings today, establishing some conventions would help us with consistency around bindings and the ability to create tooling around definitions. Whilst we may not desire the conventions above, having some agreement about where we define some of the binding properties would help both tool authors and the authors of future bindings, who would have a stronger understanding of how to implement thier own binding

@iancooper iancooper added the enhancement New feature or request label May 24, 2021
@fmvilas fmvilas self-assigned this May 24, 2021
@GeraldLoeffler
Copy link
Collaborator

See #64 for a much smaller-scoped related aspect.

@fmvilas
Copy link
Member

fmvilas commented Jun 15, 2021

I love how you've put it together. Nothing to argue against it, @iancooper. Actually, totally the opposite, let's go for it. How do you think we can set these conventions? Maybe listing them as part of the README? Also including the ones suggested by @GeraldLoeffler.

@iancooper
Copy link
Collaborator Author

@fmvilas I'll try to get together a proposal for something we could put in a README.md or a Conventions.md this week

@jessemenning
Copy link

Completely agree with @iancooper that conventions would be really helpful.

@github-actions
Copy link

github-actions bot commented Sep 8, 2021

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@github-actions github-actions bot added the stale label Sep 8, 2021
@fmvilas fmvilas added enhancement New feature or request and removed stale enhancement New feature or request labels Sep 8, 2021
@github-actions
Copy link

github-actions bot commented Jan 7, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jan 7, 2022
@fmvilas
Copy link
Member

fmvilas commented Jan 7, 2022

@iancooper is this still something you would like to do?

@github-actions github-actions bot removed the stale label Jan 8, 2022
@github-actions
Copy link

github-actions bot commented May 9, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label May 9, 2022
@iancooper
Copy link
Collaborator Author

iancooper commented May 9, 2022 via email

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Sep 7, 2022
@fmvilas
Copy link
Member

fmvilas commented Sep 7, 2022

Dear bot, don't mark this issue as stale. It's still a relevant conversation.

@github-actions github-actions bot removed the stale label Sep 8, 2022
@github-actions
Copy link

github-actions bot commented Jan 7, 2023

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jan 7, 2023
@fmvilas
Copy link
Member

fmvilas commented Jan 10, 2023

Dear bot, don't mark this issue as stale. It's still a relevant conversation.

@github-actions github-actions bot removed the stale label Jan 11, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label May 12, 2023
@fmvilas
Copy link
Member

fmvilas commented May 30, 2023

@iancooper do you need anything to move this forward?

@github-actions github-actions bot removed the stale label May 31, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Sep 29, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stale
Projects
None yet
4 participants