High level overview for building your own plugin

[neilkakkar]

Changes

Did a first pass through this, capturing feedback before I polish this further.

FigJam for the images: GeoIP - https://www.figma.com/file/luaMsTScYN1fMoYliEfjrq/Plugin-Events?node-id=0%3A1

And S3 - https://www.figma.com/file/rr8QlwmSceKRaUq3Cg5vs1/Plugin-Events-External

Not sure if worth getting these made better via the great @lottiecoxon ^

Checklist

• I have checked out our styleguide

[mariusandra]

There's just one problem with this line in that it's not technically true. I'm not sure if this is written like this for simplicity or just in error. However some plugins act on events before they are stored. Some plugins act on events at the same time that the event is being stored.

Basically:

event = await getNextEvent()
event = await processEvent1(event)
event = await processEvent2(event)
await Promise.all([
    storeEventInDatabase(event),
    onEvent1(event),
    onEvent2(event),
])

This means also that the figure for the GeoIP plugin example is correct, but the one for the S3 plugin is not. It goes into S3 and our db at the same time, two parallel arrows.

[neilkakkar]

Indeed, that's right! I wanted to emphasise how some special functions mutate and others don't.

I think it's a technical detail that doesn't add extra information from the user's perspective? (i.e., I don't think knowing this will help them make better plugins? / understand plugins?)

I could do a footnote saying how this isn't exactly true?

[mariusandra]

I guess a footnote would suffice. The only technically important thing to clarify here is that even if an onEvent fails, the event still ends up in the database (as they are running in parallel). Otherwise people might get the wrong idea, and believe that a failure here will block the event.

[neilkakkar]

Hmm, makes sense, that's the crux: there's no data loss. I'll highlight that here, and skip the footnote about the extra technical details.

[mariusandra]

Actually, we might need to explain this quite clearly for everything to click in place.

Inside events we have three special properties: $plugins_deferred, $plugins_failed, $plugins_succeeded:

The "deferred" literally implies that these plugins were run after (ingestion).

[neilkakkar]

I've made a note to add this bit in the developer reference, but not in the high level overview.

[yakkomajuri]

This is great! I like the structure and love how this takes me on a journey from the very basics.

Left a bunch of minor inline comments, but nothing particularly major.

One thing I'm not a fan of are the images. First, they're not transparent nor a neutral color that works in both light and dark mode. Second, I can't read what's going on. Third, they're not particularly beautiful (in comparison to our website images as a whole). Could be worth getting @lottiecoxon to quickly give this a look? I'm also happy to help

[yakkomajuri]

this image is a bit wrong, as it gives me processEvent vibes. I'd personally prefer 2 arrows coming out of the event, one going via the plugin to S3, and the other to the DB

[neilkakkar]

Hmmmm, as a reader I feel that to be more confusing: how/why did the event go to both places?

I think if I remove the second event from the diagram, it feels closer to onEvent ?

[neilkakkar]

I really do think this raises more questions than it answers.

Disregarding how things actually happen, a good model helps people reason about the system without knowing exactly what's happening in the system. And I think all people need to know is that it's like processEvent, but the return value doesn't matter, so they can do all sorts of "side stuff" with onEvent.

[yakkomajuri]

I wonder how people will feel given a lot of our plugins don't follow this

[yakkomajuri]

Should be fine still, just wondering

[neilkakkar]

Precisely why I want to have this section, so that atleast starting from now all plugins follow this convention

[neilkakkar]

Regarding the images, (1) and (3) make sense. For (2) - I'd expect people to click on the image to see the high res one with all details. Did this not feel obvious?

[neilkakkar]

Hiya! Are there any remaining concerns which stop us from publishing these changes?

I'll put in a design request to polish the images.

Regarding what the images should have: @mariusandra @yakkomajuri - does this comment help alleviate concerns? #1467 (comment)

[yakkomajuri]

I'm happy to get this in the name of iteration.

It's great stuff, I'd just like the images to at least be transparent (plus a few other tweaks mentioned above)

[neilkakkar]

Sounds good! I should've gotten to all the tweaks, except for the images, in aff2114