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

Snaptel does not support task creation with other separator than "/" #1477

Closed
skonefal opened this issue Jan 18, 2017 · 5 comments
Closed

Snaptel does not support task creation with other separator than "/" #1477

skonefal opened this issue Jan 18, 2017 · 5 comments

Comments

@skonefal
Copy link
Contributor

skonefal commented Jan 18, 2017

I have a metric with slashes and I cannot gather it via task manifest creation (my extension of perf plugin: http://bit.ly/2jwF3OZ).

My task manifest:

---
  version: 1
  schedule:
    type: "simple"
    interval: "1s"
  workflow:
    collect:
      metrics:  
        |intel|linux|perfevents|cgroup|intel_cqm/llc_occupacy/|docker:62167f2c9f0b62068c3e0d09ee3275190ae7e41230e9861e324621442bbda222: {}
      publish:
        (...)

Output:

$ ./snaptel task create -t workflow.yaml 
Using task manifest to create task
Error parsing YAML file input - error converting YAML to JSON: yaml: line 10: did not find expected comment or line break

Expected output:

  • It works

My metric looks like:

$ ./snaptel metric list
(...)
|intel|linux|perfevents|cgroup|intel_cqm/llc_occupacy/|docker
|intel|linux|perfevents|cgroup|intel_cqm/llc_occupacy/|docker:62167f2c9f0b62068c3e0d09ee3275190ae7e41230e9861e324621442bbda222

Snap Versions:

$ ./snapteld -version
snapteld version 1.0.0
$ ./snaptel -version
snaptel version 1.0.0

The problem also manifests when I change well known metric like /intel/psutil/vm/used to |intel|psutil|vm|used.
Also, this feature lack tests.

@kindermoumoute
Copy link
Contributor

I would guess the issue happens because you are using pipe as a separator. In YAML, pipe has a special behavior. If you change to one of the following namespace it should work:

# add quotes to the namespace
"|intel|linux|perfevents|cgroup|intel_cqm/llc_occupacy/|docker:62167f2c9f0b62068c3e0d09ee3275190ae7e41230e9861e324621442bbda222": {}

# use a different separator
\intel\namespace\/path/test\plop: {}

@IzabellaRaulin
Copy link
Contributor

IzabellaRaulin commented Jan 18, 2017

I talked with @skonefal and he is ok with defining his task manifest as a JSON file. Also, changing a separator to sth different than pipe is the another possibility which resolves that.

Currently, for a metric which contains a slash, snaptel metric list presents this metric with a pipe as a separator:

$ ./snaptel metric list
|intel|linux|perfevents|cgroup|intel_cqm/llc_occupacy/|docker

This suggests that such metric might be copied to a task manifest (json or yaml) as a metric to collect. Based on that does not work with YAML, changing these pipes in the output of snaptel metric list to other characters might be taken into consideration or even put some note in Snap's docs. @kindermoumoute, what do you think about it?

@kindermoumoute
Copy link
Contributor

kindermoumoute commented Jan 18, 2017

Maybe we should move the pipe out of the priority list (or move it on the right). And we can document separator good practices somewhere. Optionally, we could investigate and make a list of characters we can't use as separator because of YAML or JSON standard. Another one that comes in the top of my head, we can't use $ as a separator (will be interpolated).

@IzabellaRaulin
Copy link
Contributor

IzabellaRaulin commented Jan 19, 2017

@kindermoumoute, I added the label type/documentation-needed to highlight that some work related to documentation should be also done to resolve the issue.

@mbbroberg
Copy link
Contributor

@IzabellaRaulin I really like that use of the label, but not how we've used it to date. The first type/ label used before tracked generates issues for us to follow up on. We'll have to change our GH/J sync if we want to introduce that behavior.

You'll need to open another issue for docs and label that one type/documentation-needed in order for it to be assigned as work. 👍

katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 15, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 15, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 15, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 15, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 15, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit to katarzyna-z/snap that referenced this issue Mar 16, 2017
…ters as a namespace separators in the task manifest
katarzyna-z added a commit that referenced this issue Mar 20, 2017
Fixes #1477, Added information about not recommended characters as a namespace separators in the task manifest
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

5 participants