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

Incident bucket #107

Merged
merged 10 commits into from
Mar 9, 2023
Merged

Incident bucket #107

merged 10 commits into from
Mar 9, 2023

Conversation

afrittoli
Copy link
Contributor

@afrittoli afrittoli commented Jan 24, 2023

Changes

Introduce incident events.
Reference design: https://hackmd.io/h8DBfgwLQuKR7JyUPy0Xow?both

Partially-fixes: #59

Signed-off-by: Andrea Frittoli andrea.frittoli@gmail.com

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

@afrittoli
Copy link
Contributor Author

Reference hackmd: https://hackmd.io/h8DBfgwLQuKR7JyUPy0Xow

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Introduce incident events.

Partially-fixes: cdevents#59

Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
schemas/incidentreported.json Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
continuous-operations.md Outdated Show resolved Hide resolved
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
@e-backmark-ericsson
Copy link
Contributor

About the naming of the new bucket. I believe it should be without an 's' in the end. So "Continuous Operation" instead of "Continuous Operations"

schemas/incidentdetected.json Show resolved Hide resolved
schemas/incidentdetected.json Show resolved Hide resolved
"source": {
"type": "string",
"minLength": 1,
"format": "uri-reference"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comments for all: Shouldn't this be uri instead of uri-reference? Main concern is with interoperability, the reader must then know intricate knowledge of the sender e.g. which cluster is it deployed in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how it is defined in the spec https://github.com/cdevents/spec/blob/v0.1.1/spec.md#source-context so not anything defined in this PR.

The initial reason for that is that source is a URI-reference in CloudEvents. I don't think we can enforce a URI here really since not all event sources will have a URI associated, but we could do a better job at describe the format of URI-references - I'm tracking this in #29

Copy link
Contributor

Choose a reason for hiding this comment

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

The main reason for my objection about the uri-reference was that it allows relative urls. Basically you can have a system with two sources having the same source as they differ on the host which is not part of the source field.

Regarding this PR: These are the only events that have uri-reference in the schemas. I would suggest to remove it here an do a PR to add it in all the events. Main motivation for this is to keep the protocol consistent, why have do we have one event with this but not all.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say this PR should align with how the existing schemas look, and aligning the usage or uri/uri-reference should be done in #114

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 updated this PR - alternatively we could merge #116 first, and bring back the URI-reference format here.

examples/incidentdetected.json Show resolved Hide resolved
schemas/incidentreported.json Outdated Show resolved Hide resolved
schemas/incidentreported.json Outdated Show resolved Hide resolved
schemas/incidentreported.json Outdated Show resolved Hide resolved
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
README.md Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
spec.md Show resolved Hide resolved
schemas/incidentdetected.json Show resolved Hide resolved
schemas/incidentdetected.json Show resolved Hide resolved
tools/event-version.sh Outdated Show resolved Hide resolved
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
@afrittoli
Copy link
Contributor Author

About the naming of the new bucket. I believe it should be without an 's' in the end. So "Continuous Operation" instead of "Continuous Operations"

Could you elaborate on why we should remove the "s"? As far as I can see from searching the internet, the practice is commonly referred to as "continuous operations" with the "s". One example https://www.gartner.com/en/information-technology/glossary/continuous-operations

Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
@e-backmark-ericsson
Copy link
Contributor

About the naming of the new bucket. I believe it should be without an 's' in the end. So "Continuous Operation" instead of "Continuous Operations"

Could you elaborate on why we should remove the "s"? As far as I can see from searching the internet, the practice is commonly referred to as "continuous operations" with the "s". One example https://www.gartner.com/en/information-technology/glossary/continuous-operations

Well, I actually mostly use the term "continuous operations" myself, but when I google on "continuous operations" and compare it to "continuous operation", the singular version is much more common it seems. And our other buckets are named in singular.

We still have the possibility to change things like this before we reach 1.0, but it will probably hurt a bit if we do so I'd like us to make conscious decisions on names like these.

Let's bring it to our workgroup meeting later today and set it there.

Copy link
Contributor

@e-backmark-ericsson e-backmark-ericsson left a comment

Choose a reason for hiding this comment

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

Decided on workgroup meeting on March 7th to keep the 's'

README.md Show resolved Hide resolved
continuous-operations.md Show resolved Hide resolved
spec.md Show resolved Hide resolved
@afrittoli
Copy link
Contributor Author

Merging as discussed in the working group.

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.

Define new incident events for the DORA metrics
4 participants