{"action":"opened","issue":{"url":"https://api.github.com/repos/munin-monitoring/munin/issues/394","labels_url":"https://api.github.com/repos/munin-monitoring/munin/issues/394/labels{/name}","comments_url":"https://api.github.com/repos/munin-monitoring/munin/issues/394/comments","events_url":"https://api.github.com/repos/munin-monitoring/munin/issues/394/events","html_url":"https://github.com/munin-monitoring/munin/issues/394","id":66112666,"number":394,"title":"munin: calculates multigraph field widths wrongly","user":{"login":"ssm","id":18237,"avatar_url":"https://avatars.githubusercontent.com/u/18237?v=3","gravatar_id":"","url":"https://api.github.com/users/ssm","html_url":"https://github.com/ssm","followers_url":"https://api.github.com/users/ssm/followers","following_url":"https://api.github.com/users/ssm/following{/other_user}","gists_url":"https://api.github.com/users/ssm/gists{/gist_id}","starred_url":"https://api.github.com/users/ssm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ssm/subscriptions","organizations_url":"https://api.github.com/users/ssm/orgs","repos_url":"https://api.github.com/users/ssm/repos","events_url":"https://api.github.com/users/ssm/events{/privacy}","received_events_url":"https://api.github.com/users/ssm/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/munin-monitoring/munin/labels/bug","name":"bug","color":"fc2929"},{"url":"https://api.github.com/repos/munin-monitoring/munin/labels/component:%20master","name":"component: master","color":"fad8c7"}],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/munin-monitoring/munin/milestones/3","html_url":"https://github.com/munin-monitoring/munin/milestones/2.0.26","labels_url":"https://api.github.com/repos/munin-monitoring/munin/milestones/3/labels","id":830541,"number":3,"title":"2.0.26","description":"Next bugfix release, only used in the unlikely case we are not totally free of bugs yet.","creator":{"login":"ssm","id":18237,"avatar_url":"https://avatars.githubusercontent.com/u/18237?v=3","gravatar_id":"","url":"https://api.github.com/users/ssm","html_url":"https://github.com/ssm","followers_url":"https://api.github.com/users/ssm/followers","following_url":"https://api.github.com/users/ssm/following{/other_user}","gists_url":"https://api.github.com/users/ssm/gists{/gist_id}","starred_url":"https://api.github.com/users/ssm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ssm/subscriptions","organizations_url":"https://api.github.com/users/ssm/orgs","repos_url":"https://api.github.com/users/ssm/repos","events_url":"https://api.github.com/users/ssm/events{/privacy}","received_events_url":"https://api.github.com/users/ssm/received_events","type":"User","site_admin":false},"open_issues":4,"closed_issues":0,"state":"open","created_at":"2014-10-17T18:01:20Z","updated_at":"2015-04-03T09:05:35Z","due_on":null,"closed_at":null},"comments":0,"created_at":"2015-04-03T09:05:35Z","updated_at":"2015-04-03T09:05:35Z","closed_at":null,"body":"Imported from https://bugs.debian.org/781790\r\n\r\n\r\nSource: munin\r\nVersion: 2.0.25-1~bpo70+1\r\nSeverity: normal\r\n\r\nHi,\r\n\r\nSo I finally got annoyed enough by this to dig into the source enough\r\nto make a reasonable bug report about it :)  I've been writing a\r\nmultigraph plugin for a new package, and I'd seen that sometimes it\r\nwould make the field label far wider than the text was, crowding the\r\nCur/Min/Avg/Max values hard to the right, in the worst cases even\r\nwrapping them to a new line, but just generally rendering them horribly.\r\n\r\nI've now figured out where it's getting the wrong field width from at\r\nleast, and there's probably a couple of easy fixes for that, but there\r\nmight be something subtle I'm missing, so we should probably run this\r\npast upstream rather than me just attaching a naive patch for it ...\r\n\r\nThe (simplified) problem case looks something like this:\r\n\r\n    multigraph foo\r\n    foo_field.label Foo\r\n\r\n    multigraph foo.bar\r\n    bar_long_descriptive_field_name.label Bar\r\n\r\nWith the result being that when the top level 'foo' graph is generated,\r\nthe width of the \"Foo\" label, rather than being 3, will instead be\r\nstrlen(\"bar_long_descriptive_field_name\") ...\r\n\r\n\r\nThe reason for this is that in the GraphOld.pm process_service() function\r\nwe have:\r\n\r\n    $max_field_len = munin_get_max_label_length($service, \\@field_order);\r\n\r\nand @field_order for the foo graph also contains all the fields for the\r\nfoo.bar subgraph (and any others it may also have).\r\n\r\n\r\nThe munin_get_max_label_length() function (in Utils.pm), in turn does\r\nthis:\r\n\r\n    my $len = length (munin_get ($service->{$f}, \"label\") || $f);\r\n\r\nand so when it processes the bar_long_descriptive_field_name field,\r\nthe $service hash doesn't contain that member, $len instead becomes\r\nthe length of the field name.  Hilarity ensues.\r\n\r\n\r\nAs far as I can gather so far, the reason for the '|| $f' clause there\r\nis to support use of munin_get_max_label_length in munin_field_status()\r\n(also in Utils.pm) - so basically we have two (unintended?) side effects\r\nthat collide to make this behave badly.\r\n\r\nIn process_service, the subgraph fields are skipped from further\r\nprocessing for the top level graph later in the loop, but the damage to\r\n$len is already done.\r\n\r\n\r\nThis could be fixed by either removing those fields before passing the\r\narray to munin_get_max_label_length, or by ignoring fields where\r\n$service->{$f} is undefined in that function when computing the length.\r\n(but I think the latter might break the munin_field_status() use).\r\nOr this might also be an indication of some other bug in the multigraph\r\nhandling that they even in that array in the first place ...\r\n\r\n\r\nThat should hopefully be enough for someone familiar with how this was\r\noriginally intended to work to see the right place to really fix this.\r\nAFAICS, the current upstream git HEAD doesn't look significantly\r\ndifferent to the code in 2.0.25-1~bpo70+1 -- so hopefully any fix will\r\nalso be backportable to that brance too :)\r\n\r\n  Cheers,\r\n  Ron\r\n"}}