Display custom metrics for pods if they are available. #1109
Conversation
mwringe
force-pushed
the
custom-metrics
branch
2 times, most recently
from
a0ae698 to
b899219
Compare
| angular.forEach(response, function(metric) { | ||
|
|
||
| // set the label to the description if specified | ||
| var label = (metric.description !== undefined) ? metric.description : metric.name; |
There was a problem hiding this comment.
var label = metric.description || metric.name;| var label = (metric.description !== undefined) ? metric.description : metric.name; | ||
|
|
||
| // get the unit value if specified | ||
| var unit = (metric.unit !== undefined) ? metric.unit : "unknown"; |
There was a problem hiding this comment.
var unit = metric.unit || "unknown";But maybe the empty string is better if no unit set?
var unit = metric.unit || "";| }; | ||
| metrics.push(metric); | ||
| }); | ||
| return metrics; |
| description: value.tags.description, | ||
| }; | ||
| metrics.push(metric); | ||
| }); |
There was a problem hiding this comment.
Suggest using _.map. (Untested, but I think this will work.)
return _.map(response.data, function(value) {
return {
name: value.tags.metric_name,
unit: value.tags.units,
description: value.tags.description
};
});| var metric = { | ||
| name: value.tags.metric_name, | ||
| unit: value.tags.units, | ||
| description: value.tags.description, |
mwringe
force-pushed
the
custom-metrics
branch
from
b899219 to
70e3742
Compare
|
@spadgett Ok, I made the requested changes to the code. Custom metrics is something we want for 1.5, we are hoping to have something in the console to display these metrics. If we can not get it the console at this point in time, we will look to create a custom console extension or rely on something like grafana to display them. |
|
@mwringe can you add a flag to disable custom metrics to constants.js? something like |
|
@spadgett do we just want a flag to be added and the rest kept the same? Or a flag to be added and the drop down to still be used instead? |
|
my opinion: flag added and the rest kept the same. @jwforres let me know if you disagree |
|
yeah based on @spadgett 's assessment about the risk of introducing the
dropdown at this point in the release I agree
…On Mon, Jan 16, 2017 at 3:06 PM, Sam Padgett ***@***.***> wrote:
my opinion: flag added and the rest kept the same.
@jwforres <https://github.com/jwforres> let me know if you disagree
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1109 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABZk7ZGRK1L6-XNGqutortfMiI_RFv5Bks5rS83fgaJpZM4LiRhu>
.
|
mwringe
force-pushed
the
custom-metrics
branch
from
70e3742 to
497ce0a
Compare
|
Update to include a flag to enable/disable custom metrics. I also fixed an issue I found where we were not properly fetching counter based metrics and fixed a potential issue with encoding the metric name for the chart prefix. |
|
|
||
| // note: the prefix needs to follow selector rules | ||
| // converting to base64 and cleaning up the special /,+,= characters | ||
| var prefix = btoa(metric.name).replace(/\+/g,"_62_").replace(/\//g,"_63_").replace(/\=/g,""); |
There was a problem hiding this comment.
Suggest this instead for the prefix:
var prefix = _.uniqueId('custom-metric-');There was a problem hiding this comment.
awesome, that will work
| @@ -50,7 +51,12 @@ angular.module("openshiftConsole") | |||
|
|
|||
| function getRequestURL(config) { | |||
| return getMetricsURL().then(function(metricsURL) { | |||
| var template = metricsURL + POD_GAUGE_TEMPLATE; | |||
| var template; | |||
| if (config.type !== undefined && config.type === "counter") { | |||
There was a problem hiding this comment.
Maybe just
if (config.type === 'counter') { ... }although probably worth a comment saying gauge is assumed when type is not set
| var url = metricsURL + "/metrics"; | ||
|
|
||
| var params = { | ||
| tags: "custom_metric:true,pod_name:" + podName |
There was a problem hiding this comment.
Missed this on first review, but should we use pod UID instead in case a pod with the same name is created twice? (For pods that don't have generated names.)
There was a problem hiding this comment.
oh, good catch. Thanks. I will fix that.
mwringe
force-pushed
the
custom-metrics
branch
from
497ce0a to
31e7597
Compare
|
Ok, updated with the requested changes. |
|
[merge] |
|
Evaluated for origin web console merge up to 31e7597 |
|
Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/927/) (Base Commit: 695c74d) |
The Hawkular OpenShift Agent is capable of gathering a few custom metrics from pods which are deployed.
This PR will allow these pods to be displayed under the pod's metric page