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

Awkward label dance with getsantry bot whenever we open an issue for our own team #107

Closed
brentc opened this issue Feb 15, 2023 · 16 comments

Comments

@brentc
Copy link

brentc commented Feb 15, 2023

Not sure if this it the right place to report this so let me know otherwise!

The MDX team is in the process of moving over to GitHub for tracking MDX epics, tasks, and bugs, and also for our ongoing support of D&D work.

Whenever we open a new issue, we have to go through a little labeling dance with getsantry, as seen on getsentry/sentry#44663:
image

Is there some specific process when opening a ticket we can do to not do this? Or alternatively some way to upgrade the bot to be a little smarter when members of team-mdx open tickets with Team: Mobile Developer Experience / discover-n-dashboards open tickets with Team: Discover & Dashboards?

@chadwhitacre
Copy link
Member

Not sure if this it the right place to report this

This is a great place to report this, thanks! 😁

@chadwhitacre
Copy link
Member

We have a project spec'd out to improve the automations, this would fit under that. We decided to focus on other things for Q1, though. :-/

Maybe there's some low-hanging fruit somewhere in here though?

@brentc
Copy link
Author

brentc commented Feb 15, 2023

Ok, we can work around it for now. Confirmation that there wasn't an existing process for avoiding that we just weren't doing is good though.

@brentc
Copy link
Author

brentc commented Feb 15, 2023

Hmm, am I reading this wrong, or is the intended workflow that if the user opening the issue is an internal dev the Status: Untriaged shouldn't be being applied at all?

@chadwhitacre
Copy link
Member

chadwhitacre commented Feb 16, 2023

So it's a little confusing, but markUntriaged runs when issues are opened in repos other than sentry and sentry-docs.

What we're seeing at, e.g., getsentry/sentry#44663 (comment), is that:

(Looks like there's more mess with Status labels but let's stay focused for a second.)

I think for low-hanging fruit, the rule should maybe be that when someone in EPD (vs. GTM) routes an issue via a Team: Foo label, then we should add Status: Backlog instead of Status: Untriaged.

Thoughts @hubertdeng123, et al.?

@hubertdeng123
Copy link
Member

The reason why there's a weird dance is because our issue-status-helper.yml automation tries to guarantee that there is only one Status: * label 😬. Honestly, maybe we should just remove the issue status helper automation entirely since I feel like it's not super useful?

Agh, I think the remedy for this is checking to see if the membership of the user who opened the issue before routing. If the issue is made by an internal dev that doesn't belong to the GTM team, then we can skip routing altogether. I did have a PR up that could use a revisit that I planned on tackling after enabling routing for GTM members. I don't think we should default to putting things on the backlog if routed by EPD. If an issue needs to be rerouted, it'll end up on the backlog if a team member reroutes it which really isn't what we want.

@chadwhitacre
Copy link
Member

chadwhitacre commented Feb 16, 2023

(Looks like there's more mess with Status labels but let's stay focused for a second.)

The reason why there's a weird dance is because our issue-status-helper.yml automation tries to guarantee that there is only one Status: * label 😬.

Ah, yeah, I think that's for this:


action


But we also have this:


app


Right?

@chadwhitacre
Copy link
Member

Honestly, maybe we should just remove the issue status helper automation entirely since I feel like it's not super useful?

I'd be fine with that for a quick fix. Seems really susceptible to race conditions since it runs in an action, which takes 60+ seconds.

If the issue is made by an internal dev that doesn't belong to the GTM team, then we can skip routing altogether.

Pretty sure we're already good here. On the example we don't see @getsantry apply Status: Unrouted, right? Hopefully this is because @edwardgou-sentry passes the test. 😅

@hubertdeng123
Copy link
Member

That's correct.

For this, we don't want the routing to even happen in the first place

219513068-983a5f7f-e1fb-42f1-bef6-622cdfca8c0b

I think the remedy for this is checking to see if the membership of the user who opened the issue before routing. If the issue is made by an internal dev that doesn't belong to the GTM team, then we can skip routing altogether. I did have a PR up that could use a revisit that I planned on tackling after enabling routing for GTM members. I don't think we should default to putting things on the backlog if routed by EPD.

What I'm saying here addresses this, since we're checking the membership of the user who opened the issue. If it's someone in EPD I'm assuming it's very likely that it's an issue for their own team or they know who to assign it to already.

@hubertdeng123
Copy link
Member

Actually I thought about that again, maybe the best solution is checking the membership of the user who opened the issue.

Internal User (EPD):

  1. Skip the routing message
  2. Add the Status: Backlog label

Internal Member (GTM):

  1. Add the Status: Unrouted label
  2. Route as normal

External Member:

  1. Add the Status: Unrouted label
  2. Route as normal

@chadwhitacre
Copy link
Member

maybe the best solution is checking the membership of the user who opened the issue.

Pretty sure this is where I landed in #107 (comment), yes? 😅

I think for low-hanging fruit, the rule should maybe be that when someone in EPD (vs. GTM) routes an issue via a Team: Foo label, then we should add Status: Backlog instead of Status: Untriaged.

Assuming we're on the same page (pretty sure we are) then our action items are:

  1. Whack issue-status-helper.yml (part of Migrate all github actions -> eng-pipes #84, really)
  2. Modify markRouted to put on the backlog when routed by EPD.

Look right?

For (2) let's make a note in a code comment that if possible on the other side of #90 we should base backlog/untriaged on membership in the actual affected team (only put on Foo's backlog if routed by members of Team: Foo vs. EPD generally).

@hubertdeng123
Copy link
Member

Pretty sure this is where I landed in #107 (comment), yes? 😅

Almost....just the difference between the membership of the user who routed the issue vs the user who opened the issue 😜. If it's done by membership of the user routing we can have the situation where

  1. Issue comes in with Status: Unrouted label
  2. Support routes the issue to Team: Foo and automation adds Status: Untriaged label
  3. Team: Foo wants to reroute the issue. A member of the team reroutes to Team: Bar.
  4. Issue is rerouted to Team: Bar but the Status: Backlog label is applied.

Otherwise we're on the same page here.

@hubertdeng123
Copy link
Member

only put on Foo's backlog if routed by members of Team: Foo vs. EPD generally

Should we rely on the Github membership of teams for this? Last time I checked there seemed to be quite a lot of overlap of people on different teams 😅

@chadwhitacre
Copy link
Member

Should we rely on the Github membership of teams for this? Last time I checked there seemed to be quite a lot of overlap of people on different teams 😅

Not yet, relying on GitHub teams is theoretical until we get through #90.

we can have the situation where

Oof, yeah, good call. :-/

@hubertdeng123
Copy link
Member

hubertdeng123 commented Feb 27, 2023

@chadwhitacre
Copy link
Member

Closing as not an issue anymore, we are smarter with this now.

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

No branches or pull requests

3 participants