vimeo / graph-explorer

A graphite dashboard powered by structured metrics
http://vimeo.github.io/graph-explorer/
Apache License 2.0
1.06k stars 93 forks source link

Graphing JS errors when metric contains a ":" #39

Closed rfshelley79 closed 11 years ago

rfshelley79 commented 11 years ago

The generated JS that's built at graph time uses the metric name as part of the variables, and if the metric name contains special characters (in our case, a ":"), the JS will throw an error.

Full eval'ed JS below, but here's the line from it that errors:

var graph_flot_SomeApi::someAPI__gauge__app2 = $.extend({}, defaults, graph_data);

Full JS

        $(document).ready(function () {
        var graph_data = {"promoted_constants": {"cluster": "jphx1", "app": "fulfill", "resource": "APIHandler__handleGetStuff", "plugin": "jetson"}, "from": "-24hours", "until": "now", "constants": {"what": "SomeApi::someAPI", "target_type": "gauge", "server": "app2"}, "targets": [{"variables": {"type": "requests", "unit": "requests/m"}, "id": "localhost__handleGetStuff.SomeApi::someAPI.measurements", "target": "localhost__handleGetStuff.SomeApi::someAPI.measurements"}, {"variables": {"type": "min", "unit": "ms"}, "id": "localhost__handleGetStuff.SomeApi::someAPI.min", "target": "localhost__handleGetStuff.SomeApi::someAPI.min"}]};
        graph_data['constants_all'] = jQuery.extend({}, graph_data['constants'], graph_data['promoted_constants']);

        $("#service::someAPI__gauge__app2").html(get_graph_name("SomeApi::someAPI__gauge__app2", graph_data));
        vtitle = get_vtitle(graph_data);
        if (vtitle != "") {
            graph_data["vtitle"] = vtitle;
        }

        // interactive legend elements -> use labelFormatter (specifying name: '<a href..>foo</a>' doesn't work)
        // but this function only sees the label and series, so any extra data must be encoded in the label
        labelFormatter = function(label, series) {
            var data = JSON.parse(label);
            if(data.name) {
                // name attribute is already set. this is probably a predefined graph, not generated from targets
                return data.name;
            }
            name = "";
            // at some point, we'll probably want to order the variables; just like how we compose graph titles.
            $.map(data["variables"], function (v,k) { name += " " + display_tag(k, v);});
            // there's nothing about this target that's not already in the graph title
            if (name == "") {
                name = "empty";
            }
            return get_inspect_url(data, name);
        }
        $.map(graph_data['targets'], function (v,k) {
            v["name"] = JSON.stringify(v, null, 2);
        });
        var defaults = {
            graphite_url: "http://localhost:9000/render/",
            from: "-24hours",
            until: "now",
            height: "300",
            width: "740",
            line_stack_toggle: 'line_stack_form_flot_SomeApi::someAPI__gauge__app2',
            series: {stack: true, lines: { show: true, lineWidth: 0, fill: true }},
            legend: {container: '#legend_flot_SomeApi::someAPI__gauge__app2', noColumns: 1, labelFormatter: labelFormatter },
            hover_details: true,
            zoneFileBasePath: '../timeserieswidget/tz',
            tz: "America/New_York",
        };
        var graph_flot_SomeApi::someAPI__gauge__app2 = $.extend({}, defaults, graph_data);
        $("#chart_flot_SomeApi::someAPI__gauge__app2").graphiteFlot(graph_flot_SomeApi::someAPI__gauge__app2, function(err) { console.log(err); });
        //$("#chart_flot_SomeApi::someAPI__gauge__app2").graphiteHighcharts(graph_flot_SomeApi::someAPI__gauge__app2, function(err) { console.log(err); });
        // TODO: error callback should actually show the errors in the html, something like:
        // function(err) { $("#chart_flot_SomeApi::someAPI__gauge__app2").append('<span class="label label-important">' + err + '</span>'); }
    });
rfshelley79 commented 11 years ago

Updating snippet.graph.tpl with the following helps (other values may cause problems as well, however, may want to sanitize further):

%graph_id = graphkey.replace('.','').replace('-','').replace(' ','').replace(':','')

Dieterbe commented 11 years ago

ok, will fix. just curious, what do you use ':' for?

Dieterbe commented 11 years ago

fixed in f687505 properly shield graph_id from disallowed characters

just curious, what do you use ':' for?