change label seperator from default comma to options

docs/storage-schema.md

@@ -52,10 +52,34 @@ Heapster tags each metric with the following labels.
 | container_name | User-provided name of the container or full cgroup name for system containers |
 | host_id        | Cloud-provider specified or user specified Identifier of a node               | 
 | hostname       | Hostname where the container ran                                              | 
-| labels         | Comma-separated list of user-provided labels. Format is 'key:value'           |
+| labels         | Comma-separated(Default) list of user-provided labels. Format is 'key:value'  |
 | namespace_id   | UID of the namespace of a Pod                                                 |
 | resource_id    | An unique identifier used to differentiate multiple metrics of the same type. e.x. Fs partitions under filesystem/usage | 
 
+**Note**
+  * Label seperator can be configured with Heapster `--label-seperator`. Comma-seperated label pairs is fine until we use [Bosun](http://bosun.org) as alert system and use `group by labels` to search for labels.
+    [Bosun(0.5.0) uses comma to split queried tag key and tag value](https://github.com/bosun-monitor/bosun/blob/0.5.0/opentsdb/tsdb.go#L566-L575). For example if the expression used for query InfluxDB from Bosun is like this:
+```
+$limit = avg(influx("k8s", '''SELECT mean(value) as value FROM "memory/limit" WHERE type = 'node' GROUP BY nodename, labels''', "${INTERVAL}s", "", ""))
+```
+With a comma-separated labels:
+```
+nodename=127.0.0.1,labels=beta.kubernetes.io/arch:amd64,beta.kubernetes.io/os:linux,kubernetes.io/hostname:127.0.0.1
+```
+When split by a comma, something wrong happened. Bosun split it wrongly to:
+```
+nodename=127.0.0.1
+labels=labels:beta.kubernetes.io/arch:amd64
+beta.kubernetes.io/os.linux
+kubernetes.io/hostname:127.0.0.1
+```
+Last two tag key-value pairs is wrong. They should not exist and be squashed to `labels`:
+```
+nodename=127.0.0.1
+labels=labels:beta.kubernetes.io/arch:amd64,beta.kubernetes.io/os.linux,kubernetes.io/hostname:127.0.0.1
+```
+This will make bosun confused and panic with something like "panic: opentsdb: bad tag: beta.kubernetes.io/os:linux".
+
 ## Aggregates
 
 The metrics are collected initally collected for nodes and containers and latter aggregated for pods, namespaces and clusters. 

metrics/heapster.go

@@ -41,6 +41,7 @@ import (
 	"k8s.io/heapster/metrics/sinks"
 	"k8s.io/heapster/metrics/sinks/metric"
 	"k8s.io/heapster/metrics/sources"
+	"k8s.io/heapster/metrics/util"
 	"k8s.io/heapster/version"
 	kube_api "k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/client/cache"
@@ -65,6 +66,7 @@ func main() {
 	logs.InitLogs()
 	defer logs.FlushLogs()
 
+	setLabelSeperator(opt)
 	setMaxProcs(opt)
 	glog.Infof(strings.Join(os.Args, " "))
 	glog.Infof("Heapster version %v", version.HeapsterVersion)
@@ -356,3 +358,7 @@ func setMaxProcs(opt *options.HeapsterRunOptions) {
 		glog.Warningf("Specified max procs of %d but using %d", numProcs, actualNumProcs)
 	}
 }
+
+func setLabelSeperator(opt *options.HeapsterRunOptions) {
+	util.SetLabelSeperator(opt.LabelSeperator)
+}

metrics/options/options.go

@@ -38,6 +38,7 @@ type HeapsterRunOptions struct {
 	Sinks            flags.Uris
 	HistoricalSource string
 	Version          bool
+	LabelSeperator   string
 }
 
 func NewHeapsterRunOptions() *HeapsterRunOptions {
@@ -67,4 +68,5 @@ func (h *HeapsterRunOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&h.AllowedUsers, "allowed_users", "", "comma-separated list of allowed users")
 	fs.StringVar(&h.HistoricalSource, "historical_source", "", "which source type to use for the historical API (should be exactly the same as one of the sink URIs), or empty to disable the historical API")
 	fs.BoolVar(&h.Version, "version", false, "print version info and exit")
+	fs.StringVar(&h.LabelSeperator, "label_seperator", ",", "seperator used for joining labels")
 }

metrics/processors/node_autoscaling_enricher.go

@@ -43,7 +43,7 @@ func (this *NodeAutoscalingEnricher) Process(batch *core.DataBatch) (*core.DataB
 	}
 	for _, node := range nodes.Items {
 		if metricSet, found := batch.MetricSets[core.NodeKey(node.Name)]; found {
-			metricSet.Labels[core.LabelLabels.Key] = util.LabelsToString(node.Labels, ",")
+			metricSet.Labels[core.LabelLabels.Key] = util.LabelsToString(node.Labels)
 			capacityCpu, _ := node.Status.Capacity[kube_api.ResourceCPU]
 			capacityMem, _ := node.Status.Capacity[kube_api.ResourceMemory]
 			allocatableCpu, _ := node.Status.Allocatable[kube_api.ResourceCPU]

metrics/processors/pod_based_enricher.go

@@ -87,7 +87,7 @@ func addContainerInfo(key string, containerMs *core.MetricSet, pod *kube_api.Pod
 	}
 
 	containerMs.Labels[core.LabelPodId.Key] = string(pod.UID)
-	containerMs.Labels[core.LabelLabels.Key] = util.LabelsToString(pod.Labels, ",")
+	containerMs.Labels[core.LabelLabels.Key] = util.LabelsToString(pod.Labels)
 
 	namespace := containerMs.Labels[core.LabelNamespaceName.Key]
 	podName := containerMs.Labels[core.LabelPodName.Key]
@@ -120,7 +120,7 @@ func addPodInfo(key string, podMs *core.MetricSet, pod *kube_api.Pod, batch *cor
 
 	// Add UID to pod
 	podMs.Labels[core.LabelPodId.Key] = string(pod.UID)
-	podMs.Labels[core.LabelLabels.Key] = util.LabelsToString(pod.Labels, ",")
+	podMs.Labels[core.LabelLabels.Key] = util.LabelsToString(pod.Labels)
 
 	// Add cpu/mem requests and limits to containers
 	for _, container := range pod.Spec.Containers {
@@ -138,7 +138,7 @@ func addPodInfo(key string, podMs *core.MetricSet, pod *kube_api.Pod, batch *cor
 						core.LabelContainerName.Key:      container.Name,
 						core.LabelContainerBaseImage.Key: container.Image,
 						core.LabelPodId.Key:              string(pod.UID),
-						core.LabelLabels.Key:             util.LabelsToString(pod.Labels, ","),
+						core.LabelLabels.Key:             util.LabelsToString(pod.Labels),
 						core.LabelNodename.Key:           podMs.Labels[core.LabelNodename.Key],
 						core.LabelHostname.Key:           podMs.Labels[core.LabelHostname.Key],
 						core.LabelHostID.Key:             podMs.Labels[core.LabelHostID.Key],

metrics/util/util.go

@@ -21,16 +21,18 @@ import (
 	"time"
 )
 
-// Concatenates a map of labels into a comma-separated key=value pairs.
-func LabelsToString(labels map[string]string, separator string) string {
+var labelSeperator string
+
+// Concatenates a map of labels into a Seperator-seperated key:value pairs.
+func LabelsToString(labels map[string]string) string {
 	output := make([]string, 0, len(labels))
 	for key, value := range labels {
 		output = append(output, fmt.Sprintf("%s:%s", key, value))
 	}
 
 	// Sort to produce a stable output.
 	sort.Strings(output)
-	return strings.Join(output, separator)
+	return strings.Join(output, labelSeperator)
 }
 
 func CopyLabels(labels map[string]string) map[string]string {
@@ -47,3 +49,7 @@ func GetLatest(a, b time.Time) time.Time {
 	}
 	return b
 }
+
+func SetLabelSeperator(seperator string) {
+	labelSeperator = seperator
+}