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

Add syntax details for attributes in CDEvents #30

Merged
merged 9 commits into from
Mar 4, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 120 additions & 6 deletions spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,131 @@ predicate are defined in the [vocabulary](#vocabulary).

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a mix of markup used in this PR, when it comes to referencing attributes and sections in the document. Some references are encapsulated within back-quotes and some are not. Some references render as active links and some do not. Please streamline that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I've addressed this now, let me know if that looks good

Copy link
Contributor

Choose a reason for hiding this comment

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

The vocabulary section is in some places referenced with back-quotes and sometimes without back-quotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed that

## Context

### REQUIRED Event Attributes

The following attributes are REQUIRED to be present in all the Events defined in
this vocabulary:
the [vocabulary](#vocabulary):

#### id

- Type: [`String`](https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/spec.md#type-system)
- Description: Identifier for an event.
Subsequent delivery attempts of the same event MAY share the same
[`id`](#id). This attribute matches the syntax and semantics of the
[`id`](https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/spec.md#id)
attribute of CloudEvents.

- __Event ID__: defines an identifier for the event. The Event ID MUST be unique
within the given Event Source.
- __Event Type__: defines the type of event, as combination of a *subject* and
- Constraints:
- REQUIRED
- MUST be a non-empty string
- MUST be unique within the given [`source`](#source) (in the scope of the producer)
- Examples:
- A [UUID version 4](https://en.wikipedia.org/wiki/Universally_unique_identifier#Version_4_(random))

#### type
e-backmark-ericsson marked this conversation as resolved.
Show resolved Hide resolved

- Type: [`String`](https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/spec.md#type-system)
- Description: defines the type of event, as combination of a *subject* and
*predicate*. Valid event types are defined in the [vocabulary](#vocabulary).
All event types should be prefixed with `dev.cdevents.`. One occurrence may
have multiple events associated, as long as they have different event types
- __Event Source__: defines the context in which an event happened
- __Event Timestamp__: defines the time when the event was produced

- Constraints:
- REQUIRED
- MUST be defined in the [vocabulary](#vocabulary)
- Examples:
- `dev.cdevents.taskrun.started`
- `dev.cdevents.environment.created`
- `dev.cdevents.<subject>.<predicate>`

#### source

- Type: [`URI-Reference`](https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/spec.md#type-system)
- Description: defines the context in which an event happened. The main purpose
of the source is to provide global uniqueness for [`source`](#source) +
[`id`](#id).

The source MAY identify a single producer or a group of producer that belong
to the same application.

When selecting the format for the source, it may be useful to think about how
clients may use it. Using the [root use cases](./primer.md#use-cases) as
reference:

- A client may want to react only to events sent by a specific service, like
the instance of Tekton that runs in a specific cluster or the instance of
Jenkins managed by team X
- A client may want to collate all events coming from a specific source for
monitoring, observability or visualization purposes

- Constraints:
- REQUIRED
- MUST be a non-empty URI-reference
- An absolute URI is RECOMMENDED

- Examples:

- If there is a single "context" (cloud, cluster or platform of some kind)
- `/tekton`
- `https://www.jenkins.io/`

- If there are multiple "contexts":
- `/cloud1/spinnaker-A`
- `/cluster2/keptn-A`
- `/teamX/knative-1`

#### timestamp

- Type: [timestamp](https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/spec.md#type-system)
- Description: defines the time of the occurrence. When the time of the
occurrence is not available, the time when the event was produced MAY be used.

In case the transport layer should require a re-transmission of the event,
the timestamp SHOULD NOT be updated, i.e. it should be the same for the same
[`source`](#source) + [`id`](#id) combination.

- Constraints:
- REQUIRED
- MUST adhere to the format specified in [RFC 3339](https://datatracker.ietf.org/doc/html/rfc3339)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to mention this, in case someone wonders:
In Eiffel we define the timestamp using UNIX Epoch time (in milliseconds), and the main reason to that is probably as that makes it easier for a machine to calculate the diff between two events, and since the Eiffel events are not primarily intended for human consumption. I guess the benefit of having human readable timestamps should overrule that though, for CDEvents, since it makes it a lot easier for a human to troubleshoot the events.


#### version
m-linner-ericsson marked this conversation as resolved.
Show resolved Hide resolved

- Type: `String`
- Description: The version of the CDEvents specification which the event
e-backmark-ericsson marked this conversation as resolved.
Show resolved Hide resolved
uses. This enables the interpretation of the context. Compliant event
producers MUST use a value of `draft` when referring to this version of the
specification.

- Constraints:
- REQUIRED
- MUST be a non-empty string
Copy link
Contributor

Choose a reason for hiding this comment

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

When we have it some versions available, should we link to list of valid versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each version of the spec will refer to its own version only, link in L156.
Once we have more than one version, the docs website will include a dropdown with list of available versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Line 156 in this document?

Was your idea to have state the version as the only valid value?

My idea with a list was for to ease for the reader finding the valid range of version available

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The list of valid version will be listed on top of the documentation, so it should be easy to discover.

The main reason for me is that if I'm browsing a specific version of the spec, I should only use that version in my events. Also I would not like to keep cross-version information in version specific pages.

I'd be happy to include the list of versions in the main README as long as we keep it independent of the versioned docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

So your idea was to put the spec version as the only valid field?

Maybe add something like

MUST be the version of the event/spec used for producing the event


#### subject
m-linner-ericsson marked this conversation as resolved.
Show resolved Hide resolved

- Type: `Object`
- Description: This provides all the relevant details of the [`subject`](#subject). The
format of the [`subject`](#subject-1) depends on the event [`type`](#type).

The attributes available for each subject are defined in the
[vocabulary](#vocabulary). The REQUIRED and OPTIONAL attributes depend on
the event [`type`](#type) and are specified in the [vocabulary](#vocabulary).

- Constraints:
- REQUIRED
- If present, MUST comply with the spec from the [vocabulary](#vocabulary).

- Example:
- Considering the event type `dev.cdevents.taskrun.started`, an example of
subject, serialised as JSON, is:

```json
Copy link
Contributor

Choose a reason for hiding this comment

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

So in Eiffel we have our two events:

  • Activity Triggered giving the activity a name, including the possibility to give a category, and also describe how the activity was triggered
  • Activity Started linking back to the Activity Triggered but containing the execution URI and links to logs to find more information.

In your example I guess the id would be the same as name in our Activity Triggered event. The URL would be the executionUri Activity Started event. I am trying to figure out how the task would map. What would you use it for? The closest thing I can think of is the categories filed in the Activity Triggered event. Would you say the task would be used to group events together?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ID usually represents the unique identifier of the subject within the event source.
I think all subjects we define will have an ID, but this is defined in the vocabulary really.

Subjects will have different attributes, task is only valid for taskrun for instance.
The task is an identifier of the definition that is executed in the taskrun.
Here I'm just including what we have in the vocabulary today.

In terms "triggered" vs "started" - in CDEvents, they would be predicates and - at least for now - they vary event by event. There may be an opportunity to reduce the number of predicates and make them standard across events, but we'll have to see if it works well or not.

The separation of subjects in "long running" and "run to completion" could help create a standard set of predicates, one for each group... but it may turn out that it's not actually possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

In Eiffel as we have a triggered event and then link back to it so we don't need to worry about the uniqueness of an ID. So from my view we are talking about:

  • ID: unique identifier
  • task: A human friendly display name for the task instance
  • URL: the link to the task instance

If I understand correctly it would be more logical if task was Run of my-task #123

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The data model for PipelineRun and TaskRun was inspired by Tekton - here is what we have today: https://cdevents.dev/docs/core/

In Tekton, Task and Pipeline are the static definition of what a Task and a Pipeline should do.
TaskRun and PipelineRun are executions of a Task and a Pipeline respectively.

In CDEvents we only have the object TaskRun and PipelineRun and they include a few bits of data, including the name of the Task / Pipeline. The URL is an optional field that points to the execution - it could be for instance a deep-link URL into the Tekton dashboard to show the specific PipelineRun.

That said, I just wanted to provide an example based on the existing content of the vocabulary.
I didn't really mean to get into any vocabulary specific discussion for this PR 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

I think examples are vital for understanding as the subject object is a new concept and it wasn't that easy to follow your train of thoughts. If you want to keep the example here we could go back and change it later. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally agree that examples are vital! I'll try to make more of them in the next PRs and align these accordingly.

"taskrun" : {
"id": "my-taskrun-123",
m-linner-ericsson marked this conversation as resolved.
Show resolved Hide resolved
"task": "my-task",
m-linner-ericsson marked this conversation as resolved.
Show resolved Hide resolved
"status": "Running",
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we don't need to include this as the event type predicate already tells us this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly, or it could provide more insight. Again, this matches what we have in the vocabulary today - if we change the vocabulary we need to update these examples accordingly, but for this PR I would not go and touch the vocabulary part

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then you need to help me. Were do we state in the vocabulary that the should have the status here?

Copy link
Contributor

@m-linner-ericsson m-linner-ericsson Mar 2, 2022

Choose a reason for hiding this comment

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

I would not include the status field in subject object but only use it to define the subject itself as I guess it will be the same subject object in all events of the same subject disregarding the predicate. For example both taskrun.started and taskrun.finished would have the same subject object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, then you need to help me. Were do we state in the vocabulary that the should have the status here?

It's in here https://cdevents.dev/docs/core/ but it's only defined for PipelineRun, not for TaskRun. I didn't realise that, it seems to be incomplete. I can use a PipelineRun instead of a TaskRun in the example if you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would not include the status field in subject object but only use it to define the subject itself as I guess it will be the same subject object in all events of the same subject disregarding the predicate. For example both taskrun.started and taskrun.finished would have the same subject object.

Can we discuss the objects in the vocabulary in the next PR once I update those?
If we change the structure or add/remove fields, I will update this example accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, so you want to include all the pipeline attributes in the subject object? From your comment above I got it that you didn't want to. I would still vote to not include it but if you want to proceed with the PR you can keep it 😄

"URL": "/apis/tekton.dev/v1beta1/namespaces/default/taskruns/my-taskrun-123"
}
```

## Vocabulary

Expand Down