Skip to content

Technical motivation for keeping extension attributes minimal#287

Merged
duglin merged 2 commits into
cloudevents:masterfrom
cneijenhuis:limit-metadata-primer
Aug 31, 2018
Merged

Technical motivation for keeping extension attributes minimal#287
duglin merged 2 commits into
cloudevents:masterfrom
cneijenhuis:limit-metadata-primer

Conversation

@cneijenhuis
Copy link
Copy Markdown
Contributor

This paragraph should provide technical motivation for keeping the usage of extension attributes in check. For background information, see #286

I expect that the following paragraph from #277 will precede the paragraph I'm proposing, which provides a more conceptual motivation for limiting the usage:

Extension attributes to the CloudEvent specification are meant to
be additional metadata that needs to be included to help ensure proper
routing and processing of the CloudEvent. Additional metadata for other
purposes, that is related to the event itself and not needed in the
transportation of the CloudEvent, should instead be placed within the proper
extensibility points of the event itself.

I do think that at a later point in time we can still have a discussion on whether we want to set actual limits (e.g. a CloudEvent MUST NOT have more than #kb of metadata) or guarantees (e.g. a CloudEvent consumer MUST at least accept events with #kb of metadata) in spec.md.

However, I believe this change in the primer would not be affected much by that discussion.

Signed-off-by: Christoph Neijenhuis christoph.neijenhuis@commercetools.de

@cneijenhuis cneijenhuis force-pushed the limit-metadata-primer branch 2 times, most recently from 453b3d5 to 819f294 Compare August 9, 2018 15:50
@duglin
Copy link
Copy Markdown
Collaborator

duglin commented Aug 16, 2018

Seems like reasonable guidance to me.
LGTM

Comment thread primer.md
information will be added to the [Prior Art](#prior-art) section of this
document.

Extension attributes should be kept minimal to ensure the CloudEvent can be
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Propose small clarifying change:

Extension attributes required by a single CloudEvent should be kept minimal

e.g. It's ok if the community as a whole as lots of extensions for different domains, but in practice few would be needed for an individual event than needs to be transported on the wire

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:
Event producers should consider the technical limitations that might be encountered when adding extensions to a CloudEvent. For example, the HTTP Binary Mode uses HTTP headers to transport metadata; most HTTP servers will reject requests with excessive HTTP header data, with limits as low as 8kb. Therefore, the aggregate size and number of extension attributes should be kept minimal.

@duglin
Copy link
Copy Markdown
Collaborator

duglin commented Aug 16, 2018

rebase needed

@cneijenhuis cneijenhuis force-pushed the limit-metadata-primer branch from 819f294 to 2df7686 Compare August 17, 2018 14:47
@cneijenhuis
Copy link
Copy Markdown
Contributor Author

I'm sorry that I couldn't attend yesterdays call. I watched the recording, at least.

@ultrasaurus You're right, the first sentence was obviously confusing! Does the version that @duglin proposed (which I've adopted, thanks!) sound good to you?

@inlined I believe it was you who said something along the lines of "This PR says: Extensions are bad"? I'd like to clarify that this isn't the intention of my PR. In fact, I believe extensions are good and I'd like that, eventually, CloudEvents middlewares can route based on (custom) extensions.

However, I have two concerns:

  • Interoperability: If someone puts a lot of data into extensions, and tests it with e.g. JSON, everything seems fine. But if later someone else tries to transmit that CloudEvent with a different serialization format, it brakes.
  • Middleware: Parsing a huge set of extension data will impact performance. But a middleware is trying to achieve high-throughput and a predictable performance. If putting too much data into custom extensions is the norm, then middlewares may decide to not offer features like routing based on them, but effectively treat them like data.

I agree that, similar to a leaflet of a drug, it may sound like "Extensions are bad" at first, but the intention is to make sure they are used without hindering interoperability.

@duglin
Copy link
Copy Markdown
Collaborator

duglin commented Aug 20, 2018

LGTM

@duglin
Copy link
Copy Markdown
Collaborator

duglin commented Aug 30, 2018

Approved on the 8/30 call.

@cneijenhuis can you rebase this? If not, I can give it a try

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>
@cneijenhuis cneijenhuis force-pushed the limit-metadata-primer branch 2 times, most recently from a2689f0 to 81e7a33 Compare August 31, 2018 07:47
Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>
@cneijenhuis cneijenhuis force-pushed the limit-metadata-primer branch from 81e7a33 to c120601 Compare August 31, 2018 07:48
@cneijenhuis
Copy link
Copy Markdown
Contributor Author

@duglin Yes, it is rebased now.

I'd like to make further contributions - I can get started on #265, but I am also happy to pick another issue.

@duglin
Copy link
Copy Markdown
Collaborator

duglin commented Aug 31, 2018

@cneijenhuis thanks for the rebase. Merging....

re: #265 sounds good to me. Please ping @clemensv as he originally wrote the PR that introduced the "-" usage and might have been thinking about possible solutions already.

@duglin duglin merged commit 803db22 into cloudevents:master Aug 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants