feat(AIP-236) add StopPreview method and annotations (#1080)

This is to allow other stop methods for other safe rollout use cases. Also, adding annotations

Author: nitinapi

aip/general/0236.md

@@ -34,7 +34,7 @@ The expected flow for previewing a policy is as follows:
 
 1. The user creates an experiment containing a new policy configuration
    intended to replace the live policy.
-2. The user uses the "preview" method to start generating logs which compare
+2. The user uses the "startPreview" method to start generating logs which compare
    the live and experiment policy evaluations against live traffic.
 3. The user inspects the logs to determine whether the experiment has the
    intended result.
@@ -86,6 +86,9 @@ message PolicyExperiment {
   // The metadata associated with this policy experiment.
   PolicyPreviewMetadata preview_metadata = 3
       [(google.api.field_behavior) = OUTPUT_ONLY];
+
+  // Allows clients to store small amounts of arbitrary data.
+  map<string, string> annotations = 4;
 }
 ```
 
@@ -108,6 +111,8 @@ message PolicyExperiment {
     capped at a certain number and the cap **must** be documented.
 - Cascading deletes **must** occur: if the live policy is deleted, all
   experiments **must** also be deleted.
+- `map<string,string>` [annotations][aip-128-annotations] **must** allow clients
+  to store small amounts of arbitrary data.
 
 ### Metadata
 
@@ -118,9 +123,20 @@ same service with experiments.
 
 ```proto 
 message PolicyPreviewMetadata {
-  // The state of previewing the experiment. The possible values are ACTIVE
-  // and INACTIVE. ACTIVE indicates that results are being logged upstream.
-  string state = 1;
+  // Possible values of the state of previewing the experiment.
+  enum State {
+    // Default value. This value is unused.
+    STATE_UNDEFINED = 0;
+    
+    // The experiment is actively previewing.
+    ACTIVE = 1;
+
+    // The previewing of the experiment has been stopped.
+    SUSPENDED = 2;  
+  }
+
+  // The state of previewing the experiment.
+  State state = 1;
 
   // An identifying string common to all logs generated when previewing the
   // experiment. Searching all logs for this string will isolate the results.
@@ -135,27 +151,27 @@ message PolicyPreviewMetadata {
 ```
 
 - `PolicyPreviewMetadata` **must** have the fields defined in the proto above.
-  - `state` **must** be `ACTIVE` or `INACTIVE`.
   - It **may** have additional fields if the service or resource requires it.
 - When an experiment is first previewed, `preview_metadata` **must** be
   absent.
-  - It is present on the experiment once the "preview" method is used.
+  - It is present on the experiment once the "startPreview" method is used.
 - All `preview_metadata` fields **must** be output only.
-- `state` changes to and from `ACTIVE` and `INACTIVE` when the experiment is
-  started or stopped, which can only be done by the "preview" and "stop" custom
-  methods.
-- The first time the "preview" custom method is used, the system **must** create
-  `preview_metadata` and do the following: 
+- `state` changes between `ACTIVE` and `SUSPENDED` when previewing is started
+  or stopped. This happens when the "startPreview" or "stopPreview custom methods
+  are invoked, respectively.
+- The first time the "startPreview" custom method is used, the system **must**
+  create `preview_metadata` and do the following: 
   - It **must** set the `state` to `ACTIVE`
   - It **must** populate `start_time` with the current time. 
-    - `start_time` **must** be updated every time the status is changed to
+    - `start_time` **must** be updated every time `state` is changed to
       `ACTIVE`.
   - It **must** set a system generated `log_prefix` string, which is a
     predefined constant hard coded by the system developers. 
   - The same value is used for previewing experiments for the given resource
     type. For example, "FirewallPolicyPreviewLog" for FirewallPolicy.
-- When the "stop" custom method is used, the system **must** do the following:
-  - It **must** set the `state` to `INACTIVE`
+- When the "stopPreview" custom method is used, the system **must** do the
+  following:
+  - It **must** set the `state` to `SUSPENDED`
   - It **must** populate the `stop_time` with the current time. 
 
 ### Methods
@@ -196,8 +212,8 @@ cases:
   order to change the experiment being previewed because this `policy` is
   intended to replace the live policy, and the name of the live policy
   **must not** change.
-- The system **must** set the `state` to INACTIVE if the `state` was ACTIVE at
-  the time of an update.
+- The system **must** set the `state` to `SUSPENDED` if the `state` was `ACTIVE`
+  at the time of an update.
   - This is so the user can easily distinguish between different versions of
     the experiment being previewed.
 
@@ -221,25 +237,25 @@ cases:
   `google.longrunning.operation_info.response_type` **must** be
   `PolicyExperiment`.
 
-#### preview
+#### startPreview
 
 ```proto
 // Starts previewing a PolicyExperiment. This triggers the system to start
 // generating logs to evaluate the PolicyExperiment.
-rpc PreviewPolicyExperiment(PreviewPolicyExperimentRequest)
+rpc StartPreviewPolicyExperiment(StartPreviewPolicyExperimentRequest)
     returns (google.longrunning.Operation) {
   option (google.api.http) = {
-    post: "/v1/{name=policies/*/experiments/*}:preview"
+    post: "/v1/{name=policies/*/experiments/*}:startPreview"
     body: "*"
   };
   option (google.longrunning.operation_info) = {
     response_type: "PolicyExperiment"
-    metadata_type: "PreviewPolicyExperimentMetadata"
+    metadata_type: "StartPreviewPolicyExperimentMetadata"
   };
 }
 
-// The request message for the preview custom method.
-message PreviewPolicyExperimentRequest {
+// The request message for the startPreview custom method.
+message StartPreviewPolicyExperimentRequest {
   // The name of the PolicyExperiment.
   string name = 1;
 }
@@ -258,25 +274,25 @@ message PreviewPolicyExperimentRequest {
 - If the method is called on an experiment with the rules representing a no-op,
   then the system **must** preview the deletion of the live policy.
 
-#### stop
+#### stopPreview
 
 ```proto
 // Stops previewing a PolicyExperiment. This triggers the system to stop
 // generating logs to evaluate the PolicyExperiment.
-rpc StopPolicyExperiment(StopPolicyExperimentRequest)
+rpc StopPreviewPolicyExperiment(StopPreviewPolicyExperimentRequest)
     returns (google.longrunning.Operation) {
   option (google.api.http) = {
-    post: "/v1/{name=policies/*/experiments/*}:stop"
+    post: "/v1/{name=policies/*/experiments/*}:stopPreview"
     body: "*"
   };
   option (google.longrunning.operation_info) = {
     response_type: "PolicyExperiment"
-    metadata_type: "StopPolicyExperimentMetadata"
+    metadata_type: "StopPreviewPolicyExperimentMetadata"
   };
 }
 
-// The request message for the stop custom method.
-message StopPolicyExperimentRequest {
+// The request message for the stopPreview custom method.
+message StopPreviewPolicyExperimentRequest {
   // The name of the PolicyExperiment.
   string name = 1;
 }
@@ -290,7 +306,7 @@ message StopPolicyExperimentRequest {
 - Whenever the method is called successfully, the system **must** set the
   following values in the `PolicyPreviewMetadata`:
   - `stop_time` to the current time
-  - `state` to `INACTIVE`
+  - `state` to `SUSPENDED`
 
 #### commit
 
@@ -371,8 +387,11 @@ experiment to live.
 
 ## Changelog
 
+- **2023-04-27:** Methods for start and stop renamed. State to enum. Annotations
+  added.
 - **2023-03-30:** Initial AIP written.
 
+[aip-128-annotations]: https://aip.dev/128#annotations
 [aip-131]: https://aip.dev/131
 [aip-132]: https://aip.dev/132
 [aip-133-long-running]: https://aip.dev/133#long-running-create