Instance id propagation in Event proposed solutions
Change-Id: I8c39afa7e9f4f9ebf63d223e7bbaae397949dc0c
diff --git a/pages/design-docs/instance-id/alternative-solution-1.md b/pages/design-docs/instance-id/alternative-solution-1.md
new file mode 100644
index 0000000..e4be876
--- /dev/null
+++ b/pages/design-docs/instance-id/alternative-solution-1.md
@@ -0,0 +1,41 @@
+---
+title: ""
+permalink: design-docs/instance-id-alternative-solution-1.html
+hide_sidebar: true
+hide_navtoggle: true
+toc: false
+---
+
+# Overview
+
+Gerrit has already a [parameter](https://gerrit-documentation.storage.googleapis.com/Documentation/3.1.4/config-gerrit.html#gerrit.instanceName)
+to define an identifier for the instances. This could be included in the events
+generated by the different nodes.
+
+## <a id="implementation"> Implementation
+
+### Setup
+
+The `instanceName` is currently set in the `gerrit.config`. It defaults to the
+full hostname if not set in `gerrit.instanceName`.
+
+### Propagation
+
+Propagation of the id in the events can be done as explained in
+[Proposed solution](/design-docs/instance-id-solution.html) or
+[Alternative Solution - 2](/design-docs/instance-id-alternative-solution-2.html).
+
+## <a id="limitations"> Limitations
+
+`instanceName` is not used to uniquely identify an instance in a cluster.
+
+For example, it is, available to the email templating system.
+
+Using it would couple the presentation logic to something more backend specific
+as identifying an instance in a cluster.
+Also, it could not necessarily be relevant for the end users receiving it in
+their inbox.
+
+## <a id="use-case-fulfilment"> Use case fulfilment
+
+Same consideration as in the proposed [solution](/design-docs/instance-id-solution.html)
diff --git a/pages/design-docs/instance-id/alternative-solution-2.md b/pages/design-docs/instance-id/alternative-solution-2.md
new file mode 100644
index 0000000..2f58edf
--- /dev/null
+++ b/pages/design-docs/instance-id/alternative-solution-2.md
@@ -0,0 +1,41 @@
+---
+title: ""
+permalink: design-docs/instance-id-alternative-solution-2.html
+hide_sidebar: true
+hide_navtoggle: true
+toc: false
+---
+
+# Overview
+
+An alternative solution would be using a plugin to label the events.
+
+## <a id="implementation"> Implementation
+
+This is how we could achieve it:
+* intercept all the Events before they get fired from the `postEvent` method in
+`com.google.gerrit.server.events.EventBroker`
+* create a new event type (i.e.: `EventEnvelope`) as a wrapper of the Event class,
+containing the `instanceName` as an additional field
+* all the plugins which need to know the origin of an Event will have to use the
+`EventEnvelope` class as a base class to extend Events from
+
+This will label all the events, but the replication one, which are fundamental in
+a multi-master setup.
+
+To label the replication events the core plugin will need to be modified to expose
+the `postEvent` method and allow to override it.
+
+## <a id="limitations"> Limitations
+
+This would create dependency among plugins, which is a complicated pattern.
+
+Linking of the plugins into the /lib directory would be complex, since we will have
+to make sure the base plugin is loaded *before* the dependant one.
+
+Also, a different JSON payload for the stream events, depending if you have or
+not the plugin to enrich them, will be generated.
+
+## <a id="use-case-fulfilment"> Use case fulfilment
+
+Same consideration as in the proposed [solution](/design-docs/instance-id-solution.html).
diff --git a/pages/design-docs/instance-id/index.md b/pages/design-docs/instance-id/index.md
index f9da46f..096af4b 100644
--- a/pages/design-docs/instance-id/index.md
+++ b/pages/design-docs/instance-id/index.md
@@ -9,3 +9,6 @@
## Design Doc - Instance ID
* [Use Cases](/design-docs/instance-id-use-cases.html)
+* [Proposed solution](/design-docs/instance-id-solution.html)
+* [Alternative Solution - 1](/design-docs/instance-id-alternative-solution-1.html)
+* [Alternative Solution - 2](/design-docs/instance-id-alternative-solution-2.html)
diff --git a/pages/design-docs/instance-id/solution.md b/pages/design-docs/instance-id/solution.md
new file mode 100644
index 0000000..3b221ae
--- /dev/null
+++ b/pages/design-docs/instance-id/solution.md
@@ -0,0 +1,88 @@
+---
+title: ""
+permalink: design-docs/instance-id-solution.html
+hide_sidebar: true
+hide_navtoggle: true
+toc: false
+---
+
+# Overview
+
+The proposed solution would be having a dedicated configuration (i.e.: `instanceId`)
+used to identify the instance.
+
+## <a id="implementation"> Implementation
+
+### Setup
+
+The `instanceId` could be added in the `gerrit.config` under the `gerrit` section.
+It could be automatically initialised when not present at service startup,
+similarly to how the `serverId` is generated today.
+
+The id could be a UUIDv4 to avoid the need of a central authority for the generation
+of a unique ID.
+
+### Propagation
+
+Each event will need to be labelled with the `instanceId` it has been
+generated from.
+The `com.google.gerrit.server.events.Event` will need to be modified to accommodate
+the new parameter.
+
+### Exposing the identifier
+
+Gerrit will have to expose the local `instanceId` so plugins can use it.
+This can be done with a new @InstanceId annotation.
+
+# Current consumers compatibility
+
+## Gerrit Jenkins Trigger Plugin
+
+The addition of an `instanceId` in the Events won’t affect the Gerrit Jenkins
+Trigger Plugin, since the DTO used is really lax and decoupled from Gerrit.
+Additional fields added to the Event will just be ignored.
+
+## Other plugins
+
+I used [this query](https://cs.bazel.build/search?q=r%3Aplugin++com.google.gerrit.server.events.Event&num=200)
+and [this other one](https://github.com/search?l=Java&q=org%3AGerritForge+%22events.Event%22+NOT+Test&type=Code)
+to assess the plugins that might be impacted by the change of the
+`com.google.gerrit.server.events.Event`.
+
+Plugins that would need adaptation with this change will be the
+_multi-site_ and the _events-broker_ since they will have to start using the core
+`instanceId` instead of the multi-site definition of `instanceId`.
+
+The _events-log_ plugin, used for example by the Gerrit Jenkins Trigger Plugin,
+might also need extra work to store the extra field in the database.
+A database schema migration will also be needed.
+
+The _high-availability_ won't need to be modified, but there will be the chance
+of simplifying it and make it more resilient, since it won't have to store the
+forwarded events in the thread local storage anymore.
+
+## <a id="use-case-fulfilment"> Use case fulfilment
+
+This solution would satisfy the [use cases](/design-docs/instance-id-use-cases.html)
+presented in the description of the problem.
+
+Having the `instanceId` will allow to inspect the event payload to understand its origin.
+
+Let's analyse the by-products:
+
+* _Avoid loops:_ events with the same `instanceId` of the consuming instance
+can be filtered out, avoiding creation of loops. Plugins like the `high-availability`
+can be simplified avoiding to store in memory the forwarded events to keep track
+of the origin.
+* _Priority routing_: logic can be implemented around `instanceId` of an event to
+decide which one to consume first depending on its origin.
+Discoverability of the other instances in the cluster is not part of this work.
+* _Troubleshooting_: `instanceId` can be dumped in log if needed. This is already possible
+only the extra field will be present as well.
+* _Preventing events duplication_: if we take the case of the Slack plugin, notifications
+can be sent only from the instance producing the event
+(i.e.: `instanceId` of the event == `instanceId` of the Gerrit instance). This will
+prevent multiple notifications.
+This scenario is the dual of the first one, _Avoid loops_.
+In this case instance producing an event wants to perform some operations when
+consuming it, in the other case we just want a no-ops.
