Namespace not escaped when constructing in rest client

[lmroz]

Ofer Nagar reported this issue on our slack channel his exact result attached

The problem is caused by code constructing fetch request in rest client

    r := &GetMetricsResult{}
    q := fmt.Sprintf("/metrics?ns=%s&ver=%d", ns, ver)
    resp, err := c.do("GET", q, ContentTypeJSON)

While on server side when reading query values:

    q := r.URL.Query()
    v := q.Get("ver")
    ns_query := q.Get("ns")

Error on unescaping is eaten by golangs function

   907  // Query parses RawQuery and returns the corresponding values.
   908  func (u *URL) Query() Values {
   909      v, _ := ParseQuery(u.RawQuery)
   910      return v
   911  }

[IzabellaRaulin]

To describe the issue on an example (based on what Ofer Nagar reported, see attachement I prepare the following steps how to reproduce that.

Steps to reproduce

1) Load collector plugin which exposes a metric containing "%" in namespace, for example snap-plugin-collector-iostat:

$ snaptel plugin load snap-plugin-collector-iostat

$ snaptel metric list
NAMESPACE                                        VERSIONS
/intel/linux/iostat/avg-cpu/%idle                5
/intel/linux/iostat/device/*/%util               5
/intel/linux/iostat/device/*/await               5
/intel/linux/iostat/device/*/r_await             5
[...]

2) (Righ behaviour) Use metric list command <metric_name> for metric which namespace does NOT contain "%" character

a) command: list

$ snaptel metric list -m /intel/linux/iostat/device/*/await
NAMESPACE                                VERSIONS
/intel/linux/iostat/device/*/await       5

b) command: get

$ snaptel metric get -m /intel/linux/iostat/device/*/await
NAMESPACE                                        VERSION         UNIT    LAST ADVERTISED TIME                    DESCRIPTION
/intel/linux/iostat/device/[device_id]/await     5                       Tue, 25 Apr 2017 18:25:24 CEST          dynamic device metric: await

  Dynamic elements of namespace: /intel/linux/iostat/device/[device_id]/await

       NAME              DESCRIPTION
       device_id         Device ID

  Rules for collecting /intel/linux/iostat/device/[device_id]/await:

       NAME      TYPE    DEFAULT         REQUIRED        MINIMUM         MAXIMUM

So, in this case, it works as expected

3) (Wrong behaviour) Use metric list command <metric_name> for metric which namespace contains "%" character a) command: list

snaptel metric list -m /intel/linux/iostat/avg-cpu/%idle
NAMESPACE                                        VERSIONS
/intel/linux/iostat/avg-cpu/%idle                5
/intel/linux/iostat/avg-cpu/%iowait              5
/intel/linux/iostat/avg-cpu/%nice                5
/intel/linux/iostat/avg-cpu/%steal               5
/intel/linux/iostat/avg-cpu/%system              5
/intel/linux/iostat/avg-cpu/%user                5
/intel/linux/iostat/device/*/%util               5
/intel/linux/iostat/device/*/avgqu-sz            5
/intel/linux/iostat/device/*/avgrq-sz            5
/intel/linux/iostat/device/*/await               5
/intel/linux/iostat/device/*/r_await             5
/intel/linux/iostat/device/*/r_per_sec           5
/intel/linux/iostat/device/*/rkB_per_sec         5
/intel/linux/iostat/device/*/rrqm_per_sec        5
/intel/linux/iostat/device/*/svctm               5
/intel/linux/iostat/device/*/w_await             5
/intel/linux/iostat/device/*/w_per_sec           5
/intel/linux/iostat/device/*/wkB_per_sec         5
/intel/linux/iostat/device/*/wrqm_per_sec        5

All available metrics are listed, hovewer expected behaviour is listing only this one which was requested.

b) command: get Too long output to paste here, but it exactly the same effect as in point 3a) - there is info about all available metrics in the output, not only for this one which was requested.

Expected behaviour is that returned output should contain only info about requested metric /intel/linux/iostat/avg-cpu/%idle.

Other remarks

snaptel metric get <command> seems to be very useful to get details about config policy, last advertised time, etc. snaptel metric list <command> seems to be unuseful (I cannot image any use-case for having that)

It would be nice to write-down some use-cases for snaptel metric list <command> (if they exist). If this command is useless, deprecation and removing it should be considered in the future.