Our users want to see bugs which are resolved, verified and closed on the bug chart report. Thus I have modified 'reports.cgi' to show numbers of all bugs. You might not want to include CLOSED counts as this would just increase over time. I am submitting the patch with the changes. Note: There were other changes in the diff which were not applicable to this bug. I removed those, but I do not know if the line number changes that this removal caused will make this diff file invalid to 'patch' utility. In short, you might want to double check, before committing.

I also included the patch (forgot the first time) to collect stats for the other bugs states. This patch worked fine at our site when used with an existing data/mining bug stats file. The new data items were merely appended to the line and the charting script (after complaining) charted them. Note: The code does not gaurantee that the order of the values will be compatible with the old code. It just so happens that they were listed the same was as in @::legal_bug_status.

This would be possible with bug #16009, general summary charts.

tara@tequilarista.org is the new owner of Bugzilla and Bonsai. (For details, see my posting in netscape.public.mozilla.webtools, news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)

Huh, well this bug is old enough we'll have to see whether patches have bit-rotted.

Have at it, kemosabe

the concept is sound, but i'm not sure that sliding off all of the state data into a flat text file is necessarily a good idea. also, if you get a database/installation as large as mozilla.org's, it's going to get murderous. what might be better is to make this an optional. if you turn on "save state data" on, it then shoves the state log info into a table that is then querable by some other cgi. accepting this bug. we should create a new features system first before attacking this bug

Notes and comments about charting (IMHO): I believe cyeh(?)'s comment above concerning the size of the data file is not a significant issue and is far outweighed by the advantages of collecting all of the bug statuses. Currently, I believe, Bugzilla collects a date and three statuses. I am only suggestion adding the other 4 statuses. This would only add 16 bytes of data per day. The overriding advantage of recording these values (even if you do not display them in the charts) is the statistical history it gives you. There are perhaps other means of getting this statistical information with bug activity table, but this is an easy "ready-to-use" format. Since this bug we have removed CLOSED bugs from the charts as it has keeps "going up and messing up" the chart scale. We do, however, continue to collect the count each day. If you would like me to recreate the patch with the current CVS version of Bugzilla, please let me know. I would be happy to. Mark Hamby

if could re-apply to the latest version in cvs, that would be great. i thought about what you said, and i agree that the impact isn't as big as i thought it would be. once you update your patches to the tip, i can apply them and do some testing. thanks.

I am adding patch against Mozilla CVS version as of yesterday. Patch contains the changes listed below. I have tried to describe them so you can determine which which you want to apply. * collectstats.pl - Now collects count stats on all bug statuses instead of only on 'NEW', 'ASSIGNED', and 'REOPENED'. Data collected whether it is used in reports.cgi or not. reports.cgi - Reads all status counts from data file. (Actually this code could/should be improved to remove status hardcoding via 'qw' and get statuses from '@@::legal_bug_status' the way that collectstats.pl does.) * collectstats.pl - Data filename modified to replace ' ' with '_'. (This corrects an unrelated problem with spaces in product names.) * reports.cgi - UNCONFIRMED status added to all querys from database. (As far as I can tell, UNCONFIRMED status should be added, but was overlooked.) * reports.cgi - Added UNCONFIRMED, RESOLVED, and VERIFIED status graphs to chart. (Not sure how many of these should be on "standard" chart. We use them, but I do not know how generally useful they are. A parameter to allow configuration of which statuses are graphed would probably be the best approach.) * reports.cgi - Added UNCONFIRMED and RESOLVED to Bug Count report tables. (This may also not be generally used. We like to see the RESOLVED bug counts to track how many have still not been VERIFIED. Again, a parameter to allow configuration would "be good".) A few more comments: (Everyone always has two more cents. :-) Please do not let my earlier statements discourage you from moving the collection statistics from a flat file to the database. I heartily agree that the database is a much better place to put the information. Reporting and analysis would be much easier if it were in the database. Parameters should probably be added to configure what statuses are charted. The chart can be confusing with too many statused plotted. New versions of GD allow plot lines to use dotted and dashed line styles, I believe. If anyone knows how toeasily use that feature, it would make the chart more readible, especially when adding additional status plots. This becomes particularly true when printed to a B&W laser printer. Short of that, perhaps the line color choices should be examined. I'll stop gabbing now. Patch follows. Mark

it's going to take me a moment to get this patch in. i don't have a test setup with a big enough data pool.

Chris, Tara - if this patch is difficult to add for some reason, I have a five-line interim patch to add UNCONFIRMED plotting to bugzilla.mozilla.org, which (as I mentioned in n.p.m.webtools) I would really like to see included as soon as possible. Is there any chance of that? It's a one-line backwardly-compatible change to collectstats.pl, and four obvious lines in reports.cgi. And a fetching shade of purple ;-) I will, of course, have to figure out how to install and use CVS on Unix, check out the latest sources, and then unlock the secrets of CVS diff if I actually want to produce patch files for it, but that's secondary... Gerv

patch now being testing on http://landfill.bluemartini.com

Chris - it aint working right now ;-) A query - what's the view on moving nightly collected info into the database? I'm doing the dupes stuff right now, and using a single file for each day's data (using Perl DBM). But I could change it, I suppose... Also, when I've finished this, any chance of testing it on landfill? Gerv

comitted to trunk

Chris - should we be seeing this on bugzilla.mozilla.org? 'Cos we ain't :-) Gerv

the installation of bugzilla at bugzilla.mozilla.org is maintained by endico@mozilla.org. checkin's to the trunk are not reflected at mozilla.org until they elect to update to the tip.

Why is b.m.o showing Resolved, Verified and _Closed_ (but no Unconfirmed) on the chart legend, but not the data? I would expect Bugzilla not to put something on the legend which is not actually being recorded ...

So, is this still an open issue?

It's charting everything now; but this means the smaller figures (which used to occupy the whole chart) now get compressed. We really need on/off switches for each parameter... Gerv

Oh, and it still doesn't do UNCONFIRMED - the one I _really_ want. Gerv

At the moment it looks to me like 30000 bugs are CLOSED. That can't be right. Either the colours of Closed/Verified are way too close to each other, or it's producing the wrong statistic.

If you're going to use on/off switches, pls make sure it's user specifiable.

Sounds like this is not quite fixed yet. Not ALL bug states coming up. Reopening. Also, there are currently 483 bugs set to Closed. So something is still wrong :-(

I've noticed that since this change filtered into my installation, that all of my bugs that used to be ASSIGNED are suddenly showing up as REOPENED on the chart. I have no REOPENED bugs on my system right now.

Please, please do *not* include the VERIFIED bugs as long as there is no option to turn it off. This makes the complete chart useless. Unconfirmed bugs would be much more useful. Currently the chart on mozilla.org seems to be completely buggy. :(

I suspect this gets out of alignment when you use the UNCONFIRMED status. I think everyone is going to want to chart something different. Best just to record everything and let the user specify.

Don--I think a general go over of all the chart stuff is in order. Can you take a look at this as well as 44653 (since I don't think we can't test that one until this one is fixed?)

while we're at it, we might want to examine the charting prerequisites, because it appears that charting is just screwed up right now. the latest version of the GD library is 1.8.3 the latest version of the GD perl module is 1.30. I'm no longer sure the latest Chart::Base works with this combo, since I tried this on landfill earlier (trying to get landfill to a state where I can look at this bug) and I was getting errors about png and gif routines not being available.

I've got a very simple charting patch that would be against a version where charting worked (i.e. the last stable release) and adds support for charting UNCONFIRMED bugs. Seeing as charting is currently stuffed, what say we roll things back to where they were in the last release, and I have a look at where I put that patch? Then, maybe, we could work on adding more general support... Gerv

charting is working again on landfill, if you could find your patch, we can toss it on landfill and retest this.

You mean, remove the current patch we are testing for this problem and use mine instead? Gerv

Yes, please attach your patch for this so we can evaluate it on landfill. Best patch wins. :)

Due to a stupid tree deletion, I had to rewrite this, so I did it better. I've attached my patch to fix this bug. It allows charting of any number at once of the following states: NEW, ASSIGNED, REOPENED, FIXED, INVALID, WONTFIX, LATER, REMIND, DUPLICATE, WORKSFORME. The FIXED, INVALID etc. totals are sums of those which are RESOLVED, VERIFIED and CLOSED. This seemed like a good balance between configurability and storing data in a too-fine-grained manner. There are several hacks in this patch I'd like to get resolved before it's checked in (although it can be tested on landfill now): 1) Because we can now have different graphs for the same product, I came across a problem where I had to press shift-reload to get the browser to pick up the new graph for the second and subsequent graphing attempts, as the filename was always the same. To get around this I put the time() in the filename. However, this is not ideal if several people are graphing at once, as it only has a 1-second resolution. What we really want is a guaranteed-unique temp file name. I know this is possible in C - how is this done in Perl? Second, I modified collectstats.pl to clear up all the old graphs. This isn't optimal, but we can't really remove them in reports.cgi either. Solutions welcomed. Thirdly, I couldn't work out how to get multiple values out of a $FORM::{'fred'} -style form variable - they seemed to come out as one long string - so I added colons to the <option name="fred:"> HTML as a separator. This is a bit nasty, and the Right Way to do this is also required. I think that'll do for now :-) Gerv

Tara, Don, Chris - can one of you review and then landfill this, perhaps? :-) Gerv

Apologies in advance for this (hopefully constructive) criticism... Please do not apply Gerv's patch as is, as it introduces a gaping security hole through an abuse of eval() in the last hunk. It takes user input data and includes it straight in the eval() unchecked, which means anyone can run arbitrary code on the webserver. Very bad. This is not really the place for an in-depth Perl style lesson, but I'll try to summarise quickly: You hardly ever need to use eval(). In this case, what you were trying to do should have been done with symbolic references instead, except for the fact that symbolic references also should only be used in rare circumstances, and this isn't one of them. Really, you should have been using a hash, so you could have done something like push @labels, $data{$chartme};

Of course, a hash is the solution. I did try using references, but couldn't work out how to turn strict refs off. Thanks for the review :-) I'll make another attempt at this after I've moved this weekend. Is the patch OK otherwise? Gerv

The patch looks OK otherwise, yes, modulo the bits below. Addressing your three concerns: 1) Caching of images is a sticky issue. There are actually two aspects to it - how you cache images on the server (currently this is not done, so a graph gets generated every hit, which is very inefficient), and how you stop browsers from doing unwanted. Fortunately, in the case of bugzilla daily report charts, the answer is simple - include the date, but /not/ the time, in the image filename. Then reports.cgi only has to check whether the file exists for the current day. If so, simply return the HTML pointing to it. If not, generate the new one. 2) Once we implement the above scheme, I would say that your patch to make collectstats.pl remove all cached graphs for previous days /is/ optimal, so let's keep that. 3) Presumably you mean $::FORM{'fred'} not $FORM::{'fred'}, but anyway, this is all part of the divine retribution hurled down on all bugzilla developers ever since the horror which is CGI.pl was invented ;-) Earlier yesterday, before I discovered the existence of this bug, I was trying to fix bugzilla's charting of the various bug states. Problem number one was that the format of the data files changed mid-file when I upgraded our local bugzilla from 2.8 a while back. I got round this problem by writing a script which would "upgrade" the older-format lines in the data file to the new format. I'll attach the script below, and request that it be added into contrib/, or even better, merged into checksetup.pl. <mini_rant> If you change the data formats internal to bugzilla (in this case the daily stats data file) or accept a patch from someone, please don't just break backwards compatability without any warning. </mini_rant> Secondly, I addressed this bug directly myself, and decided that it would be far more sensible NOT to hard-code the various bug states/resolutions into . Maybe I'll have time in the next few hours to merge my patch with Gerv's (mine didn't add graphing of resolutions, nor did it offer choice as to what should be graphed). Watch this space ...

One more thing I forgot to mention: the whitespace in reports.cgi is an unholy mess. Despite the excellent intentions showed by setting indent-tabs-mode to nil at the top of the file, there are tabs galore in the file, and there is also evidence of the file being edited by people with different coding styles and editors with varied setups of hard tab width. IIRC this applies to more of the bugzilla code than just this one file ... bonsai too, probably? One of the golden rules of shared code development is to standardise on one style, and then make sure everyone sticks to it. Bugzilla hasn't done that, which makes reviewing the code and patches much harder. Ideally one of the core developers would just go over the entire code-base converting tabs to the appropriate number of spaces. Arguments as to why this is a good thing are here: http://www.jwz.org/doc/tabs-vs-spaces.html http://www.new.ox.ac.uk/~adam/computing/why_no_tabs.html

If you aren't going to hard-code the bug states/resolutions, you might as well move the data into the database and implement my "next generation" plan to allow graphing of the results of any Bugzilla search query ;-) I think the plan here is to get it working, and possibly upgraded, for 2.12 but leave the cutting-edge and major changes to the 3.0 "rewrite". At least, that's as I understand it. Do fix up the tabs, though... Gerv

OK, this is now done. I didn't move the data into a database yet as it's a fair bit more work (would need to change checksetup etc.) However, this patch should aid such a transition if/when someone does it. This patch is a merge of mine and the reports.cgi of Gerv's -- it only changes reports.cgi, applies direct to current Mozilla CVS. The bits of Gerv's which patch collectstats.pl look good and should work great alongside this; however I would strongly recommend that collectstats.pl *doesn't* get patched until you have some kind of automated way of upgrading the data files to the new format, preferably incorporated into checksetup.pl. Here's what this patch does to reports.cgi: - fixed security hole in Gerv's patch - add menu for choosing which resolutions/statuses you want to plot - derive the choice of resolutions/statuses from the -All- data file - removed hardcoded order of resolutions/statuses when reading from daily stats file, so now works independently of collectstats.pl version - added image caching by date and datasets, to give the poor servers a rest ;-) - made conform better to bugzilla code style conventions - removed use strict related brokenness ($::FORM) - handle blank data points correctly Can someone up the severity of this bug from 'enhancement'? It appears that charting on bugzilla.mozilla.org is screwed, and I reckon (famous last words) that my patch stands a good chance of fixing it. Anyway, here it comes ...

Although I'm not certain that the stuffed-ness on b.m.o would be fixed by this, it's worth a go :-) CCing endico for assessment. Gerv

> If you aren't going to hard-code the bug states/resolutions, you > might as well move the data into the database and implement my "next > generation" plan to allow graphing of the results of any Bugzilla > search query ;-) I'm not sure if you're serious here, due to the smiley and since this is going to be really hard, if indeed we have all the version information necessary in the database (and I suspect we don't) and the performance would be feasible (and I suspect it can't be). However this is actually filed as bug #16009 if you're interested.

The plan is simple - I've outlined it in another bug, but I can't remember exactly where. You allow the administrator to create/delete any number of lines in a "graph" table, with the following columns: 1) an SQL "count" query, probably created with the assistance of query.cgi 2) A "display name" for this query (for the graphing page) 3) An enabled/disabled flag 4)...X) The results of said query run at midnight for each date. You allow the admin to remove all date columns which are older than a given date, if they want to, and perhaps automatically remove rows which no longer contain any data. The graphing interface then lets you choose one or more of the rows to display, and does The Right Thing. This is definitely not hard to implement. I'd like to do this for Bugzilla 3.0. Gerv

Oh OK, not something the user can arbitrarily choose then. Sounds fine, but how does it interact with the desire to allow the user to choose what statuses/resolutions they display? Do you intend on dumping 7 (or whatever) values per query?

The user can choose between any of the queries the admin has set up. So, for mozilla.org, the first 10 queries would be the current (or rather, proposed) 10 of new, unconfirmed, assigned, duplicate etc. In this way, the new system would reproduce the functionality of the old. Then, there might be queries for nsbeta3+, next milestone or whatever. Gerv

Oh I see. So there's a query for NEW, etc., and you can graph any number of "queries" at the same time. Sounds like you should file a bug.

I'm not going to file a bug, because I think the above Spiers/Markham patch is perfectly sufficient for the moment (particularly considering we need to get b.m.o _working_ in the charting arena ASAP). However, it's on the list for Bugzilla 3.0. Anyway, back to business - can someone please give permission for me to apply this patch to landfill? Gerv

Okay... <phew> I'm about to attach the canonical version of the graphing patch, against CVS from a short time ago. This patch will fix bugzilla.mozilla.org's charting (I hope) if, before applying it, you run my tweaked version of Adam's fixing script over the data files. The script currently takes filename params and outputs to stdout, so may need to be wrapped or tweaked to do large quantities of stats files at once. It puts a 0 if the data is not known. This script fixes the fact that someone checked in a non-backwardly-compatible change to what data was stored in the stats files, so they are internally inconsistent. It will update the files to match the version of collectstats.pl attached. reports.cgi will now read and do reports for any consistent data files. (fixstats.pl will make them consistent.) It caches graphs by including the names of the states charted in the filename of the graphic. collectstats.pl will now collect stats for the following (hard-coded, but that doesn't really matter) states: NEW|ASSIGNED|REOPENED|UNCONFIRMED|FIXED|INVALID|WONTFIX|LATER|REMIND|DUPLICATE|WORKSFORME|MOVED It also deletes all graphs, so they get regenerated every day they are asked for. I have tested this collection of patches with a local Bugzilla and a bugzilla.mozilla.org Browser stats file, and it seems to work fine. (I note we're not keeping a lid on UNCONFIRMED at the moment...) This patch is basically similar to the one Tara's already (I think) reviewed. If it needs to go up on Landfill first, fine, but let's get it banged on and then up on b.m.o. The attachment is a .tar.gz of three files - the fixing script, a diff -u and a diff -wu, which is half the size and a lot easier to review because it doesn't include whitespace changes. Gerv

All patches verified and commited

The patch for this is indeed in. However, we need to take fixstats.pl and graft it into checksetup.pl, so that people who update from 2.10 will not have their stats files made inconsistent. Basically, we have to run fixstats.pl, with the correct internal setup, over every file in data/mining (if they have just been using 2.10, there should be nothing else in there apart from stats files.) I've started work on this. Gerv

I applied these changes to bugzilla.mozilla.org and fixed the data files but bug charts still aren't working. For security, we've turned off access to the bugzilla/data directory which means that bugzilla/data/mining is also inaccessible and you can't load the gif images for the charts. Gerv says he'll find a new home for the gifs. I briefly turned on access to bugzilla/data and everything seemed to work fine other than wierd looking graphs because most of the fields are new and we haven't been collecting stats on them, or not for long.

Reopening as there's a little more to do :-) Attached is a .tar.gz of a one-line diff to fixstats.pl which puts "" instead of 0 if there's no data. This makes the graphs look much more sensible. Also, a diff to reports.cgi to put the graphs in a new "graphs" subdirectory (which needs creating manually) and collectstats.pl (deletes the graphs from somewhere else now.) Either Tara can check these last patches in and Dawn can repull, or you can apply them directly. Remember fixstats isn't in CVS - at some point, we are going to have to work out how to checksetup.pl a lot of this... checksetup.pl scares me a bit :-) These diffs are against CVS from 9.55pm GMT today. Gerv

I also used "" not 0 for empty data points, and found it generated warnings in Chart::Base. Attaching a small mbox mail folder containing my correspondence with the (ex-)maintainer of Chart::Base, which also contains a patch to fix the warnings. Quick summary: Chart is no longer maintained, and so we should be using GDGraph.

Are these warnings a problem (i.e. will they fill up any important logs)? If not, can we patch our local copy of Chart::Base with Adam's patch, and just leave things there for the minute? Long term, we'll move (in the next big charting upheaval) but that will almost certainly be post-2.12. Gerv

Is this bug major, or can it be set to normal?

It's major because charting on bugzilla.mozilla.org is currently broken. Gerv

Tara - the Bugzilla-core parts of patch http://bugzilla.mozilla.org/showattachment.cgi?attach_id=18667 need checking in. They are very tiny - they just move the directory the graphs are generated in from data/mining to graphs. As long as the Bugzilla 2.12 tarball contains a graphs directory, we should then be fine. Gerv

Gerv can check in his own damn changes now. :)

I've checked in the little patches. All we need now is for checksetup.pl to create the graphs dir and convert the data file formats. I'll get to that later this week. Gerv

Is there an ETA when charting on b.m.o is supposed to work again? Or can someone publish the unaccessible data/mining/-All- and -All-.gif files somewhere else?

Only Dawn knows when she's going to update b.m.o to the tip (which should re-enable charting). I suspect she plans to do that when Tara blesses 2.12, but I could be wrong. Dawn? Gerv

i'll update when dmose has gotten his anti-spam code in. (a week or so?)

I have now checked in all the patches given in this bug to the tip. All that is needed now is for some testing to happen. Basically, we need an installation of Bugzilla with some serious data running the tip code, so we can bang on reports.cgi and see what happens. Like landfill :-) Also, it would be good for someone to code review (again) the code in checksetup.pl for updating old data files, as that's a bit more difficult to test. Adam? It's basically what we were using before. Having some data files to upgrade with said code would be handy. I have one from bugzilla.mozilla.org, but a couple from landfill would be handy too... Any ETA on having it back up, Tara? Gerv

OK... so this doesn't quite work yet. <sigh> Gerv

*ahem* -- dude, don't forget to get your code reviewed before checkins. As for landfill, some people have offered to help lock it down security-wise, but it's hard getting all the schedules coordinated. I'll do my best to come up with something as soon as possible. Very sorry about the inconvenience.

The code I checked in had already been reviewed and tested in its standalone form, so I thought it wasn't a problem. But fair enough. I checked in (twice, I think) to checksetup.pl, if you want to see what I did. Sadly, something seems to have broken with reports.cgi :-( Working on it... Gerv

OK, I've checked in what I hope is the last set of fixes. Important Note: Anyone upgrading to the tip from 2.10 or an earlier tip needs to run checksetup.pl to get their graph data files in order. I've tried to accommodate the wierd configs they could get into for people running tip code, but I might not have caught all of them, so please, check your data/mining/* charting data files for consistency and sanity after running checksetup.pl. Gerv

Okay, I had a look and put it up on landfill. It upgraded cleanly, it generated charts accurately... looks good to me. Closing out again unless somebody screams (landfill's dataset is not exactly large so what it does at mozilla.org is up in the air)... Thanks for the great work Gerv!

*** Bug 38631 has been marked as a duplicate of this bug. ***

Sorry for the spam, but I needed to be able to query for all of these correctly.

i updated to the tip and ran checksetup.pl. the reports page was broken. There was no submit button on the page and some garbled error messages. "There is no graph available for" (incomplete sentence) "Couldn't read daily statistics file" The problem was fixed by running collectstats.pl. I believe that collectstats.pl should run this for you or at least warn you to do it yourself.

Dawn: you mean checksetup.pl should run collectstats.pl? I think the right fix here is for reports.cgi to cope better when there's no data. I've run it with no data before... I will investigate. Gerv

yes, that's what i meant. I don't actually know that its the correct fix. I suggested it because empirical evididence suggests it is. Before I ran collectstats the reports page was broken. Afterwards, it was fixed. Do whatever's right tho.

checked in patch from gervase.markham@univ.ox.ac.uk (Gervase Markham)

Verified this is present on bugzilla.mozilla.org.

Moving closed bugs to Bugzilla product

The chart possibility is great, but is there a way to show the graphs so it won't show every day, but every week or other period? I mean - the new bugs and the bugs that have been marked as assigned on the 09/09/2001, before on the 09/02/2001, and so on?

Roni, you may want to change the "generate_charts" sub in reports.cgi: http://lxr.mozilla.org/mozilla/source/webtools/bugzilla/reports.cgi#542

I don't quite understand your question, but no, I don't think so. A better place to ask would be news.mozilla.org/netscape.public.mozilla.webtools. Gerv