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 support for multiple teams #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

just-chillin
Copy link

@just-chillin just-chillin commented Oct 30, 2023

This PR adds support for querying multiple teams, for example, foo and bar.

This is for the case where a single on call report may require joining incidents owned from multiple teams. For example a query like below is supported:

./incidentist --team=foo --team=foo-bar

This will return all of the datadog incidents owned by foo and foo-bar.

Next Steps

At the moment, this PR simply concatenates the incidents owned by all of the teams. If foo and foo-bar both own incident-1234, it will show up twice in the incident report. To fix this, we need to do a query to the datadog-api like teams:("foo" OR "foo-bar") instead of a single query per team. I didn't do this here because I didn't want to spend more than 30 minutes on this PR, and forming a query with raw strings didn't sound like much fun 🙃

@badovm
Copy link
Contributor

badovm commented Oct 30, 2023

Can we just use something simple on the client side to filter out already-seen incidents in the report?

Comment on lines +71 to +75
teamIncidents, err := fetchIncidents(team, sinceAt, untilAt)
if err != nil {
exit("Failed to fetch incidents from Datadog: %v", err)
}
incidents = append(incidents, teamIncidents...)
Copy link
Owner

Choose a reason for hiding this comment

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

Should we instead change the filter in fetchIncidents to grab multiple teams, I haven't touched this code in a while but should be possible? cc @badovm @robertjli

@robertjli
Copy link
Contributor

#11 was merged which resolves this issue, @just-chillin you can close this PR at your convenience 🙇

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.

4 participants