CA-400124 - rrd: Serialize transform parameter for data sources (#6043)

Previously, the transform function was not serialized in the
plugin-server protocol (it was only used in xcp_rrdd itself, not in
plugins). This issue was revealed by the previous work around splitting
cpu metrics into a separate plugin.

Instead of allowing arbitrary functions (which would be difficult to
serialize), for 'fun x' offer just two options:
* Inverse (1.0 - x), and
* Identity (x)

Default (if the parameter is not provided, either in OCaml or JSON), is
Identity.

ocaml/libs/xapi-rrd/lib/rrd.ml

@@ -22,6 +22,12 @@ exception No_RRA_Available
 
 exception Invalid_data_source of string
 
+(** Inverse is (fun x -> 1.0 - x) *)
+type ds_transform_function = Inverse | Identity
+
+let apply_transform_function f x =
+  match f with Inverse -> 1.0 -. x | Identity -> x
+
 type ds_owner = VM of string | Host | SR of string
 
 (** Data source types - see ds datatype *)
@@ -84,6 +90,12 @@ let ds_value_to_string = function
   | _ ->
       "0.0"
 
+let ds_transform_function_to_string = function
+  | Inverse ->
+      "inverse"
+  | Identity ->
+      "identity"
+
 (** The CDP preparation scratch area.
     The 'value' field should be accumulated in such a way that it always
     contains the value that will eventually be the CDP. This means that
@@ -417,7 +429,7 @@ let ds_update rrd timestamp values transforms new_domid =
                  )
             in
             (* Apply the transform after the raw value has been calculated *)
-            let raw = transforms.(i) raw in
+            let raw = apply_transform_function transforms.(i) raw in
             (* Make sure the values are not out of bounds after all the processing *)
             if raw < ds.ds_min || raw > ds.ds_max then
               nan
@@ -450,7 +462,7 @@ let ds_update_named rrd timestamp ~new_domid valuesandtransforms =
     valuesandtransforms |> List.to_seq |> StringMap.of_seq
   in
   let get_value_and_transform {ds_name; _} =
-    Option.value ~default:(VT_Unknown, Fun.id)
+    Option.value ~default:(VT_Unknown, Identity)
       (StringMap.find_opt ds_name valuesandtransforms)
   in
   let ds_values, ds_transforms =
@@ -519,7 +531,7 @@ let rrd_create dss rras timestep inittime =
     }
   in
   let values = Array.map (fun ds -> ds.ds_last) dss in
-  let transforms = Array.make (Array.length values) (fun x -> x) in
+  let transforms = Array.make (Array.length values) Identity in
   ds_update rrd inittime values transforms true ;
   rrd
 

ocaml/libs/xapi-rrd/lib_test/crowbar_tests.ml

@@ -81,7 +81,7 @@ let rrd =
       List.iteri
         (fun i v ->
           let t = 5. *. (init_time +. float_of_int i) in
-          ds_update rrd t [|VT_Int64 v|] [|Fun.id|] (i = 0)
+          ds_update rrd t [|VT_Int64 v|] [|Identity|] (i = 0)
         )
         values ;
       rrd

ocaml/libs/xapi-rrd/lib_test/unit_tests.ml

@@ -131,7 +131,7 @@ let gauge_rrd =
   let rrd =
     rrd_create [|ds; ds2; ds3; ds4|] [|rra; rra2; rra3; rra4|] 1L 1000000000.0
   in
-  let id x = x in
+  let id = Identity in
   for i = 1 to 100000 do
     let t = 1000000000.0 +. (0.7 *. float_of_int i) in
     let v1 = VT_Float (0.5 +. (0.5 *. sin (t /. 10.0))) in
@@ -159,7 +159,7 @@ let _deserialize_verify_rrd =
 
   let rrd = rrd_create [|ds|] [|rra1; rra2; rra3|] 5L init_time in
 
-  let id x = x in
+  let id = Identity in
   for i = 1 to 100 do
     let t = init_time +. float_of_int i in
     let t64 = Int64.of_float t in
@@ -178,7 +178,7 @@ let ca_322008_rrd =
 
   let rrd = rrd_create [|ds|] [|rra1; rra2; rra3|] 5L init_time in
 
-  let id x = x in
+  let id = Identity in
 
   for i = 1 to 100000 do
     let t = init_time +. float_of_int i in
@@ -198,7 +198,7 @@ let ca_329043_rrd_1 =
 
   let rrd = rrd_create [|ds|] [|rra1; rra2; rra3|] 5L init_time in
 
-  let id x = x in
+  let id = Identity in
 
   let time_value_of_i i =
     let t = 5. *. (init_time +. float_of_int i) in
@@ -228,7 +228,7 @@ let create_rrd ?(rows = 2) values min max =
     rrd_create [|ds1; ds2; ds3|] [|rra1; rra2; rra3; rra4|] 5L init_time
   in
 
-  let id x = x in
+  let id = Identity in
 
   List.iteri
     (fun i v ->

ocaml/xapi-idl/rrd/ds.ml

@@ -25,11 +25,11 @@ type ds = {
   ; ds_min: float
   ; ds_max: float
   ; ds_units: string
-  ; ds_pdp_transform_function: float -> float
+  ; ds_pdp_transform_function: Rrd.ds_transform_function
 }
 
 let ds_make ~name ~description ~value ~ty ~default ~units ?(min = neg_infinity)
-    ?(max = infinity) ?(transform = fun x -> x) () =
+    ?(max = infinity) ?(transform = Rrd.Identity) () =
   {
     ds_name= name
   ; ds_description= description

ocaml/xcp-rrdd/bin/rrdd/rrdd_monitor.ml

@@ -77,14 +77,6 @@ module OwnerMap = Map.Make (struct
         String.compare a b
 end)
 
-let owner_to_string () = function
-  | Host ->
-      "host"
-  | VM uuid ->
-      "VM " ^ uuid
-  | SR uuid ->
-      "SR " ^ uuid
-
 (** Updates all of the hosts rrds. We are passed a list of uuids that is used as
     the primary source for which VMs are resident on us. When a new uuid turns
     up that we haven't got an RRD for in our hashtbl, we create a new one. When

ocaml/xcp-rrdd/bin/rrdp-cpu/rrdp_cpu.ml

@@ -142,8 +142,7 @@ let dss_pcpus xc =
               ~description:("Physical cpu usage for cpu " ^ string_of_int i)
               ~value:(Rrd.VT_Float (Int64.to_float v /. 1.0e9))
               ~min:0.0 ~max:1.0 ~ty:Rrd.Derive ~default:true
-              ~transform:(fun x -> 1.0 -. x)
-              ()
+              ~transform:Rrd.Inverse ()
           )
           :: acc
         , i + 1
@@ -158,9 +157,7 @@ let dss_pcpus xc =
     , Ds.ds_make ~name:"cpu_avg" ~units:"(fraction)"
         ~description:"Average physical cpu usage"
         ~value:(Rrd.VT_Float (avg_array /. 1.0e9))
-        ~min:0.0 ~max:1.0 ~ty:Rrd.Derive ~default:true
-        ~transform:(fun x -> 1.0 -. x)
-        ()
+        ~min:0.0 ~max:1.0 ~ty:Rrd.Derive ~default:true ~transform:Rrd.Inverse ()
     )
   in
   avgcpu_ds :: dss

ocaml/xcp-rrdd/lib/transport/base/rrd_json.ml

@@ -44,6 +44,15 @@ let ds_owner x =
         string "sr %s" sr
   )
 
+let ds_transform x =
+  match x with
+  | Rrd.Identity ->
+      []
+      (* This is the default when transform is absent, and not including it
+         makes the file smaller *)
+  | Rrd.Inverse ->
+      [("transform", string "inverse")]
+
 let bool b = string "%b" b (* Should use `Bool b *)
 
 let float x = string "%.2f" x
@@ -63,6 +72,7 @@ let ds_to_json (owner, ds) =
          [
            description ds.Ds.ds_description
          ; [ds_owner owner]
+         ; ds_transform ds.Ds.ds_pdp_transform_function
          ; ds_value ds.Ds.ds_value
          ; [ds_type ds.Ds.ds_type]
          ; [

ocaml/xcp-rrdd/lib/transport/base/rrd_protocol_v2.ml

@@ -193,8 +193,13 @@ let uninitialised_ds_of_rpc ((name, rpc) : string * Rpc.t) :
   let default =
     bool_of_string (Rrd_rpc.assoc_opt ~key:"default" ~default:"false" kvs)
   in
+  let transform =
+    Rrd_rpc.transform_of_string
+      (Rrd_rpc.assoc_opt ~key:"transform" ~default:"identity" kvs)
+  in
   let ds =
-    Ds.ds_make ~name ~description ~units ~ty ~value ~min ~max ~default ()
+    Ds.ds_make ~name ~description ~units ~ty ~value ~min ~max ~default
+      ~transform ()
   in
   (owner, ds)
 

ocaml/xcp-rrdd/lib/transport/base/rrd_rpc.ml

@@ -53,3 +53,13 @@ let owner_of_string (s : string) : Rrd.ds_owner =
       Rrd.SR uuid
   | _ ->
       raise Rrd_protocol.Invalid_payload
+
+(* Converts a string to value of ds_transform_function type. *)
+let transform_of_string (s : string) : Rrd.ds_transform_function =
+  match s with
+  | "inverse" ->
+      Rrd.Inverse
+  | "identity" ->
+      Rrd.Identity
+  | _ ->
+      raise Rrd_protocol.Invalid_payload

ocaml/xcp-rrdd/lib/transport/base/rrd_rpc.mli

@@ -21,3 +21,5 @@ val assoc_opt : key:string -> default:string -> (string * Rpc.t) list -> string
 val ds_ty_of_string : string -> Rrd.ds_type
 
 val owner_of_string : string -> Rrd.ds_owner
+
+val transform_of_string : string -> Rrd.ds_transform_function

ocaml/xenopsd/xc/mem_stats.ml

@@ -291,7 +291,8 @@ let observe_stats l =
           | Rrd.VT_Unknown ->
               nan
         in
-        ds.Ds.ds_pdp_transform_function f |> Printf.sprintf "%.0f"
+        Rrd.apply_transform_function ds.Ds.ds_pdp_transform_function f
+        |> Printf.sprintf "%.0f"
     )
   in
   D.debug "stats header: %s" (String.concat "," names) ;