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 team API #5

Merged
merged 3 commits into from
Jun 18, 2015
Merged

Add team API #5

merged 3 commits into from
Jun 18, 2015

Conversation

dominikschulz
Copy link
Collaborator

This commit adds an team API to manage teams (ordered lists of people).

This does not yet include any kind of notification or escalation
handling. I'm willing to provide an implementation for my proposal from #2 in an follow-up PR.

This commit adds an team API to manage teams (ordered lists of people).

This does not yet include any kind of notification or escalation
handling.
@chrissnell
Copy link
Owner

Thanks, Dominik! The goal of my work is to create an open source alternative to PagerDuty with some additional features (like "page this specific person"). I agree with keeping things simple but I think having support for on-call rotations is pretty critical and I don't want to force people to re-order their teams every week when shifts change.

I was just looking at how VictorOps has it structured and it's insanely complex. Each team has one or more rotations, each of which can have one more shifts, which may or may not overlap, and each shift can have one or more people. Way more than most folks need.

A simpler solution for Chicken Little might look something like this:

type Team struct {
   Name string
   Description string
   Rotation RotationPolicy
   EscalationSteps []EscalationStep
   Members []Person
}

type RotationPolicy struct {
   Desceription string
   RotationFrequency time.Duration  // How often we rotate
   RotationTime time.Time  //  The start time of the very first shift
}

type EscalationMethod int

const (
   NotifyOnDuty EscalationMethod = iota // 0
   NotifyNextInRotation                 // 1
   NotifyOtherPerson                    // 2
   CallWebhook                          // 3
   SendEmail                            // 4
)

// Defines how alerts are escalated when a team member doesn't respond
type EscalationStep struct {
  TimeBeforeEscalation time.Duration
  Method EscalationMethod
  NotifyOtherPerson Person   // Only set if Method == NotifyOtherPerson
  Webhook string // Only set if Method == CallWebhook
  Email string // Only set if Method == SendEmail
}

Thoughts?

@dominikschulz
Copy link
Collaborator Author

Great! I'd also like to see an open-source alternative to PD. And I'd like to help with this as good as I can.

We seem to agree that a Team needs a Name, Description and a list of Members.

The RotationPolicy is for automated changes to the notification order, right? This may be a little bit too simple. What if there is the need for a manual change in shifts? Someone getting sick or something like that? Wouldn't the fixed RotationPolicy mean that person gets rotated in or out anyway?

The EscalationStep logic looks good. Perhaps we could do something like Target string instead of Webhook and Email, but this is a minor detail with drawbacks as well.

Where would you store the current notification order? In the Members list of the team, like I intended?
How do you disable automatic rotations?

I'd be happy to update my PR once we agree on these points.

@dominikschulz
Copy link
Collaborator Author

I've updated my PR to include most of your suggestions. I didn't include the nested Person objects so far, to keep the API a bit simpler. This can later be changed if it makes sense.

@chrissnell
Copy link
Owner

RotationPolicy is a sort of overlay for the Team structure. Team, obviously, defines a group of people; RotationPolicy defines how we rotate on-call responsibility through the Team. Applying the RotationPolicy to the team yields a list of shifts. Eventually, we could have some kind of "override" policy that gets applied on top of that, to allow people to temporarily exchange shifts when someone's sick.

A Team might get you a slice of four People:

[PersonA, PersonB, PersonC, PersonD]

Applying a RotationPolicy with a RotationFrequency of 1 * time.Week() and a RotationTime of now (Monday June 15) gets you these shifts:

15 Jun - 22 Jun   PersonA
22 Jun - 29 Jun   PersonB
29 Jun  - 6 Jul   PersonC
6  Jul - 13 Jul   PersonD
13 Jul - 20 Jul   PersonA
...

json.NewEncoder(w).Encode(res)
}

// Updates an existing person in the database
Copy link
Owner

Choose a reason for hiding this comment

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

should be "team"

This commit renames some remaining artifacts to align with the subject.
@dominikschulz
Copy link
Collaborator Author

Thanks for the careful review. Updated my PR.

@chrissnell
Copy link
Owner

Thanks Dominik. I will try to take a look at this tonight after my kids go to bed. I've been thinking about the nesting of the Person objects, too. This is one of the compromises we have to make when using BoltDB. If Chicken Little used a relational DB, we could have primary keys to link between the objects. With our simple Bolt key-value store, it probably makes more sense to do as you have done and make Team.Members a []string of usernames from the people database.

@dominikschulz
Copy link
Collaborator Author

In my experience this project would really benefit from sticking with a simple database backend like BoltDB and sacrificing some integrity constraints. This makes the development much easier and I see no relevant drawbacks in this domain.

@chrissnell
Copy link
Owner

Working on this now. I noticed that you get an error when doing team operations (e.g. fetch all teams) when the "teams" bucket doesn't exist. I'm going to see about calling CreateBucketIfNotExists() on all of the buckets when the daemon first starts up.

@chrissnell chrissnell merged commit c9eb61d into chrissnell:master Jun 18, 2015
@dominikschulz dominikschulz deleted the teams branch June 18, 2015 07:35
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.

2 participants