-
Notifications
You must be signed in to change notification settings - Fork 589
Propose holistic simplification of source- attributes #129
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
Conversation
@inlined Obviously since it your PR you're free to keep it as-is, but it seems to me that it might be worth splitting the event-type discussion from the source* discussion since it would probably be easier to reason about them separately - it would be a more focused discussion. And, from your write-up it does look like each one could be adopted independent of the other (ie. I don't see a direct link between the two write-ups). To the specific proposal:
|
@duglin you make a lot of good points. RE:
|
* Drops source-type; there is no clear use according to the usage scenarios. * Consolidates namespace and the source-authority * Clarifies that source-path should not be redundant with source-authority/namespace * Adds namespacing to event-type * Drops documentation for "source" that was redundant with each of its subfields. Opts for a name prefix instead. Signed-off-by: Thomas Bouldin <inlined@google.com>
0a14964
to
c952748
Compare
(rebase squashed to kick Travis and get it to correctly mark this PR as clean) |
@duglin, since these are attributes that work together, it makes sense to me to have a conversation about them together. |
Signed-off-by: Thomas Bouldin <inlined@google.com>
FYI: Renamed |
* Examples: | ||
* customer.created | ||
* com.github.pull.create |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpicking: com.github.pull.create
reads as a command rather than an event that took place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: github docs use "create" for a notification that something has been created... not that we need to conform to some specific vendor. We could include a few divergent examples to be clear that we're not mandating anything or consider some guidance about naming if someone wants to see if there's a trend in existing implementations or other spec we want to borrow from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree this is a nit, and suggest it could be addressed in a follow-on PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a consumer of an event, "event-type", to me, denotes whether it is a storage event or an messaging event or a timer event , etc. Then depending on the type of event, it could have different properties. For example, a storage event could have properties like "bucket name", "action performed on that bucket (created or update or delete)", a messaging event could have properties like "topic of the message".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SeanFeldman I lean 20% more towards infinitive tense for a few reasons:
- It's nice that these names can be reused for other policies in the source software. E.g. only some developers will have "com.github.pull.create" permission in a repository.
- There seems to be precedent today for events to be named in the infinitive. E.g.
onClick
rather thanonClicked
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First scenario is describing a permision, not an event. Permission to create a PR.
Second example is historical OnClick
as an event handler for the click
event. You'd not call it "WhenClick", but rather "WhenClicked" if that would be the convention back in the day.
I might remain in the minority with my belief that an event is a manifistation of something immutable that happened and therefore should be described in the past tense 🙂
spec.md
Outdated
* Constraints: | ||
* OPTIONAL | ||
* Keys MUST match the regular expression `[_.a-z0-9]+` | ||
* Keys SHOULD use the character "." is as namespace separator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave out "is"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops. This is what happens when I copy+paste from other standards.
Signed-off-by: Thomas Bouldin <inlined@google.com>
Can I get a 👍 or 👎 on this comment for whether |
Can I get a 🎉 to suggest that the source URI should be more strictly clarified to include an authority and a ❤️ to suggest that URI should be split to enforce that it includes an authority. |
Please vote and explain why in the next 24hrs. I'll break this up into smaller PRs then. As I'll explain in the labels PR, I didn't leave a vote for labels as the source URI query string. There are many intuitive cases where the source should include literally a URI that an end-user has interacted with. These query fragments are application-level whereas labels are typically infrastructure-level. They have different meanings and are frankly considered with different levels of security (I trust the labels that come from my deployment. I verify the query fragments that come from my users) |
@inlined if you make it just |
@inlined i think we need a single namespaced URI for source, also seems like its confusing the event details with the source e.g. the object IMO the event can be a notification from the blob service with |
Agree with @yaronha about One remark - I personally refer to events in the past tense. It's something that has happened and not an instruction to take place. As such, should be also in the past tense. E.g.: |
@inlined will try to have a response/vote by EOD tomorrow - just slightly past your 24hr mark :-) but it takes time to try to gather the view-points. |
@deissnerk @yaronha in this context everything is an attribute of the event. The source is an event detail. My understanding of our work on describing shared meta-data is to figure out some details of the events that are commonly useful to include to help with routing, aggregation and to simply the work of a consumer. The Does that help clarify? or am I missing something? |
* myorg/myrepo | ||
* my/long/ftp/path.jpg | ||
|
||
### source-labels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do they refer to the "properties" of the source?
If so, I feel "source-properties" is a better name since property feels something intrinsic while "label" feels "add-on" .
Is it TRUE that different event source types could have different source-labels? Are we going to leave the definition of source-labels to event producers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I proposed source-labels
because that's the use case I had in mind. I'm leaving myself open to being convinced that other systems would be able to add their own labels as well. If we allow this, we should have a discussion about whether they are explicitly annotating the source (letting us keep it source-labels
) or if they're adding other annotations that shouldn't be tied to the source.
The labels that I'm used to interacting with are under the developer's control, though the developer's software might automatically manage some labels with the developer's credentials. Some examples I've used in the past:
- Google was rolling out some new infrastructure near a major conference. They told speakers to add a specific label on our project if we wanted to defer migration. Because projects (like many resources on Google Cloud) support labels, the team was able to skip over projects that would be used for on-stage demos without any coordination from any other team or a new permanent attribute on their API like "defer_2017_rollout": true
- Firebase releases its own simplified tooling on top of Google Cloud Functions, which provides our customers with a golden road. We want to track when customers use this tooling, so we add a label that says our CLI managed the deploy. Since our CLI experience is K8S style (the customer gives us source and we make the project look like that source), we use this label to avoid deleting functions that are being managed with other tools.
You seem to know dramatically more about IoT and can interpret how the label concept would apply there. It seems totally reasonable to me that the user portal for managing devices would manage labels on those devices. It's also true that there could be a bit of disconnect originally from vendors that they use different label names to refer to similar concepts. Personally, I think this problem will be smaller in practice than deciding that every IoT device can only be correlated by one property.
@ultrasaurus i dont think we are aligned
Our goal is to make sure events get out of a system and arrives to a different system and can be understood by the receiver. i dont see why it make sense to add so much data from within the event message and place it in the context (label/envelop).
Take AWS example (i placed code in Slack), SNS fires an event in which it is the source (no info about the object), in the SNS payload there is an S3 service message where the S3 service is the source, and in that there are a bunch of records which describe the modified objects. such a model make sense to me, right we may choose to make SNS invisible, but still, the S3 service is the event issuer and not the object name.
|
It seems like we are still not in sync as to the semantics of "Event Source". From an event consumer point of view, what I care is the detail information about the event and event source, for example, is it a storage event or a messaging event or a streaming event or a timer event? if it is the storage event, is the event triggered by a bucket creation or modification or deletion? what is the bucket name? If it the the messaging event, what is the topic of the message, who is the user that triggers this message etc. ? If we can give self-explanatory names to these attributes, that is great. Otherwise, we may need more clarification on what each attribute name means. As to the name of the "source-label", I think having the "source" keyword there makes it clear that the labels are associated with the "source". But as stated in my inline comment, I think a better name is "source property". |
@cathyhongzhang why would you expect to have the event details as context fields ? The cloud-event context fields are NOT replacing the event body fields, they are merely the fields which allow us to understand the type and organisation of the event and few generic attributed for observers to watch (ID, Time, Source), every field which is event specific (e.g. Bucket name) is not a generic attribute by definition. If we standard Event schema for common events like IoT sensor reports, Blob service updates, etc. those will be in a separate scope of work. |
@inlined sorry for the delay but with vacations/holidays I didn't get as quick feedback as I had hoped. From our side we're leaning towards a model where the source is a URI but whether or not there's an authority in there would be an implementation choice. |
This commit has been spliced off of cloudevents#129 Signed-off-by: Thomas Bouldin <inlined@google.com>
@inlined should we close this one since you've opened up new PRs that appear to cover at least some of these same aspects? |
This commit has been spliced off of cloudevents#129 Signed-off-by: Thomas Bouldin <inlined@google.com>
Will close. Was waiting because the last part, URIs with authorities, was not yet spun off into a sub-PR. WDYT about adding a SHOULD clause that the URI should include an authority component? When setting up the infrastructure to subscribe for events, we'll want more than a relative URI to know what service to wire up. |
@inlined saying "a 'source' SHOULD have an authority component to the URI" seems reasonable to me. |
This commit has been spliced off of cloudevents#129 Signed-off-by: Thomas Bouldin <inlined@google.com>
* Add namespacing to an event. This commit has been spliced off of #129 Signed-off-by: Thomas Bouldin <inlined@google.com> * Update serialization.md to namespace event-type. Noticed that "namespace" was still in the searialization docs, even though it was removed from spec.md. Fixed. Signed-off-by: Thomas Bouldin <inlined@google.com> * Soften stance on namespacing to SHOULD. Accept wording suggestions. This closes issue #32 Signed-off-by: Thomas Bouldin <inlined@google.com> * Clarify the meaning of the event-type package namespace. It is explicitly OK for one software organization to emit event-types of another organizatoin, so long as they are conforming to the standard set by the organization in the namespace. Signed-off-by: Thomas Bouldin <inlined@google.com>
* Add namespacing to an event. This commit has been spliced off of cloudevents#129 Signed-off-by: Thomas Bouldin <inlined@google.com> * Update serialization.md to namespace event-type. Noticed that "namespace" was still in the searialization docs, even though it was removed from spec.md. Fixed. Signed-off-by: Thomas Bouldin <inlined@google.com> * Soften stance on namespacing to SHOULD. Accept wording suggestions. This closes issue cloudevents#32 Signed-off-by: Thomas Bouldin <inlined@google.com> * Clarify the meaning of the event-type package namespace. It is explicitly OK for one software organization to emit event-types of another organizatoin, so long as they are conforming to the standard set by the organization in the namespace. Signed-off-by: Thomas Bouldin <inlined@google.com>
Overview
This PR drops source-types, applies namespacing to event-type and source, and clarifies that source path need not include the source-authority.
My goal is to build on Clemens' Usage Scenarios and revisit our attributes in related groups. This PR looks at event-type, namespace, and source, which are attributes that describe common metadata for the individual occurrence. I would like to cover the fields related to parsing data (event-type-version, schema-url, content-type) in a future PR.
Changes
scenarios.
source-authority/namespace
of its subfields. Opts for a name prefix instead.
Usage Scenarios
IoT Case
This case demonstrates an IoT vendor who may use UUIDs for the sensors they sell and use non-hierarchical attributes for filtering/routing events in an event-connected system. Note that the IoT device's source may not be addressable from the Action which receives the event data.
Hosted service
This use case focuses on a developer who extends their application by receiving events from a managed cloud service.
Deployed software
This case demonstrates the value of splitting the concept of "namespace" to both the event-type and the source. Here "bigcompany.com" has deployed GitHub enterprise inside their corporate network. The event-type is still associated with GitHub (the software author) and the source is associated with "bigcompany.com". This example shows a pull request creation event for the "project88/backend" repo.
I'd really like feedback related to:
source-labels
? If so, what is a reasonable upper bound?source-labels
the same as an emptysource-labels
?