Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

$ as a separator in the task manifest causes panic #1550

Closed
katarzyna-z opened this issue Mar 15, 2017 · 3 comments
Closed

$ as a separator in the task manifest causes panic #1550

katarzyna-z opened this issue Mar 15, 2017 · 3 comments

Comments

@katarzyna-z
Copy link
Contributor

$ as a separator in the task manifest causes panic. Recovery mechanism works and Snap still works but unexpected error occurs.

Step to reproduce:

  1. Run Snap
  2. Load plugin
  3. Create a task manifest with separator set a $, for example:
{
  "version": 1,
  "schedule": {
    "type": "simple",
    "interval": "1s"
  },
  "workflow": {
    "collect": {
      "metrics": {
        "$intel$procfs$cpu$*$active_jiffies": {}
      }
    }
  }
}

Current state:

snaptel task create -t task.json 
Using task manifest to create task
Error creating task:Unknown API response: invalid character 'P' looking for beginning of value

 Received: PANIC: runtime error: index out of range
goroutine 49 [running]:
github.com/intelsdi-x/snap/vendor/github.com/urfave/negroni.(*Recovery).ServeHTTP.func1(0x7f6134e0b110, 0xc4201c8f40, 0xc4203059a0)
	/home/kujawka/.gvm/pkgsets/go1.7/global/src/github.com/intelsdi-x/snap/vendor/github.com/urfave/negroni/recovery.go:34 +0xf2
panic(0xad9be0, 0xc420010120)
	/home/kujawka/.gvm/gos/go1.7/src/runtime/panic.go:458 +0x243
github.com/intelsdi-x/snap/control.(*mttNode).search(0xc4201b3440, 0xf67298, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xf67298, 0x0)
	/home/kujawka/.gvm/pkgsets/go1.7/global/src/github.com/intelsdi-x/snap/control/mttrie.go:296 +0x180
github.com/intelsdi-x/snap/control.(*mttNode).GetMetrics(0xc4201b3440, 0x0, 0x0, 0x0, 0x0, 0x0, 0xa2b000, 0x8, 0xc19ae0, 0x4)
	/home/kujawka/.gvm/pkgsets/go1.7/global/src/github.com/intelsdi-x/snap/control/mttrie.go:201 +0xc4
github.com/intelsdi-x/snap/control.(*metricCatalog).GetMetrics(0xc4201bcd80, 0xf67298, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/home/ku

Expected state:
Panic does not exist. Error message is returned.

@rashmigottipati
Copy link
Contributor

Great catch, @katarzyna-z . Is it in the scope of #1551 to handle illegal namespace separators other than $ in the task manifest?

@katarzyna-z
Copy link
Contributor Author

#1551 is to protect against index out of range error. Other errors connected with namespace separator are returned by unmarshalling, for $ namespace was empty and Snap wasn't prepared for this case. $ in task manifest indicates env variables. If variables are not set then namespace is empty.

@rashmigottipati
Copy link
Contributor

Thanks for clarifying @katarzyna-z

katarzyna-z added a commit that referenced this issue Mar 20, 2017
Fixes #1550, checking number of namespace elements in requested metric
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants