add confirmation step to interactive slack judgements

[erikmunson]

Today we have wonderful interactive slack buttons on manual judgement notifications that allow us to judge without switching contexts — unfortunately that ease of approval also means that sometimes people (ahem, I would never do such a silly thing 🙈) accidentally fat-finger the approve or reject buttons on, say, their iPhone while trying to scroll through messages.

Slack has some pretty incredible building blocks for creating modals, it'd be awesome if we could avoid those scenarios by having both the approve and reject buttons launch into a confirmation modal kind of like the ones we have in deck, which would let you say 'yes i really want to do this' or back out e.g.:

[luispollo]

@erikmunson @robzienert I'd been meaning to bring this up but wasn't sure how to frame it (I'm still not). I thought I'd add a couple notes here in the hope of gathering some early feedback.

@erikmunson, on the point about leveraging the Slack building blocks for user interaction, you're totally right, but the challenge is that, based on conversations with @robzienert at the time, the approach I took was to essentially create Spinnaker's own framework for interactive notifications, which, while heavily influenced by the Slack design, introduced a layer of abstraction between Spinnaker and the various notification tools it supports.

The catch is, I started with a very small feature set to address the immediate need of our interactive notifications at the time, with an implementation for Slack only. While it should be entirely technically feasible to add additional building blocks to Spinnaker's framework, I find myself frequently questioning the value of doing that because (1) we're only using Slack, (2) there hasn't been any traction using interactive notifications in any other Spinnaker services at Netflix and (3) there hasn't been any traction in the OSS community either to contribute in that area (not that I specifically reached out to ask for contributions, in fairness).

It would be a breeze to create really nice interactive flows for MD with keel interacting with Slack directly. Not so much so when I have to involve gate and echo and add the shim to convert abstract building blocks to Slack's implementation, not to mention the challenge of using echo as the router for callbacks without any persistent data store.

I feel bad to even bring this up because, conceptually, I have no doubt the approach we took is the "right" one. But, pragmatically, is it worth pursuing further?

Please do share your thoughts.

[robzienert]

🤷 I was trying to get ahead of the inevitable issue of people in OSS not using Slack and wanting similar chatops interactive functionality.

I have very strong doubts that any (or many) organizations in OSS will be adopting MD, so if we're fine with tightly coupling to the Netflix stack (and thus making Slack alternatives unavailable) because that makes the Netflix experience better, I'm all for it.

[robzienert]

One issue to this root proposal that may just affect me: I don't have VPN on the phone, and won't be getting VPN on the phone. If I click "approve" or "reject" from my phone, it'll go to a website that I don't have access to - will Slack report that I made an action on that message and now make it unavailable for anyone with VPN to click on it? 🤔

[erikmunson]

Knowing nothing about the actual backend code involved here, I'm fairly unopinionated about the merits of having an abstract contract vs. coding against Slack directly when it comes to technical implementation, along with whether it makes it less write-once-use-for-everything if someone wants to add a new chat app. However, I'd be somewhat concerned if going in the Slack-specific direction meant that it would be much harder for other spinnaker services to start talking to Slack interactively as I think we'll see orca and maybe more services end up wanting it.

At this point I'm not even sure how much of this full-featured interactivity other popular chat apps like Google Chat or Microsoft Teams have in comparison to Slack, it's possible this type of modal flow wouldn't even be an option for them. There's also a question in my mind about how interoperable / "abstractable" the different chat app UI building blocks would be if they did support it — no idea currently.

On it's own, getting full-featured Slack interactivity out into the world feels like a pretty high impact thing to do even if it reduces the likelihood of other apps being added, as Slack's adoption means there's a big chunk of the community that this would apply to (even though there will be some percentage that are left out). If the story for orca and other spinnaker services doesn't get meaningfully worse as a result, I'd be sort of inclined to get something out there vs. nothing.

Not sure what the VPN part is about? Do those buttons make calls to VPN'd endpoints when clicked (or is that was this is proposing)? We definitely wouldn't be kicking you over to deck or anything like that in a browser.

[robzienert]

Ha. Nevermind the VPN thing. I had a brain lapse when I saw the screenshot in the issue and forgot the modal would be in Slack itself. Carry on. :)

[luispollo]

On it's own, getting full-featured Slack interactivity out into the world feels like a pretty high impact thing to do even if it reduces the likelihood of other apps being added, as Slack's adoption means there's a big chunk of the community that this would apply to (even though there will be some percentage that are left out). If the story for orca and other spinnaker services doesn't get meaningfully worse as a result, I'd be sort of inclined to get something out there vs. nothing.

Sorry about the delay responding to this. The only downside I can think of in terms of other Spinnaker services wanting to use Slack interactive notifications is that they'd need to replicate the code to make REST calls to Slack. That essentially entails writing models for the objects (if you don't want to use a Map), and interfaces to build a Retrofit client. But we already do the exact same thing for all of our own services, since there are no shared client libraries. And we actually already have an example of the Slack API being used in different services: echo has notifications, and code was added to gate recently to help the UI auto-fill in the list of Slack channels. In other words, I feel the impact is minimal.

On the upside, each service would be entirely free to interact with Slack however they like, and I think no one will be shocked that the Slack documentation for the API and the available test tools are much better than our own... 🙂

Finally, this is timely because we've switched all services at Netflix to communicate over Metatron, which was one of the prerequisites to integrate with Wall-e. Which means we have all the pieces of the puzzle to allow each service to integrate directly with Slack.

Given all the above, my vote goes for ditching the abstraction layer. Thoughts?

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[robzienert]

I guess I never followed up on this, sorry! +1 to ditching the abstraction layer.

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[emjburns]

I'm also a +1 towards interacting with slack directly. I'd rather have something that works well for us and abstract it later if needed. I'm also fine to say that the interactive portion only works for slack, and send a different and simpler message for other things.

Given that we're proposing to do complicated things with Slack, I also think it's ok to try to do that directly from keel (going through gate is unavoidable because that's the only way users interact with us). We can always abstract to echo if that is useful later.

[luispollo]

Thanks @emjburns. I'll try to pick this up as soon as I have some room to breathe. 🙂

[spinnakerbot]

This issue hasn't been updated in 50 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[luispollo]

It only took us 10 months! Yay! 😄 🎉

[gal-yardeni]

Addressed in PR https://github.com/spinnaker/keel/pull/1733 and other PRs in keel.