hawkular / hawkular-openshift-agent

A Hawkular feed that collects metrics from Prometheus and/or Jolokia endpoints deployed in one or more pods within an OpenShift node.
16 stars 21 forks source link

Added ability to convert multiple prometheus labeled datapoints to h-… #106

Closed garethahealy closed 7 years ago

garethahealy commented 7 years ago

…metrics based on id replacement

105

jmazzitelli commented 7 years ago

Question - what happens if we have this:

some_metric_name{label1="foo",label2="gorilla"} 1.11
some_metric_name{label1="wot",label2="gorilla"} 2.22

but we only use one of those in the ID:

id: some_metric_name_${label1}

There is this label2 that isn't referenced in the ID. In the example above, they are the same value ("gorilla") but may be different in some cases. What happens in either case?

garethahealy commented 7 years ago

@jmazzitelli I don't fully understand your question, but will attempt to answer... If you had HOSA defined as:

id: some_metric_name_${label1}

Collecting metrics:

some_metric_name{label1="foo",label2="gorilla"} 1.11
some_metric_name{label1="wot",label2="gorilla"} 2.22

Then, HOSA would create two metrics as:

some_metric_name_foo = 1.11
some_metric_name_wot = 2.22

I still add all labels as tags onto the metric datapoints, so any datapoints with 'label2:gorilla' could be queried (if H-Metrics supports)

If your question is related to, "what happens if we duplicate the IDs by mistake?" i.e.:

some_metric_name_gorilla = 1.11
some_metric_name_gorilla = 2.22

Then we are in the same state as today. HOSA would create 2 metrics, with a datapoint each for the same timestamp, which when viewed via grafana, would give you: some_metric_name_gorilla = 3.33

* NOTE: Not tested this, as not got a dataset with that example. But can try this week if thats a worry.

jmazzitelli commented 7 years ago

This PR does not look complete. It is missing a fix to the func CollectMetricDetails - this is what is called when the agent wants to create metric definitions in h-metrics. This method needs to build the names of the new metrics that CollectMetric is storing.

jmazzitelli commented 7 years ago

@garethahealy see https://github.com/jmazzitelli/hawkular-openshift-agent/commit/15ad2342da85e152034deb7236358426345dd6d4 for changes I think need to be made to the CollectMetricDetails (some of those changes are trivial - due to running gofmt)

I haven't run or tested any of this, just prototyping

jmazzitelli commented 7 years ago

also need to fix the metric_storage.go method consumeMetricDefinitions. This is the place where metric defs are stored in h-metrics - and it tries to determine if one already exists and needs to be updated, or if one exists but needs to be purged. Now that the "id" field is not longer static but now requires actual data from the prometheus endpoint to complete it, this needs some refactoring.

I'm seeing a metric def where the ${x} is replaced with an empty string, but has the appropriate h-metric tags, but then all the metric defs with ${x} replaced properly has no h-metric tags. I think it has to do with the way that consumeMetricDefinitions is now broken.

jmazzitelli commented 7 years ago

And another thing I just realized that this breaks - the declared ID can have other ${x} tokens (like ${POD:name} or other allowed token). Those are not replaced properly.

I'm looking into implementing this a different way. I may take this opportunity to get the system to declare metric definitions a little better, too (as it is now, if the endpoint isn't up when the agent starts, the agent never creates metric definitions for that endpoint. I'll see if that can get corrected along the way).

jmazzitelli commented 7 years ago

this PR is probably going to be rejected and closed in favor of PR #117 once I determine that PR is actually working and good for merging.

jmazzitelli commented 7 years ago

This PR's implementation broke several things. PR #117 supersedes this PR - thus I'm closing this PR as obsolete. See PR #117 for the implementation of this feature.