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

CloudEvents: relax requirement levels #2600

Closed
lmolkova opened this issue Jun 2, 2022 · 7 comments · Fixed by #2618
Closed

CloudEvents: relax requirement levels #2600

lmolkova opened this issue Jun 2, 2022 · 7 comments · Fixed by #2618
Assignees
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory

Comments

@lmolkova
Copy link
Contributor

lmolkova commented Jun 2, 2022

While CloudEvents spec requires most of the required attributes to be present on the event, not all of them may be required for telemetry purposes.

event_source + event_id would uniquely identify the event, so let's keep them required.

For others requirement level could potentially be relaxed:
- event_subject is optional per CloudEvents spec

  • event_spec_version is probably mostly static and rarely changed. Making it recommended or optional seem to be more reasonable
  • event_type: can it become recommended?

Recommended attributes would still be populated by default unless there are strong reasons not to. Instrumentation may provide a way to disable them explicitly.

@lmolkova lmolkova added the spec:trace Related to the specification/trace directory label Jun 2, 2022
@lmolkova
Copy link
Contributor Author

lmolkova commented Jun 2, 2022

@joaopgrassi what do you think? I can do the change if you're ok with it.

@joaopgrassi
Copy link
Member

joaopgrassi commented Jun 2, 2022

Hi @lmolkova !

event_source + event_id would uniquely identify the event, so let's keep them required.

👍

About the others:

  • event_subject is optional as you said, so no changes here
  • event_type as recommended: Sounds reasonable to me!
  • event_spec_version: I'm divided a bit on this one. It's probably static as you said, but if there's cross-systems communications and they use different versions, by using optional we might miss the info that could help in observing things? So maybe recommended is better? It's always present in the event so, should not have any performance penalty in adding/looking it up.

@Oberon00 Oberon00 added the area:semantic-conventions Related to semantic conventions label Jun 2, 2022
@lmolkova
Copy link
Contributor Author

lmolkova commented Jun 2, 2022

event_subject is optional as you said, so no changes here

ah, right! sorry for the confusion

event_spec_version

recommended sounds reasonable and I think we can suggest skipping if default (1.0)

@joaopgrassi
Copy link
Member

joaopgrassi commented Jun 3, 2022

suggest skipping if default (1.0`)

Not sure, do we really need to skip it? What is your concern in always having it? My rationale is it's a required attribute in the CloudEvents spec, so making it optinal/not always present here might be odd since it goes against what it's in their spec. I know we are not the same thing but.. maybe keeping it consistent is better? And if people really think it's unecessary, using recommended they can turn it off, right?

@lmolkova
Copy link
Contributor Author

lmolkova commented Jun 3, 2022

@joaopgrassi the only worry is redundancy: while only 1.0 is available, there is not much gain in populating it and lack of it can just mean default value. But I agree that recommended is good enough and no need to go further.

@joaopgrassi
Copy link
Member

joaopgrassi commented Jun 5, 2022

Sounds good then! Would you submit the PR? Or should I do it? Either is fine with me. Feel free to ping me for the approval if you open :)

@lmolkova
Copy link
Contributor Author

lmolkova commented Jun 6, 2022

@joaopgrassi Thanks! I'll send the PR, I want to merge #2594 first so I can use new requirement levels here properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants