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

feat: separate servers as provided and used ones #874

Closed

Conversation

fmvilas
Copy link
Member

@fmvilas fmvilas commented Nov 20, 2022

Separate servers as providedServers and usedServers

Related issue(s):

#689
#618

Potential concerns

Existing discussion already happening at #811.

Now providedServers and usedServers don't make sense anymore in the "domain" kind of document. It makes sense when we're talking about an application only. Some ideas on how to overcome this issue:

Idea 1. Have servers, providedServers, and usedServers

servers:
  myWSServer:
    ...
  kafka:
    ...

providedServers:
  - $ref: '#/servers/myWSServer'

usedServers:
  - $ref: '#/servers/kafka'

This adds unnecessary overhead to the document but might not be that bad. We should restrict [provided|used]Servers to be $ref pointers to the servers object. In other words, they should always start with #/servers/. Otherwise, why would we want to have servers in the first place, right?

Idea 2. Add a new top-level field to determine the kind of document.

This will clearly help us identify when the user is using AsyncAPI to document an application or a collection/domain of things. For instance:

asyncapi: 3.0.0
kind: application # The default if not specified

providedServers: ...
usedServers: ....
channels: ...
operations: ...
components: ...
asyncapi: 3.0.0
kind: domain # or collection, or menu, or library, or whatever we come up with :)

servers: ...
channels: ...
components: ...

Idea 3. Create a new spec for the "domain" kind of document.

We can still start these documents with:

asyncapi: 3.0.0
kind: domain # or collection, or menu, or library, or whatever we come up with :)

However, the spec itself will be in a separate place. The reason I'm proposing this idea is that the current spec contains lots of references to "this application", which will not make sense in a kind of document that's not meant for applications. We either separate specs in different documents or fix these occurrences and are very careful not to introduce new ones in the future.

Idea 4. Split the AsyncAPIObject (root object) in the spec into application and domain objects.

We keep everything in the same spec but we make it super clear that application-related documents can have a set of fields and domain-related documents can have a different one. We fix the "this application" occurrences and are very careful not to introduce new ones in the future.


Notice how this problem is not appearing only with the introduction of usedServers and providedServers. We already had that problem with operations because they only make sense in an application kind of document.

We now have 3 different properties that don't make sense outside the application realm. We should clean this before it bites us later.

@fmvilas fmvilas added the 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) label Nov 20, 2022
@sonarcloud
Copy link

sonarcloud bot commented Nov 20, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@magicmatatjahu
Copy link
Member

I think this problem is also related to the "collection of the applications" problem - #658

Honestly looking at the solutions I don't think I like any of them 😆 or rather, I don't like the ways of describing these things. At first glance I wondered why we need this kind: domain field (I think system would be a better term), however with the current naming in the spec it would be difficult to determine if the spec is describing an application or some system/broker by looking only at the definitions of channels and servers.

I also wonder why you dropped the nested fields servers.locals and servers.remotes. Would you explain? Do you see problems with nested $refs?

Maybe instead of the kind field, maybe we should think about this applications field (#658) again and if this field is defined then you can't have operations, usedServers and the document describes the system/domain defined?

On the other hand, the kind: application|system|library field is so much easier to understand that it would give easier to write tooling for a given type of specification.

It's hard for me to say which way we should go.

@fmvilas
Copy link
Member Author

fmvilas commented Nov 28, 2022

I think this problem is also related to the "collection of the applications" problem - #658

Yeah, it may also be related but we ditched this long ago. We're not gonna solve it for v3.0.0.

At first glance I wondered why we need this kind: domain field (I think system would be a better term)

A system can also be a collection of applications, which can make it confusing. A domain is a more common word in EDA and Domain-Driven Design. A domain is more about the boundaries of the information and the responsibilities. If we're not happy with that word, I'd suggest using library instead, as that's what it is, a library of reusable objects. They don't define anything specifically. It's just a way to keep things together in a single place. The reason I was calling it domain is that people usually group these things by information domain.

I also wonder why you dropped the nested fields servers.locals and servers.remotes. Would you explain? Do you see problems with nested $refs?

No problem. I just thought this naming was more explicit as suggested by @thake at #689 (comment).

Maybe instead of the kind field, maybe we should think about this applications field (#658) again and if this field is defined then you can't have operations, usedServers and the document describes the system/domain defined?

I think you're confusing domain with a collection of applications? 🤔 It's definitely not its purpose.

@derberg
Copy link
Member

derberg commented Nov 28, 2022

I'm having a hard time understanding what we need it for. How is this related to pub/sub confusion?

remote/local made sense. Majority of cases are EDA with broker, so remote is default. Folks doing websocket would use local. Only reusing servers with $ref would not be nice. Is this why you propose separate servers objects?

And sorry but I still don't get 👇🏼 #811 (comment)

It is not "it is Application or a Broker". It is about being an Application or simply a collection of resources (messages, channels, servers, etc.). I called the latter a Domain for the lack of a better term. Can also be called a Library, Menu, etc.

Where is this "domain" idea coming from originally? I only heard from people here and there that they would like to "describe their broker with AsyncAPI" or "that having AsyncAPI document created for a broker is useful for them as then they could have broker-specific bindings also provided to handle broker provisioning based on provided data". So at the moment, my brain automatically maps the idea of a domain to the idea of enabling people to describe broker

PS. mentioning it here and not in related issue as I think it is related

@fmvilas
Copy link
Member Author

fmvilas commented Nov 29, 2022

Only reusing servers with $ref would not be nice. Is this why you propose separate servers objects?

Yes, that's exactly why I think it should be separate things. A server may be local for one application but remote for the rest. So local or remote is not a property of the server itself. It's a property of the relationship between the application and the server. Therefore, I'm separating them to improve reusability.

Where is this "domain" idea coming from originally? I only heard from people here and there that they would like to "describe their broker with AsyncAPI" or "that having AsyncAPI document created for a broker is useful for them as then they could have broker-specific bindings also provided to handle broker provisioning based on provided data". So at the moment, my brain automatically maps the idea of a domain to the idea of enabling people to describe broker

Yes, some people will want it to define a broker. Some people might want to define a subset of what they find in a broker. Some people will want to define reusable objects between an HTTP/WS server and client(s). So, what I'm trying to do here is to be a little less opinionated and offer a solution that will work for all of them. Again, domain might not be the best term, say we go with library or collection. Something that lets you group stuff together. It may represent a broker. It may represent just a collection of common stuff you $ref from multiple files. Remember this article I wrote a while back: https://www.asyncapi.com/blog/organizing-asyncapi-documents. I'm just trying to make the "common/*.json" documents a thing instead of leaving them there as unstructured JSON/YAML files. And that would also solve the problem for those wanting to document their whole system/broker, or a part of it (domain).

Does that make sense now @derberg?

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Nov 29, 2022

Thanks for the clarification!

Hmm, when it comes to naming domain|library I would prefer domain - of course, if we are talking about what this PR wants to add - in my previous comment I misunderstood the idea of domain and thought there was a "bigger" system behind it. From what I understand this idea of domains is trying to create some kind of "interface" that applications should "implement" - this is not exactly interface but rather information relative to which "regions" the application works in the system.

As for this naming, as I wrote I prefer the name domain because library has more of a meaning that something is generic. The domain itself does not have to be such - for example, we can have several domains and in relation to this use some messages collection - so under the name library there should be something like that:

asyncapi: 3.0.0
kind: library
info:
  title: Reusable components of domain/system
  version: 0.1.0
components:
  ...

no channels, servers etc, only components.

It also makes me wonder how this kind: domain can then interact with (if we define in the future) kind: system. What we wanted to do in the proposal #658 (with the addition of the applications field) we are now doing in the opposite way, i.e. describing that a given application is part of a certain domain, not that a given system consists of given applications. Do I understand this correctly?

@fmvilas
Copy link
Member Author

fmvilas commented Nov 30, 2022

It also makes me wonder how this kind: domain can then interact with (if we define in the future) kind: system. What we wanted to do in the proposal #658 (with the addition of the applications field) we are now doing in the opposite way, i.e. describing that a given application is part of a certain domain, not that a given system consists of given applications. Do I understand this correctly?

Partially 😄 We're not saying that an application belongs to a certain domain at any time. Only that an application is using "resources" from a given domain (when the application AsyncAPI file $refs something on a domain kind of document). For instance, messages, schemas, servers, etc.

If we ever want to get into describing a system, it's totally fine. A system is a group of applications and domains are just a collection of stuff that applications can use. Application documents would use domains as I'm proposing now and system documents would just reference a bunch of application documents.

As for this naming, as I wrote I prefer the name domain because library has more of a meaning that something is generic.

Please note that "something is generic" might be an advantage here. Some people might be into DDD and will understand the concept of a domain but some will not. Actually, most engineers don't understand it or deal with this concept at all :)

no channels, servers etc, only components.

I like it. It might make it easier to spot.

@derberg
Copy link
Member

derberg commented Jan 16, 2023

It will be a long comment, sorry, but I want to write down how I got to my conclusions.


AsyncAPI for Kafka broker

  • I get we have people that want to have AsyncAPI document where they can provide all topics/messages. Much easier for visualisation tools + nice to have a single source of truth, even if schemas are in the schema registry.
  • people also want it so they can use AsyncAPI document as a source of information for deployment, so they can specify the number of replicas and stuff like that in bindings.
  • server information, in this case, is the same for both, provided and used
  • In this case, I think it is ok to say that if provided and used is missing, it means server is both.
  • We do not need operations as they do not make much sense. Anyone can consume/produce to any topic, as long as you are able to authorize.

AsyncAPI for WebSocket server

  • WebSocket server is nothing else than an app
  • You need to specify operations to make it clear to users what they can do with the server
  • server information, in this case, is the same for both, provided and used

AsyncAPI for "I have no idea what broker" but for transport WebSocket is used

  • what AsyncAPI document should I use, should I expose all topics, no operations, and treat it as broker.
  • maybe I should have 2 files, one that I use for internals and one that I use for customers. So for internals, broker document contains only operations that internals can do, and for customers vice versa. In both I would have operations provided. And then there would be 3rd document without operations. In the end it is all about an interface and what documents I give my users access to

AsyncAPI for consumer app that listens to some specific messages from topic A of some Kafka broker

  • this is the most common right, I provide document for my consumer or producer (of course my app can do both)
  • operations are obviously needed
  • providedServers is info about the deployment of my app, and userServers is basically details on how to connect to the broker

Some thoughts on "application"

At the moment spec says:

The AsyncAPI specification does not assume any kind of software topology, architecture or pattern. Therefore, a server MAY be a message broker, a web server or any other kind of computer program capable of sending and/or receiving data. However, AsyncAPI offers a mechanism called "bindings" that aims to help with more specific information about the protocol.

So doesn't matter if you have application, broker, or one file for "internal" broker operations and one file for "external" broker operations, or maybe you just describe operations of a set of IoT devices -> in the end we do not, and we should not assume what it is. It is all about interfaces that you/we provide for your audience.

The reason I'm proposing this idea is that the current spec contains lots of references to "this application", which will not make sense in a kind of document that's not meant for applications

I think we just need to update the spec to not use this application as it brings confusion and contradicts does not assume any kind of software topology statement.

We should just describe all possible definitions out there (we kinda have it already) and introduce a clear definition that groups all the others. Taking current spec:

  • The file(s) MUST describe the operations an application accepts should say The file(s) MUST describe the operations your interface exposes

  • Identifier of the application the AsyncAPI document is defining. should say Identifier of the interface the AsyncAPI document is defining.

  • For example, an operation might describe a chat application use case where a user sends a text message to a group. A publish operation describes messages that are received by the chat application, whereas a subscribe operation describes messages that are sent by the chat application. - this one is good, and we just need to make sure we provide another example, not for application

Remember this article I wrote a while back: https://www.asyncapi.com/blog/organizing-asyncapi-documents. I'm just trying to make the "common/*.json" documents a thing instead of leaving them there as unstructured JSON/YAML files. And that would also solve the problem for those wanting to document their whole system/broker, or a part of it (domain).

@fmvilas Yeah, we have this social media example in the spec repo where we have common json as well. In your article you write:

The same way you don’t use a single OpenAPI (Swagger) file to define many of your REST APIs

This is actually common and makes lots of sense to me. Otherwise APIMatic would not be a profitable business. In the end it is all about the interface. Doesn't matter what is beneath, how many APIs/servers.

The same is with AsyncAPI. You can have multiple files that you bundle into one for the customer.

An AsyncAPI file is meant to define the behavior of a single application

So my point is, we should just say An AsyncAPI document is meant to define the interface of any actor in your architecture, whether it is a broker, microservice or any other server


The conclusion of my brain exercise is that:

  • I do not think we need a separate spec.
  • We say AsyncAPI is for API/Interface. Imho we do not need local/remote or used/provided. Server is interface info, on how to connect to it. Enabling AsyncAPI to be also an interface deployment infomation does not make much sense. We will get confusion on the bindings level imho
  • I really do not understand why we need a property to say kind: domain (no matter wording and location). Why tools need to behave differently depending on what "flag" was provided. In the end it is human that choose a tool and uses it.
    • AsyncAPI document maintainer know what interface they're documenting, so they know that they should not use it as a collection, we should just explain that separate AsyncAPI document should be created, with proper title and description.
    • AsyncAPI document users know from the title and description what document is it and what is its purpose, no flag needed
    • If I have glee, and it is capable of being either client or server, then I don't think it should be on a spec level where it is told if AsyncAPI document is for client or for server. It should be a conscious decision of the user/maintainer to provide proper flags to the tool, to get desired result.

Sorry for a long comment but I wanted to write down my thoughts and how I got into my conclusions so it is easier to understand where I'm coming from and prove me wrong. I just think that:

  • we just need to improve descriptions and examples
  • not all should be solved by props in spec. I just think that tools are used by humans, and tools should just have flags that humans can use. And if, for example, users have AsyncAPI document that should be used for client generation, they should be allowed to generate server if they want, without AsyncAPI document modification.

I wrote it on Monday, I'm wondering how many mistakes and stupid conclusions I made 😃 Would be cool if at least one person in the world agrees.

@dalelane
Copy link
Collaborator

My thoughts are along a similar line to @derberg, I think. My use of AsyncAPI is essentially API documentation for Kafka - to allow someone who has a Kafka topic to share that Kafka topic with other developers in their organisation, by enabling them to describe the events that are on that topic and how they could be used.

That needs to include describing where the Kafka cluster is - to allow other developers to be able to connect to the cluster and consume the events.

But if we went down this path of separating servers - which would I use. In this scenario, would my Kafka cluster be a "provided" server or a "used" server?

@fmvilas
Copy link
Member Author

fmvilas commented Jan 19, 2023

But if we went down this path of separating servers - which would I use. In this scenario, would my Kafka cluster be a "provided" server or a "used" server?

@dalelane you would do it under servers and would ignore providedServers or usedServers. I'm not removing the servers property precisely for this case. I'm doing this PR for those who use AsyncAPI to define an application. So you can "annotate" a server to say that your application is providing it (the application is a server) or use it as a client.

@fmvilas
Copy link
Member Author

fmvilas commented Jan 23, 2023

As I can see, this PR is not ready to be introduced in v3.0.0 and it might even introduce more confusion. Therefore, I'm going to close it and will propose an x-providedServers extension on the extension catalog if/when we implement support for it on Glee. In any case, adding a providedServers keyword on the root would not be a breaking change so there's no reason to rush.

Agree with that? @derberg @dalelane @char0n @smoya @magicmatatjahu @jonaslagoni

@fmvilas fmvilas closed this Feb 1, 2023
@fmvilas fmvilas deleted the provided-used-servers branch February 1, 2023 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants