Adding More Transport Protocols - A Proposal

What follows is a thought dump, so sorry if it rambles a bit.

This module is in a pretty good state with the HTTP Protocol, so i started thinking about what the next protocol that could be added.

As I thought about it, I realized a couple things.

This first is we probably need to make our top level Receiver and Emitter a little more generic. At the moment, they have things related to the HTTP Protocol in them.

This second thins is when we add a new transport protocol in, we are also potentially going to increase the dependencies for each one added. My assumption would be that we would use a library related to that transport protocol for emitting events. For example, if we added MQTT then we might use the mqtt module.

The problem, and maybe it isn't a problem, is that a user might only want or need 1 protocol, and if we had multiple protocols implemented, then they would be getting more dependencies than needed.

One solution to this could be creating new modules that depend on this module as a base for each of the new Protocols. I would assume we would just keep the HTTP protocol in the base module since, and i have no data to back this up, it might the most common to use. So if a user wanted the MQTT protocol, they could just install that module. However; if they wanted to use multiple protocols, I could see this becoming a little messy.

Another solution i thought about was some sort of plugin architecture, but i not really sure of the UX of it. with this case we would still be creating separate modules to plugin, but there would be one "entry point" to creating new Emitters and Receivers

Again, this was a thought dump to start a discussion on our best path forward.

@cloudevents/sdk-javascript-maintainers and anyone else

@lholmquist

@lholmquist thanks for bringing this up! As you know, I'm interested in moving ahead with MQTT protocol support in the near future.

One thing to consider is that we don't really need to support the MQTT protocol itself, we just need to provide a means for developers to express a CloudEvent as an MQTT publish message. In fact, I would say with regard to HTTP, our choice to use Axios to send over the wire may not actually be the best thing. The module could be smaller and more concise if we simply provided methods similar to this.

// send a cloud event
const ce = new CloudEvent({ /** attributes */ });
const structuredHTTPMessage = HTTP.asStructured(ce);

// then it's up to the developer to actually send these using whatever HTTP module they like
axios.request( {
  method: POST,
  data: structuredHTTPMessage.body,
  headers: structuredHTTPMessage.headers
});

// receive a cloud event using express
app.post('/', (req, res) => {
  const incomingEvent = HTTP.receive(req.headers, req.body);
});

In fact, that's essentially how our Receiver works today. It doesn't really know about the HTTP protocol itself - it just accepts some headers and a body. It's a little strange to me that we do it so differently by actually sending an event over the wire when emitting an event.

I say all of this to emphasize that we don't really need to worry about the MQTT protocol. We just need to provide a way to transform a CloudEvent instance into an MQTT publish message, and to read an MQTT publish message and convert it into an event. This is good, because the mqtt module is not small. Including all of its dependencies, it runs about 10MB.

It's a little strange to me that we do it so differently by actually sending an event over the wire when emitting an event.

One reason i can see having an Emitter, is to make the users life a little easier with not having to worry about setting the correct method and those other tiny details. But I think that making this library smaller, and focused on just formatting(receieving and sending) the CloudEvents might be a good direction to go in

That would allow a user to use their http lib of choice(or other protocol lib of choice).

This would also solve the issue #314 about the Emitter being static, since we wouldn't have an emitter function anymore

@lance

So i wanted to close the loop on this, since we talked about this a little bit in the SDK meeting. It looks like we might be in favor of reworking Emitter to remove any type of transport functionality and let the user decide which library they want to use to send/emit the events.

This would allow us not to have to add other 3rd part libs related to sending over a protocol and also makes it so we don't have to worry about creating plugins

We would provide a function/functions that takes a CloudEvent. The return value could be an object with properly formatted properties such as headers and body that a user would use in with their transport library

An example of what a function could look like for HTTP, it could look something like @lance post above:

HTTP.asStructured(ce)

and a MQTT version might look like: MQTT.asStructured(ce)

Another version could be the function taking the CloudEvent, the mode(structured/binary) and the protocol

functionName({cloudEvent: ce, mode: Mode.Structured, protocol: Protocol.HTTP})

I'm not tied to either option

As for the Receiver class, I think the way that works now, is probably ok. I still think the top level Receiver function needs to be refactored to remove HTTP related code. HTTP can still be the default though

I think once we have a path forward, we should add a deprecation warning to the current emitter and release a version, so users now what is happening

Thanks for the follow up. In terms of namespacing, what do you think about having this and some sender interfaces under a messages namespaces? Here's a strawman API.

const { CloudEvent } = require('cloudevents');
const axios = require('axios');

// this could also be exported at the top level - shown this way to illustrate the namespace
const { HTTP } = require('cloudevents/messages');

// user-provided send function conforming to an interface
// each protocol may have it's own interface depending on
// the message structure determined by the spec. For HTTP
// this is, of course, the headers and body
function sender(headers, body) {
  return axios.post('https://receiver.example.com', {
    headers: headers,
    data: body
  });
}

// create a CloudEvent
const ce = new CloudEvent({
  source: '/com.redhat.time.server',
  type: 'com.redhat.time',
  data: Date.now()
});

// create a timestamp message in binary format
const message = HTTP.asBinary(ce);

// send the message
HTTP.send(sender, message);

// This could all be inlined, of course
HTTP.send((headers, body) => {
  return axios.post('https://receiver.example.com', {
    headers: headers,
    data: body
  });
}, HTTP.asBinary(new CloudEvent({
  source: '/com.redhat.time.server',
  type: 'com.redhat.time',
  data: Date.now()
})));

I like the concept. I know in the above post i said we could probably leave the Receiver alone, but would make sense to add an accept method(or similar) to this, so HTTP.accept or MQTT.accept?

I realize this is just a strawman, but i wonder if it makes sense for the send methods signature to be send(message, sender) instead. I've been doing some swift in my spare time and it is starting to influence my function names :) So that function would read " send a message using this sender "

@cloudevents/sdk-javascript-maintainers and anyone else

Hey, thanks for raising this. I'll add my 2 cents.

Ideally this module does not need to worry about the protocol or module we want to use with the protocol too much. We can just provide utility functions to help a protocol. For example, we shouldn't wrap modules like axios because axios is going to have many features that aren't in this library and folks can just use that module or other ones directly.

As Lance mentioned, the receiver.ts and emitter.ts logic seen here
https://github.com/cloudevents/sdk-javascript/tree/main/src/transport
should really be in the http folder as it is http specific. Yup 👍

With respect to some of the ideas above, Node core and node modules, from what I know, it's less common to have classes like HTTP and HTTP.accept. It's more common to export functions import {accept} from 'cloudevents/transport/http'.
Likewise, I could imagine import {accept} from 'cloudevents/transport/mqtt'.

Then we can expand protocols by adding another folder with a set of functions.

(imo accept(req) is a weird term which could be toCE(req))

Hope this is a useful perspective.

WRT the namespacing, Are we going to run into the issue where we have to have "dist" in the namespace path?

This was how we used to get Constants before we moved it to the top level:

const Constants = require('cloudevents/dist/constants').default;

maybe there is a way to get around that?

WRT namespacing, we can probably still import from cloudevents, not with src.

const {accept} = require('cloudevents').http;
# or maybe
const {fromHTTP} = require('cloudevents');

The main point I wanted to say is that we ideally don't overdo the protocols, and just have utility functions for getting things like HTTP headers, body, and forming CE to other transport primatives.

WRT the namespacing, Are we going to run into the issue where we have to have "dist" in the namespace path?

Good point. I suppose it should only be available via the top level export.

With respect to some of the ideas above, Node core and node modules, from what I know, it's less common to have classes like HTTP and HTTP.accept. It's more common to export functions import {accept} from 'cloudevents/transport/http'.

I wasn't thinking classes, exactly. Something more like this, where HTTP is just an object with a few functions hanging off of it:

const { CloudEvent, HTTP, Headers } = require('cloudevents');

// implements the Sender interface
function sender(headers: Headers, body: string) {
  return axios.post('https://receiver.example.com', {
    headers: headers,
    data: body
  });
}

HTTP.send(sender, HTTP.binary(ce));

I've been playing around with this on a branch, with interfaces like this:

// file: src/messages/index.ts

import { binary, structured, invoke, send } from './http'';

/**
 * Binding is an interface for transport protocols to implement,
 * which provides functions for sending CloudEvent Messages over
 * the wire.
 */
export interface Binding {
  binary: Serializer;
  structured: Serializer;
  send: Invoker;
  receive: Receiver;
}

/**
 * Message is an interface representing a CloudEvent as a
 * transport-agnostic message. 
 * see: https://github.com/cloudevents/spec/blob/v1.0/spec.md#message
 */
export interface Message {
  headers: Headers;
  body: string;
}

/**
 * Headers is an interface representing transport-agnostic metadata as
 * key/value string pairs. All transport protocols supported by CloudEvents
 * binding specifications have message metadata represented as key/value
 * string pairs except for NATS, which always uses structured mode.
 */
export interface Headers {
  [key: string]: string;
}

/**
 * Serializer is an interface for functions that can convert a
 * CloudEvent into a Message.
 */
interface Serializer {
  (event: CloudEvent): Message;
}

/**
 * Sender is a function interface for user-supplied functions
 * capable of transmitting a Message over a specific protocol
 */
export interface Sender {
  (headers: Headers, body: string): Promise<boolean>;
}

/**
 * Invoker is an interface for functions that send a message
 * over a specific protocol by invoking the user-supplied
 * Sender function with the message headers and body.
 */
export interface Invoker {
  (sender: Sender, message: Message): Promise<boolean>;
}

/**
 * Receiver is a function interface that converts an incoming
 * Message to a CloudEvent
 */
export interface Receiver {
  (message: Message, version: Version | undefined): CloudEvent;
}

// Export HTTP transport capabilities
export const HTTP: Binding = {
  binary: binary as Serializer,
  structured: structured as Serializer,
  send: invoke as Invoker,
  receive: receive as Receiver,
};

I will try and push a branch to this repo with a first pass at an implementation similar to this next week. We can iterate on it there since, it's often easier to actually talk about code that actually exists.

OK - I've pushed a branch with a first pass at something like this. https://github.com/cloudevents/sdk-javascript/tree/lance/ce-next

This folder contains the bulk of the work: https://github.com/cloudevents/sdk-javascript/tree/lance/ce-next/src/messages
And there's a single simple test: https://github.com/cloudevents/sdk-javascript/blob/lance/ce-next/test/integration/message_test.ts#L39

On this new branch, I've removed the concept of Sender entirely as it was really just a wrapper around invocation of a user supplied function. I don't think we need to have this level of indirection. It seems like overkill. The concept of Receiver has been changed to Deserializer and the exported function named toEvent(message: Message): CloudEvent.

I've changed the existing HTTP transport bits to use Message and HTTP.binary(), HTTP.structured() and HTTP.toEvent(). Have not added any deprecation notices yet. I think this is coming along. @cloudevents/sdk-javascript-maintainers please feel free to pull my fork and let me know what you think.

Regarding Deprecations. I wonder if we should have some sort of deprecation policy for the future. Is it to overkill to deprecate a feature and then need to wait for 2 major releases before it is removed?

So for example, we release a 3.2 which has deprecation notices for Receiver and Emitter, then in 4.0 we introduce the new way from the above post, but we don't remove the old way until 5.0?

Thats similar to how node core does things, but maybe we don't need to be that process heavy and a new major version is fine for removal. But we should probably do a minor release of the current version adding the deprecations in

@lholmquist

@lholmquist I think we can probably push 3.2.0 with deprecations for everything under src/transport, and the addition of src/messages, since it's only additive. This way, we can document and promote the usage of Message instead of the hackey transport bits, and even iterate on the new stuff, fixing any bugs that are found and potentially changing the API until 4.0.0 when we remove src/transport and src/messages becomes a fixed API with breaking changes requiring major version bumps. What do you think?

I'm thinking it's probably time to create a WIP pull request for detailed feedback if folks have it.

Navigation Menu

Repository navigation

Adding More Transport Protocols - A Proposal #322

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Adding More Transport Protocols - A Proposal #322

Description

Activity

lholmquist commented on Aug 20, 2020

lance commented on Aug 20, 2020

lholmquist commented on Aug 20, 2020

lholmquist commented on Aug 21, 2020

lholmquist commented on Aug 21, 2020

lance commented on Aug 21, 2020

lholmquist commented on Aug 21, 2020

grant commented on Aug 21, 2020

lholmquist commented on Aug 21, 2020

grant commented on Aug 21, 2020

lance commented on Aug 21, 2020

lance commented on Aug 21, 2020

lance commented on Aug 22, 2020

lholmquist commented on Aug 22, 2020

lance commented on Aug 22, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions