Skip to content

Factor Distributed Tracing extension into new doc.#242

Merged
duglin merged 2 commits into
cloudevents:masterfrom
inlined:extensions-breakout
Jun 28, 2018
Merged

Factor Distributed Tracing extension into new doc.#242
duglin merged 2 commits into
cloudevents:masterfrom
inlined:extensions-breakout

Conversation

@inlined
Copy link
Copy Markdown
Contributor

@inlined inlined commented Jun 20, 2018

  • Extends extensions.md to clarify that RFC 2119 keywords
    are only for those who implement an extension.
  • Notes our resolved rules that each memory format defines
    extensions' placement and each transport binding defines
    a default placement which an extension may override.
  • Refactor the contents now in extensions/distributed-tracing.md
    as a first SWAG at a reusable template.

@inlined
Factor Distributed Tracing extension into new doc.
815c252 
* Extends extensions.md to clarify that RFC 2119 keywords
  are only for those who implement an extension.
* Notes our resolved rules that each memory format defines
  extensions' placement and each transport binding defines
  a default placement which an extension may override.
* Refactor the contents now in extensions/distributed-tracing.md
  as a first SWAG at a reusable template.

Signed-off-by: Thomas Bouldin <inlined@google.com>
duglin
duglin reviewed Jun 20, 2018
View reviewed changes
Comment thread extensions.md Outdated
representation when transported (e.g. tracing standards that require
specific headers).

## Known Extensions
Copy link
Copy Markdown
Collaborator

@duglin duglin Jun 20, 2018

Choose a reason for hiding this comment

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

For consistency with your other comment in my PR, can we make this "Documented"?

Copy link
Copy Markdown
Contributor Author

@inlined inlined Jun 20, 2018

Choose a reason for hiding this comment

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

Done

@duglin
Copy link
Copy Markdown
Collaborator

duglin commented Jun 20, 2018

One comment, but otherwise LGTM

@inlined
s/Known/Documented
344cb27 
Signed-off-by: Thomas Bouldin <inlined@google.com>
inlined added a commit to inlined/cloudevents that referenced this pull request Jun 20, 2018
@inlined
Add sampling extension.
93b212e 
This pull request supercedes cloudevents#76 and builds upon cloudevents#242.

Per agreement at the f2f meeting 2018-06-15, this will be ratified
as an extension but not currently as part of the core spec. The
early inclusion of the spec and the ability for side-cars to impose
sampling early in the pipeline make it reasonable to start with
this as an extension.

I know the original author hoped to make this a core feature, though
there are two upsides:
1. Due to other decisions on 2018-06-15, this will have zero impact
   on JSON encoding (but will in Proto and may in SDKs)
2. Extensions now get a whole document. This lets us pitch sampling
   in more detail.

Necessary additional changes:
* Added `Integer` to our type system
* Added convention for how scalar extensions should be documented.
  I'm not really excited about this proposal so others are welcome.

Special thanks to @maplebed for kicking this effort off.

Signed-off-by: Thomas Bouldin <inlined@google.com>
@inlined inlined mentioned this pull request Jun 20, 2018
Merged
@duglin
Copy link
Copy Markdown
Collaborator

duglin commented Jun 28, 2018

Approved on the 6/28 call

@duglin duglin merged commit 669e7c5 into cloudevents:master Jun 28, 2018
inlined added a commit to inlined/cloudevents that referenced this pull request Jul 25, 2018
@inlined
Add sampling extension.
8e74614 
This pull request supercedes cloudevents#76 and builds upon cloudevents#242.

Per agreement at the f2f meeting 2018-06-15, this will be ratified
as an extension but not currently as part of the core spec. The
early inclusion of the spec and the ability for side-cars to impose
sampling early in the pipeline make it reasonable to start with
this as an extension.

I know the original author hoped to make this a core feature, though
there are two upsides:
1. Due to other decisions on 2018-06-15, this will have zero impact
   on JSON encoding (but will in Proto and may in SDKs)
2. Extensions now get a whole document. This lets us pitch sampling
   in more detail.

Necessary additional changes:
* Added `Integer` to our type system
* Added convention for how scalar extensions should be documented.
  I'm not really excited about this proposal so others are welcome.

Special thanks to @maplebed for kicking this effort off.

Signed-off-by: Thomas Bouldin <inlined@google.com>
duglin pushed a commit that referenced this pull request Aug 16, 2018
@inlined
Add sampling extension. (#243)
3248e31 
* Add sampling extension.

This pull request supercedes #76 and builds upon #242.

Per agreement at the f2f meeting 2018-06-15, this will be ratified
as an extension but not currently as part of the core spec. The
early inclusion of the spec and the ability for side-cars to impose
sampling early in the pipeline make it reasonable to start with
this as an extension.

I know the original author hoped to make this a core feature, though
there are two upsides:
1. Due to other decisions on 2018-06-15, this will have zero impact
   on JSON encoding (but will in Proto and may in SDKs)
2. Extensions now get a whole document. This lets us pitch sampling
   in more detail.

Necessary additional changes:
* Added `Integer` to our type system
* Added convention for how scalar extensions should be documented.
  I'm not really excited about this proposal so others are welcome.

Special thanks to @maplebed for kicking this effort off.

Signed-off-by: Thomas Bouldin <inlined@google.com>

* PR feedback; also add conttent to extensions.md

Signed-off-by: Thomas Bouldin <inlined@google.com>

* @douglin doesn't like the world 'may' 😉

Signed-off-by: Thomas Bouldin <inlined@google.com>

* Add 'e.g.' to list of scalar types in conventions

Signed-off-by: Thomas Bouldin <inlined@google.com>
zpencer pushed a commit to zpencer/spec that referenced this pull request Aug 22, 2018
@inlined @zpencer
Add sampling extension. (cloudevents#243)
6660edd 
* Add sampling extension.

This pull request supercedes cloudevents#76 and builds upon cloudevents#242.

Per agreement at the f2f meeting 2018-06-15, this will be ratified
as an extension but not currently as part of the core spec. The
early inclusion of the spec and the ability for side-cars to impose
sampling early in the pipeline make it reasonable to start with
this as an extension.

I know the original author hoped to make this a core feature, though
there are two upsides:
1. Due to other decisions on 2018-06-15, this will have zero impact
   on JSON encoding (but will in Proto and may in SDKs)
2. Extensions now get a whole document. This lets us pitch sampling
   in more detail.

Necessary additional changes:
* Added `Integer` to our type system
* Added convention for how scalar extensions should be documented.
  I'm not really excited about this proposal so others are welcome.

Special thanks to @maplebed for kicking this effort off.

Signed-off-by: Thomas Bouldin <inlined@google.com>

* PR feedback; also add conttent to extensions.md

Signed-off-by: Thomas Bouldin <inlined@google.com>

* @douglin doesn't like the world 'may' 😉

Signed-off-by: Thomas Bouldin <inlined@google.com>

* Add 'e.g.' to list of scalar types in conventions

Signed-off-by: Thomas Bouldin <inlined@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@duglin duglin duglin left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@inlined @duglin