Bug 296631 - Provide a tracing UI enablement page
Summary: Provide a tracing UI enablement page
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.8 M5   Edit
Assignee: PDE-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed, noteworthy
Depends on:
Blocks: 368780
  Show dependency tree
 
Reported: 2009-12-01 15:52 EST by Troy Bishop CLA
Modified: 2012-01-27 10:11 EST (History)
18 users (show)

See Also:


Attachments
Full JAR bundle (with source) providing the functionality mentioned above (153.16 KB, application/octet-stream)
2009-12-01 15:53 EST, Troy Bishop CLA
no flags Details
Example preference page when tracing is disabled (i.e. the default look) (82.25 KB, image/jpeg)
2009-12-01 15:55 EST, Troy Bishop CLA
no flags Details
Example preference page when tracing is enabled showing the JDT Core component expanded (110.46 KB, image/jpeg)
2009-12-01 15:56 EST, Troy Bishop CLA
no flags Details
Proposed extension point changes and some fixes (4.41 KB, patch)
2010-02-04 17:28 EST, Eduard Bartsch CLA
no flags Details | Diff
updated full JAR bundle (with source) (155.47 KB, application/octet-stream)
2010-02-10 13:54 EST, Troy Bishop CLA
no flags Details
updated full JAR bundle (with source) (99.03 KB, application/octet-stream)
2010-03-05 13:24 EST, Troy Bishop CLA
no flags Details
ZIP of org.eclipse.equinox.trace.ui project (106.56 KB, image/png)
2010-03-05 17:38 EST, Troy Bishop CLA
no flags Details
ZIP of org.eclipse.equinox.trace.ui project (106.56 KB, application/octet-stream)
2010-03-05 17:41 EST, Troy Bishop CLA
no flags Details
Fixes missing updates on change of debug options (1.42 KB, patch)
2012-01-27 10:05 EST, Christian Georgi CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Troy Bishop CLA 2009-12-01 15:52:27 EST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 (.NET CLR 3.5.30729)
Build Identifier: 

In Eclipse 3.5 (bug 258705) the OSGi layer introduced a dynamic tracing API.  I am opening this bug to see if a corresponding UI can be introduced (in 3.6) which allows an end-user of an Eclipse based product to enable tracing for any component that utilizes this tracing API.  The UI would need to do the following:

1) Allow an end user to enable or disable tracing without the need of the product restarting.
2) Allow an end user to specify the ".option" to enable (i.e. specify the trace string).
3) Be able to modify the trace repository (i.e. tracing file, maximum file size, and number of historical files to keep).

I am attaching a full bundle (source included) which provides this functionality as a workspace preference page ( Window > Preferences > General > Product Tracing).  The UI populates the available tracing options via an extension point called 'traceComponent' which each component owner would be responsible for extending.  The traceComponent extension point accepts a set of bundles (by providing the name - regular expressions are supported) and the UI will iterate over each of these bundles and read in the corresponding .option file to get a list of the available tracing strings for these components.  It is also possible for a component to "consume" a bundle, meaning that the bundle can only be visible in the specified component and is not available in any other component.  This allows an Eclipse-based product to customize their own logical components and have the ability to override what Eclipse may provide out of the box.

I will attach a couple of screenshots to show this preference page using a few standard Eclipse components.

I'm sure there are still many bugs with this code (I don't claim it to be "complete" at all - it works for the most part) but I wasn't sure how to proceed with including it in Eclipse, so I wanted to publish it to this bug and start this discussion.  I don't know if it would make sense to keep this as a stand-alone bundle or integrate it into an existing Eclipse bundle.  I also wasn't sure which bugzilla component to assign this to be figured platform UI would be a good place to start.

Please let me know if this is something that can be done and if so, how you wish to proceed with having it included.  Thank you!

Note: the attached ZIP file contains a few simple examples of how to provide the 'traceComponent' extension.

Reproducible: Always
Comment 1 Troy Bishop CLA 2009-12-01 15:53:58 EST
Created attachment 153534 [details]
Full JAR bundle (with source) providing the functionality mentioned above
Comment 2 Troy Bishop CLA 2009-12-01 15:55:00 EST
Created attachment 153535 [details]
Example preference page when tracing is disabled (i.e. the default look)
Comment 3 Troy Bishop CLA 2009-12-01 15:56:03 EST
Created attachment 153536 [details]
Example preference page when tracing is enabled showing the JDT Core component expanded
Comment 4 Troy Bishop CLA 2009-12-01 15:57:04 EST
Setting version to 3.6 as this is the version I am reporting this enhancement against.
Comment 5 Troy Bishop CLA 2010-01-20 14:03:41 EST
Hello, I just wanted to follow-up on this request to see if it is something that would even be considered for Eclipse 3.6?  Thank you.
Comment 6 Susan McCourt CLA 2010-01-20 14:20:50 EST
Hi, Troy.
Thanks for the starting this discussion (and including code!)  This is pretty cool.  

The question wrt to 3.6 is who has cycles to look this code over and what component should contribute the UI?

Tom, do you think this is a piece of equinox UI?  
Should I move this bug to you?  
I don't mind chiming in on usability questions but I don't have any cycles to take this on.  cc'ing Christopher since this might be really useful for serviceability.
Comment 7 Thomas Watson CLA 2010-01-20 14:59:49 EST
This feels more like something that should be included in PDE.  PDE already has some UI parts for displaying trace options based on the .option files included in the bundles.

I'm not sure I understand why the traceComponent extension point is needed.  If tracing is enabled it seems like the UI could be automatically populated with the information provided by the .options files contained in bundles similar to how PDE does it for configuring tracing for launch configuration.  Is this needed to allow better grouping of bundle options instead of one flat space of bundle symbolic names in the UI?  I would vote to have a UI more like the one provided by PDE.  Not necessarily because I think it is better but because I think folks are already used to the way PDE presents the trace options.

I don't think there is any good place to put this in Equinox.  We currently only have UI in p2 and a small amount in security.  So this would be a stand alone bundle in Equinox.  If possible I would prefer to have this in PDE.
Comment 8 Troy Bishop CLA 2010-01-20 15:38:05 EST
(In reply to comment #7)
> This feels more like something that should be included in PDE.  PDE already has
> some UI parts for displaying trace options based on the .option files included
> in the bundles.
> 
> I'm not sure I understand why the traceComponent extension point is needed.  If
> tracing is enabled it seems like the UI could be automatically populated with
> the information provided by the .options files contained in bundles similar to
> how PDE does it for configuring tracing for launch configuration.  Is this
> needed to allow better grouping of bundle options instead of one flat space of
> bundle symbolic names in the UI?  I would vote to have a UI more like the one
> provided by PDE.  Not necessarily because I think it is better but because I
> think folks are already used to the way PDE presents the trace options.
> 
> I don't think there is any good place to put this in Equinox.  We currently
> only have UI in p2 and a small amount in security.  So this would be a stand
> alone bundle in Equinox.  If possible I would prefer to have this in PDE.

You are correct for why the extension point is needed - simply to allow a development owner of a set of bundles (for a product) to have a logical grouping of their components trace strings (and any other bundles trace strings that they want to include for their component).  The purpose of this is also to remove the "technical" aspects of the product (i.e. bundle symbolic names) from the initial UI and present a name that is a little more friendly to an end-user.  All of the .option strings for a component can be enabled/disabled at once rather than a user having to (potentially) enable multiple options for (potentially) multiple bundles.
Comment 9 Susan McCourt CLA 2010-01-20 15:47:52 EST
Moving to PDE for now based on Tom's comments.
I agree that we should attempt to share code with the launch config/tracing UI that's already in place.
Comment 10 Darin Wright CLA 2010-01-20 16:13:26 EST
I would suggest that the tooling could try to populate the trace options based on the bundles that contain trace .options files (using vendor/bundle names as well, to avoid symbolic names). 

However, a large issue is the dynamic nature of the trace options. Many existing bundles analyze trace options once on startup to set corresponding DEBUG boolean flags. They don't check the trace options dynamically, and thus the tooling won't work for many bundles. (I suppose using an extension point would be a way for bundles that do support dynamic trace options to hook into this selectively).

For 3.6, we have not have cycles to allocate to this.
Comment 11 Thomas Watson CLA 2010-01-20 16:17:33 EST
I agree with Darin.

I'm getting the sense that this is a bit producty.  We could put some UI in PDE but I think that will be more geared towards developers instead of end users like you describe.  But without doing the work to make our own bundles listen to dynamic changes to the trace options, I am not sure it is worth putting this into Eclipse for 3.6.  Your product certainly could add the UI itself and display only the bundles you develop and know support dynamic trace enablement.
Comment 12 Troy Bishop CLA 2010-01-20 16:28:51 EST
(In reply to comment #10)
> I would suggest that the tooling could try to populate the trace options based
> on the bundles that contain trace .options files (using vendor/bundle names as
> well, to avoid symbolic names). 

That is what this UI functionality does now - you provide a list of bundles
names via the extension point and it will read the .options file from those
bundles and populate the tree based on this content.

> 
> However, a large issue is the dynamic nature of the trace options. Many
> existing bundles analyze trace options once on startup to set corresponding
> DEBUG boolean flags. They don't check the trace options dynamically, and thus
> the tooling won't work for many bundles. (I suppose using an extension point
> would be a way for bundles that do support dynamic trace options to hook into
> this selectively).
> 
> For 3.6, we have not have cycles to allocate to this.

Correct, the thought was that components would not implement the extension
point unless they had already updated their code to use the trace API.  The
purpose of this defect is not to pursue this (asking people to use the UI
without the underlying support) but rather to just see if we can get the UI in
place so that when components move to the trace API they have a mechanism to
test it.  In parallel to this request I will also be opening defects against
each Eclipse component to pursue converting their trace code to the new trace
API.
Comment 13 Troy Bishop CLA 2010-01-20 16:42:57 EST
(In reply to comment #11)
> I agree with Darin.
> 
> I'm getting the sense that this is a bit producty.  We could put some UI in PDE
> but I think that will be more geared towards developers instead of end users
> like you describe.  But without doing the work to make our own bundles listen
> to dynamic changes to the trace options, I am not sure it is worth putting this
> into Eclipse for 3.6.  Your product certainly could add the UI itself and
> display only the bundles you develop and know support dynamic trace enablement.

You are correct that this is "producty" - but any (that I can think of) Eclipse-based product could use it (and I'm sure Eclipse itself as well) - it's not just the product I work on.  This code could absolutely be put into the product I work on but any Eclipse product would need a way to trace the Eclipse code as well (and preferably dynamic to keep it consistent with all of the components we provide).  This means that all of the Eclipse code would need to use the new trace API that we worked on in 3.5.  However, without the UI in place (in Eclipse) there does not seem to be any incentive for the Eclipse code to move to use the dynamic trace API.
Comment 14 Eduard Bartsch CLA 2010-01-21 03:37:18 EST
Hi everybody,

I would like to see this UI as part of the platform. In fact, I was just about to contribute the same piece and was very happy to see that the work was already being done. Thanks, Troy. :-)

I am working on a big Eclise-Workbench-based project that implements different kind of tools that run on the Workbench as a product. It is very important for us to be able to trace in customer environments and switch the trace on and off dynamically. So I was very happy to see the dynamic tracing API for 3.5.

The project, I am working on, is not related to Java so that PDE and JDT are not included. Adding this UI to PDE would mean that we would either need to implement the same UI a second time or to include only this plugin with our installation (as well as other pieces of PDE this UI may rely upon in the future, but it is not the best way to go). 

In fact, without such a UI available to everybody there is not much incentive to use this API for doing tracing over the competition (java.util.Logging, Log4J, SLF4J etc). If possible, we would even like this UI plugin to be 3.5 compatible so that we can include it with our tools running on 3.5 IDE. 

The charme of Troy's work is that there is only one place the user must to go to configure tracing and there is only one place where the traces go to.

Best regards,
Ed
Comment 15 Eduard Bartsch CLA 2010-01-21 03:54:08 EST
Hi, Troy.

I have played around with the UI and like it (especiall that the logical categories are shown on the first level instead of technical bundle/feature names). 

I have two small issues:

First, when I try to search and type-in something into the search field, only the first and second levels are filtered. If there is a match on the third level (within /debug options), those options are not concidered.

Second, it is a bit confusing for me that /debug options are shown in a subtree but other options are not. Some plugins like org.eclipse.core.jobs are not compliant with the spec so that their trace settings are under different prefixes (/jobs for example) and you cann't switch all of them on and off with one checkbox.

I would rather prefer uniformity so that all options would be shown  hierarchically.
Comment 16 Thomas Watson CLA 2010-01-21 09:32:04 EST
We don't have cycles to work on this at the moment for 3.6.  

It looks like there is interest from the community.  I am a bit reluctant to put this in Equinox.  If it is to be used for general RCP apps then I have the feeling it belongs in UI somewhere, similar to how we moved the error log view from PDE to UI a while back.

This will not make M5.  M6 is getting a bit late to start adding completely new UIs.  Perhaps we should incubate this and get it in early in the next release?
Comment 17 Eduard Bartsch CLA 2010-01-22 05:23:56 EST
I have found another issue when playing around with the UI but this issue it probably is better handled on Equinox level. 

When I switch all the tracing off using the "Enable Tracing" checkbox without clearing the individual debug attributes, there is no notification on registered DebugOptionListeners and none of the methods optionsChanged() is called. 

The trace file is not written anymore because methods of DebugTrace class check for isDebugEnabled() that returns false. But the performance penalty remains. When the trace is switched on and then off, the overhead of preparing parameters and invoking the trace methods will remain until you clear all debug attributes or restart Eclipse.
Comment 18 Troy Bishop CLA 2010-01-25 10:49:58 EST
(In reply to comment #15)
> Hi, Troy.
> 
> I have played around with the UI and like it (especiall that the logical
> categories are shown on the first level instead of technical bundle/feature
> names). 
> 
> I have two small issues:
> 
> First, when I try to search and type-in something into the search field, only
> the first and second levels are filtered. If there is a match on the third
> level (within /debug options), those options are not concidered.
> 
> Second, it is a bit confusing for me that /debug options are shown in a subtree
> but other options are not. Some plugins like org.eclipse.core.jobs are not
> compliant with the spec so that their trace settings are under different
> prefixes (/jobs for example) and you cann't switch all of them on and off with
> one checkbox.
> 
> I would rather prefer uniformity so that all options would be shown 
> hierarchically.

Hello Eduard, Thanks for trying the UI out - any suggestions / improvements are greatly appreciated.  I'm sure there are more problems then what you've found :)  I have not fully tested everything yet.  I'll look into the items you mentioned and try to fix these problems.

As for the tree structure display - I'm not sure I see the problem with the org.eclipse.core.jobs bundle.  When I load it I see:

Eclipse Jobs
   - org.eclipse.core.jobs/jobs
        - org.eclipse.core.jobs/jobs/beginend
        - org.eclipse.core.jobs/jobs/errorondeadlock
        - org.eclipse.core.jobs/jobs/locks
        - org.eclipse.core.jobs/jobs/shutdown
        - org.eclipse.core.jobs/jobs/timing

so I can click on either the 'Eclipse Jobs' or 'org.eclipse.core.jobs/jobs' node to enable all of the debug options.  As for your suggestion, would you (using the example able) rather see:

Eclipse Jobs
   - org.eclipse.core.jobs/jobs
   - org.eclipse.core.jobs/jobs/beginend
   - org.eclipse.core.jobs/jobs/errorondeadlock
   - org.eclipse.core.jobs/jobs/locks
   - org.eclipse.core.jobs/jobs/shutdown
   - org.eclipse.core.jobs/jobs/timing
Comment 19 Troy Bishop CLA 2010-01-25 10:57:34 EST
(In reply to comment #16)
> We don't have cycles to work on this at the moment for 3.6.  
> 
> It looks like there is interest from the community.  I am a bit reluctant to
> put this in Equinox.  If it is to be used for general RCP apps then I have the
> feeling it belongs in UI somewhere, similar to how we moved the error log view
> from PDE to UI a while back.
> 
> This will not make M5.  M6 is getting a bit late to start adding completely new
> UIs.  Perhaps we should incubate this and get it in early in the next release?

Hello Tom,

As I understand (and please correct me if I am wrong), the M7 milestone is the final release for new features for 3.6.  Since this UI is not introducing new API as it is all internal (unless new extension points are considered API?) which means that there is still the remaining 4 days of M5 and all of M6 and M7 before the release stream would be frozen for accepting this type of request.  If this is correct would you still be willing to accept this function into 3.6 (also assuming that Equinox UI is the right component to own it)?  Thanks!
Comment 20 Troy Bishop CLA 2010-01-26 15:18:08 EST
(In reply to comment #17)
> I have found another issue when playing around with the UI but this issue it
> probably is better handled on Equinox level. 
> 
> When I switch all the tracing off using the "Enable Tracing" checkbox without
> clearing the individual debug attributes, there is no notification on
> registered DebugOptionListeners and none of the methods optionsChanged() is
> called. 
> 
> The trace file is not written anymore because methods of DebugTrace class check
> for isDebugEnabled() that returns false. But the performance penalty remains.
> When the trace is switched on and then off, the overhead of preparing
> parameters and invoking the trace methods will remain until you clear all debug
> attributes or restart Eclipse.

Thanks Eduard,  I've opened bug 300911 and attached a patch which should correct this problem.
Comment 21 Eduard Bartsch CLA 2010-02-01 11:31:28 EST
(In reply to comment #18)
> As for the tree structure display - I'm not sure I see the problem with the
> org.eclipse.core.jobs bundle.  When I load it I see:
> Eclipse Jobs
>    - org.eclipse.core.jobs/jobs
>         - org.eclipse.core.jobs/jobs/beginend
>         - org.eclipse.core.jobs/jobs/errorondeadlock
>         - org.eclipse.core.jobs/jobs/locks
>         - org.eclipse.core.jobs/jobs/shutdown
>         - org.eclipse.core.jobs/jobs/timing
> so I can click on either the 'Eclipse Jobs' or 'org.eclipse.core.jobs/jobs'
> node to enable all of the debug options.  As for your suggestion, would you
> (using the example able) rather see:
> Eclipse Jobs
>    - org.eclipse.core.jobs/jobs
>    - org.eclipse.core.jobs/jobs/beginend
>    - org.eclipse.core.jobs/jobs/errorondeadlock
>    - org.eclipse.core.jobs/jobs/locks
>    - org.eclipse.core.jobs/jobs/shutdown
>    - org.eclipse.core.jobs/jobs/timing
Hi Troy, you are right. The jobs plugin works as you have described it. I should have tried this before taking this plugin as example. 
But let's take another example you have as default in your setup: 
 - Equinox Framework
   - ...
   - org.eclipse.osgi/trace/activation
   - org.eclipse.osgi/trace/classLoading
   - ...
I would rather expect to see the following:
 - Equinox Framework
   - ...
   - org.eclipse.osgi/trace
      - org.eclipse.osgi/trace/activation
      - org.eclipse.osgi/trace/classLoading
   - ...

As far as I understand the implementation, the reason is that .options file lacks the entry: 
  org.eclipse.osgi/trace=false
If I add this line, the hierarchy will appear.

But this will not help one level deeper so that all other options are shown in a flat list like:
 - My Test Category
   - MyTestPlugin/trace
     - MyTestPlugin/trace/a
     - MyTestPlugin/trace/a/b
     - MyTestPlugin/trace/a/b/c
     - MyTestPlugin/trace/x
     - MyTestPlugin/trace/x/y

This means that I am not able to swith tracing on/off for all options alltogether that start with MyTestPlugin/trace/a. 

So my question now is whether this is the intended behavior?
Comment 22 Troy Bishop CLA 2010-02-04 15:57:13 EST
(In reply to comment #21)
> (In reply to comment #18)
> > As for the tree structure display - I'm not sure I see the problem with the
> > org.eclipse.core.jobs bundle.  When I load it I see:
> > Eclipse Jobs
> >    - org.eclipse.core.jobs/jobs
> >         - org.eclipse.core.jobs/jobs/beginend
> >         - org.eclipse.core.jobs/jobs/errorondeadlock
> >         - org.eclipse.core.jobs/jobs/locks
> >         - org.eclipse.core.jobs/jobs/shutdown
> >         - org.eclipse.core.jobs/jobs/timing
> > so I can click on either the 'Eclipse Jobs' or 'org.eclipse.core.jobs/jobs'
> > node to enable all of the debug options.  As for your suggestion, would you
> > (using the example able) rather see:
> > Eclipse Jobs
> >    - org.eclipse.core.jobs/jobs
> >    - org.eclipse.core.jobs/jobs/beginend
> >    - org.eclipse.core.jobs/jobs/errorondeadlock
> >    - org.eclipse.core.jobs/jobs/locks
> >    - org.eclipse.core.jobs/jobs/shutdown
> >    - org.eclipse.core.jobs/jobs/timing
> Hi Troy, you are right. The jobs plugin works as you have described it. I
> should have tried this before taking this plugin as example. 
> But let's take another example you have as default in your setup: 
>  - Equinox Framework
>    - ...
>    - org.eclipse.osgi/trace/activation
>    - org.eclipse.osgi/trace/classLoading
>    - ...
> I would rather expect to see the following:
>  - Equinox Framework
>    - ...
>    - org.eclipse.osgi/trace
>       - org.eclipse.osgi/trace/activation
>       - org.eclipse.osgi/trace/classLoading
>    - ...
> 
> As far as I understand the implementation, the reason is that .options file
> lacks the entry: 
>   org.eclipse.osgi/trace=false
> If I add this line, the hierarchy will appear.
> 
> But this will not help one level deeper so that all other options are shown in
> a flat list like:
>  - My Test Category
>    - MyTestPlugin/trace
>      - MyTestPlugin/trace/a
>      - MyTestPlugin/trace/a/b
>      - MyTestPlugin/trace/a/b/c
>      - MyTestPlugin/trace/x
>      - MyTestPlugin/trace/x/y
> 
> This means that I am not able to swith tracing on/off for all options
> alltogether that start with MyTestPlugin/trace/a. 
> 
> So my question now is whether this is the intended behavior?

Thanks Eduard, I see what you mean now.  I'm beginning to think that the nested tree structure isn't the best idea and having it flattened (like your example with 'My Test Category') would be the best solution. However, it would look like:

  - My Test Category
    - MyTestPlugin/trace
    - MyTestPlugin/trace/a
    - MyTestPlugin/trace/a/b
    - MyTestPlugin/trace/a/b/c
    - MyTestPlugin/trace/x
    - MyTestPlugin/trace/x/y

The only concern is that a component (i.e. JDT Core in my original example) would have a lot of entries for the one branch.  However, it would certainly help clean up a lot of the code since it would reduce a lot of the complexity... I never really like the implementation of it ;)

I've got the other items you mentioned fixed -  I'll look into changing this and attaching a new UI bundle as soon as I can.
Comment 23 Eduard Bartsch CLA 2010-02-04 17:25:47 EST
(In reply to comment #22)
> Thanks Eduard, I see what you mean now.  I'm beginning to think that the nested
> tree structure isn't the best idea and having it flattened (like your example
> with 'My Test Category') would be the best solution. However, it would look
> like:
>   - My Test Category
>     - MyTestPlugin/trace
>     - MyTestPlugin/trace/a
>     - MyTestPlugin/trace/a/b
>     - MyTestPlugin/trace/a/b/c
>     - MyTestPlugin/trace/x
>     - MyTestPlugin/trace/x/y
> The only concern is that a component (i.e. JDT Core in my original example)
> would have a lot of entries for the one branch.  However, it would certainly
> help clean up a lot of the code since it would reduce a lot of the
> complexity... I never really like the implementation of it ;)
> I've got the other items you mentioned fixed -  I'll look into changing this
> and attaching a new UI bundle as soon as I can.
I agree that the flat list will probably be less confusing than current partial hierarchy and closer to what we have already have on the tracing tab in PDE. 

Meanwhile, I played a bit more with the current version. There are some minor issues.

Restore Defaults doesn't restore changed values for non-boolean options until Eclipse is restarted. The reading ot .options files happens only once per Eclipse run and the original values are not kept. 

Restore Defaults doesn't switch debigging off.

The boolean values are not displayed but the edit field is not disabled.

I am looking forward to see the next version. I hope that the trace UI will make it into 3.6. I see that we are late but 3.7 is more than one year away.

I concider extension points as an API so it is important to get them straight.

I propose a slight change for the extension point definition to allow a situation where one bundle defines a category and other bundles contribute the debug options in a decentral manner. This can be done by making the category label optional and allowing zero min bundles.

I will attach a patch for the proposed extension point definition and with fixes for some of the issues mentioned above.
Comment 24 Eduard Bartsch CLA 2010-02-04 17:28:22 EST
Created attachment 158239 [details]
Proposed extension point changes and some fixes

Here is the proposed patch.
Comment 25 Thomas Watson CLA 2010-02-04 18:11:54 EST
Hi all, great work so far on getting the UI and function in shape.  Unfortunately the integration of this into the Eclipse SDK is not possible at this stage in 3.6.  One possibility we should consider is putting this into core tools which is not shipped as part of the SDK and is not considered API.  But it would allow others to use it and provide feedback for the next release.
Comment 26 Eduard Bartsch CLA 2010-02-05 04:43:42 EST
(In reply to comment #25)
> Hi all, great work so far on getting the UI and function in shape. 
> Unfortunately the integration of this into the Eclipse SDK is not possible at
> this stage in 3.6.  One possibility we should consider is putting this into
> core tools which is not shipped as part of the SDK and is not considered API. 
> But it would allow others to use it and provide feedback for the next release.

Hi all, this would be a good compromise for us. Otherwise, we would have to develop and ship the same thing with our product. :-(

Would it then be possible to make it compatible with 3.5? It actually runs fine on 3.5 (after patching the version number in MANIFEST.MF). The only thing is that the input fields for trace file max size and max count are useless (and can be hidden depending on runtime version).
Comment 27 Troy Bishop CLA 2010-02-05 10:06:07 EST
> I propose a slight change for the extension point definition to allow a
> situation where one bundle defines a category and other bundles contribute the
> debug options in a decentral manner. This can be done by making the category
> label optional and allowing zero min bundles.
> 
> I will attach a patch for the proposed extension point definition and with
> fixes for some of the issues mentioned above.

Thanks Eduard.  I've got all of your changes added but I wanted to discuss your last suggestion.  I don't understand why the label would need to be optional in this case? wouldn't you want the label set when the component is created?  I like your change which sets the minOccurs of the bundle attribute to 0 but I'm not so sure about the label change - when would the label ever get set?  Thanks again for your assistance.
Comment 28 Troy Bishop CLA 2010-02-05 10:14:36 EST
(In reply to comment #26)
> (In reply to comment #25)
> > Hi all, great work so far on getting the UI and function in shape. 
> > Unfortunately the integration of this into the Eclipse SDK is not possible at
> > this stage in 3.6.  One possibility we should consider is putting this into
> > core tools which is not shipped as part of the SDK and is not considered API. 
> > But it would allow others to use it and provide feedback for the next release.
> 
> Hi all, this would be a good compromise for us. Otherwise, we would have to
> develop and ship the same thing with our product. :-(
> 
> Would it then be possible to make it compatible with 3.5? It actually runs fine
> on 3.5 (after patching the version number in MANIFEST.MF). The only thing is
> that the input fields for trace file max size and max count are useless (and
> can be hidden depending on runtime version).

I agree, I think this is a good compromise (for the same reason that Eduard mentioned) given the situation.

Eduard, why is it that those two trace file options are useless on 3.5? Those properties do exist and are used by the trace API (see org.eclipse.osgi.framework.debug.EclipseDebugTrace#readLogProperties() and org.eclipse.osgi.framework.debug.EclipseDebugTrace#checkTraceFileSize() found in the org.eclipse.osgi bundle).  They are just system properties that the UI sets and the trace API reads.
Comment 29 Eduard Bartsch CLA 2010-02-08 04:53:07 EST
(In reply to comment #28)
> Eduard, why is it that those two trace file options are useless on 3.5? Those
> properties do exist and are used by the trace API (see
> org.eclipse.osgi.framework.debug.EclipseDebugTrace#readLogProperties() and
> org.eclipse.osgi.framework.debug.EclipseDebugTrace#checkTraceFileSize() found
> in the org.eclipse.osgi bundle).  They are just system properties that the UI
> sets and the trace API reads.
Hi Troy, I was misguided by the Eclipse help for Eclipse runtime options in 3.6:
  eclipse.trace.size.max NEW 
  eclipse.trace.backup.max NEW 
and wasn't careful enough to check the state of 3.5 (it was new already for 3.5). My fault. :-(

But it is even better that the trace UI can be used with 3.5 without any changes.n :-)
Comment 30 Eduard Bartsch CLA 2010-02-08 05:09:02 EST
(In reply to comment #27)
> I don't understand why the label would need to be optional in
> this case? wouldn't you want the label set when the component is created?  I
> like your change which sets the minOccurs of the bundle attribute to 0 but I'm
> not so sure about the label change - when would the label ever get set?  
Troy, my proposal is to allow following:

Let's suppose, bundle A creates an extension for trace component with id T1 and label 'XYZ' but without any bundles.

Bundle B needs to contribute itself to the above trace component so it will create an extension with trace component id T1 and {B} as list of bundles but without a trace component label (as proposed by me).

A mandatory label would force the bundle B to specify a label in a redundant way. Let's suppose, bundle B would specify 'ABC'. Depending, which extension is iterated over first, either 'ABC' or 'XYZ' will appear als label. So it is better to state that the label should be specified only once.

The change in TracingCaches.java proposed by my patch will set the label independently of the extension sequence as soon as it appears:
  // set the label if specified 
  if ( element.getAttribute 
         (TracingConstants.TRACING_EXTENSION_LABEL_ATTRIBUTE) != null ) { 
    component.setLabel(element.getAttribute         
         (TracingConstants.TRACING_EXTENSION_LABEL_ATTRIBUTE)); 
  }
Comment 31 Troy Bishop CLA 2010-02-10 13:54:46 EST
Created attachment 158757 [details]
updated full JAR bundle (with source)

Here's the latest code (included in the attached bundle) which should correct all of the previously mentioned problems.  As a result of bug 300911 a change was made which is not optimal:

Tracing is forced on to get the current state of the debug options (since they are only accessing when tracing is on now) and the off again once the list is populated.  This means that all listeners are notified twice as a result of just populating the page (once to say tracing is enabled, once to say tracing is disabled).

I think the solution to this problems is to ask for the tracing api to have an additional API:

DebugOptions#setDebugEnabled(boolean value, boolean notify);

If notify is set to false then enabling or disabling tracing will not cause notifications to be sent.  This API would be used solely (as far as I can tell now) by the tracing UI.

Another suggestion would be to expose the 'disabledOptions' as API so that they can be accessed or modified. (see bug 300911 for more info on this topic).


As well, there is another non-optimal situation which I spotted (it has always been there):

When you change a specific set of debug options then each change (either by selecting a new or unselecting an existing option) causes a notification to be sent.  This means that a bundle contributing more than one option could be notified more than once.  Ideally it would be nice to just notify the bundle once to tell it to re-assign all of its tracing flag(s).

I think the best solution for this is to ask for an API to do batch updates of options, i.e.

DebugOptions#setOptions(Properties optionsToAdd);
DebugOptions#removeOptions(Properties optionsToRemove);

the implementation in FrameworkDebugOptions would then be able to set the options and notify only the bundles that are affected the one time.
Comment 32 Thomas Watson CLA 2010-02-11 09:48:00 EST
(In reply to comment #31)
> I think the best solution for this is to ask for an API to do batch updates of
> options, i.e.
> 
> DebugOptions#setOptions(Properties optionsToAdd);
> DebugOptions#removeOptions(Properties optionsToRemove);
> 
> the implementation in FrameworkDebugOptions would then be able to set the
> options and notify only the bundles that are affected the one time.

I opened bug 302586 with a slightly different suggestion.
Comment 33 Troy Bishop CLA 2010-03-05 13:01:26 EST
(In reply to comment #25)
> Hi all, great work so far on getting the UI and function in shape. 
> Unfortunately the integration of this into the Eclipse SDK is not possible at
> this stage in 3.6.  One possibility we should consider is putting this into
> core tools which is not shipped as part of the SDK and is not considered API. 
> But it would allow others to use it and provide feedback for the next release.

Hi Tom,

Do you know how I can proceed (or who I could speak with) to look into your suggestion of incorporating this UI into the core tools?
Comment 34 Troy Bishop CLA 2010-03-05 13:24:03 EST
Created attachment 161156 [details]
updated full JAR bundle (with source)

This patch utilizes the new API that Tom added via bug 302586 to address the concerns raised in comment 31.  This means that this JAR now depends on Eclipse 3.6 since the API added will only exist in 3.6+.  I also corrected a small problem with the build.properties which caused the classes files to be exported twice so this JAR is a lot smaller.
Comment 35 Thomas Watson CLA 2010-03-05 14:44:34 EST
(In reply to comment #33)
> 
> Hi Tom,
> 
> Do you know how I can proceed (or who I could speak with) to look into your
> suggestion of incorporating this UI into the core tools?

I have thought about this a bit more.  I don't think core tools is the best place for this.  Core tools has lots of other dependencies (such as jdt) that we likely don't want to force on folks that want this tracing UI.  I suggest we add the bundle to the equinox incubator under the components subfolder:

CVS repo: dev.eclipse.org:/cvsroot/rt
subfolder: org.eclipse.equinox/incubator/components/bundles/

I think the project should be renamed to org.eclipse.equinox.trace.ui.  Troy could you rename the bundle?  I can then get the project released and start building it as part of the equinox incubator build.  How does that sound?
Comment 36 Troy Bishop CLA 2010-03-05 17:38:22 EST
Created attachment 161203 [details]
ZIP of org.eclipse.equinox.trace.ui project

Here's a ZIP of the project which can be imported into a workspace via the File > Import > General > Existing Projects into Workspace wizard.
Comment 37 Troy Bishop CLA 2010-03-05 17:39:37 EST
(In reply to comment #35)
> (In reply to comment #33)
> > 
> > Hi Tom,
> > 
> > Do you know how I can proceed (or who I could speak with) to look into your
> > suggestion of incorporating this UI into the core tools?
> 
> I have thought about this a bit more.  I don't think core tools is the best
> place for this.  Core tools has lots of other dependencies (such as jdt) that
> we likely don't want to force on folks that want this tracing UI.  I suggest we
> add the bundle to the equinox incubator under the components subfolder:
> 
> CVS repo: dev.eclipse.org:/cvsroot/rt
> subfolder: org.eclipse.equinox/incubator/components/bundles/
> 
> I think the project should be renamed to org.eclipse.equinox.trace.ui.  Troy
> could you rename the bundle?  I can then get the project released and start
> building it as part of the equinox incubator build.  How does that sound?

Sounds good Tom, thanks a lot!  The attached ZIP (org.eclipse.equinox.trace.ui.zip) has the project renamed to org.eclipse.equinox.trace.ui and is ready for initial check-in.
Comment 38 Troy Bishop CLA 2010-03-05 17:41:06 EST
Created attachment 161204 [details]
ZIP of org.eclipse.equinox.trace.ui project

sorry, I selected the wrong content type for the last attachment...
Comment 39 Troy Bishop CLA 2010-03-18 17:13:09 EDT
Tom,

One of the problems I'm facing now with the tracing support is when you restart a product.  If you enable tracing and enable an option-path then tracing works for the duration of that product session; However, if I restart the product then that information is effectively lost until the tracing UI is loaded and OK is pressed again.  I'd like to try and find a better solution to this problem but I hit a road block which I think you may be able to help.

Currently in the constructor of org.eclipse.osgi.framework.debug.FrameworkDebugOptions the following is done:

****
String debugOptionsFilename = FrameworkProperties.getProperty(OSGI_DEBUG);
if(debugOptionsFilename == null)
   return;
****

This is why tracing is disabled when restarting a product... The product argument -debug (the value of OSGI_DEBUG) is never set so tracing is never enabled.  What I would like to do in FrameworkDebugOptions is something like the following:

****
String debugOptionsFilename = FrameworkProperties.getProperty(OSGI_DEBUG);
if(debugOptionsFilename == null) {
   // tracing could still be enabled if the preference file says it is.
}
****

I think (correct me if I am wrong) the problem is that there is no concept of preferences for the org.eclipse.osgi bundle (since all of that logic resides in org.eclipse.equinox.preferences - i.e. where IEclipsePreference resides).  Do you have any other suggestion for how I can store this type of state data that the FrameworkDebugOptions class could utilize?  i.e. the tracing UI would write information to a file showing the state of tracing (i.e. if it's enabled, options that are enabled, etc) and then the FrameworkDebugOptions could use this information during initialization.
Comment 40 Thomas Watson CLA 2010-03-18 17:46:40 EDT
You could have a bundle that reads the preferences on startup and uses the methods

DebugOptions.setOptions(options)
DebugOptions.setDebugEnabled(true)

With the options gotten out of the saved prerferences, right?  I am not for adding some persistence of this data into the framework.  That persistence should be kept at a layer above.
Comment 41 Troy Bishop CLA 2010-03-18 18:36:39 EDT
(In reply to comment #40)
> You could have a bundle that reads the preferences on startup and uses the
> methods
> 
> DebugOptions.setOptions(options)
> DebugOptions.setDebugEnabled(true)
> 
> With the options gotten out of the saved prerferences, right?  I am not for
> adding some persistence of this data into the framework.  That persistence
> should be kept at a layer above.

Thanks, I will implement an IStartup extension and do what you suggested. I just wanted to make sure that there wasn't some other persistence mechanism that was used within org.eclipse.osgi.
Comment 42 Christian Georgi CLA 2010-11-10 07:32:04 EST
Any progress here?  I would like to see this included in Eclipse 3.7.
Comment 43 Curtis Windatt CLA 2011-06-22 10:04:25 EDT
This will be considered for 3.8 inclusion.  However, it is not clear to me who should own the code and where the UI should fit in.
Comment 44 Ankur Sharma CLA 2011-11-30 13:02:15 EST
One of the benefit we seek from this work is to have a runtime control on modification of the options. To achieve this, we have to set the params (like trace file location, size, count, etc) before other plug-ins (who wish to consume osgi tracing). The problem was already underscored in the Comment 39. The solution suggested in Comment 40 may be solve it completely.

The Startup extension does not guarantee the order of loading of plug-ins. Thus, a plug-in using osgi trace can get loaded before our plug-in can initialize the parameters. These plug-ins (which got loaded before) will behave according to default settings.

One possible solution could be this. The plug-ins consume osgi trace service need to register themselves as DebugOptionsListener. When they are notified of the change in options (when we initialize system properties from preferences), they can recreate the trace (DebugTrace). However, this will need changes to the (org.eclipse.osgi).framework.debug.EclipseDebugTrace It currently caches the trace for each bundle. Thus, if one gets created, it always supplies that from its cache. Note that there is no API to communicate the change in system properties (file size, count, etc) to it either.
Comment 45 Ankur Sharma CLA 2011-11-30 13:03:41 EST
The work is in progress in the branch http://git.eclipse.org/c/pde/eclipse.pde.ui.git/log/?h=296631_tracing_ui_prefs
Comment 46 Ankur Sharma CLA 2011-12-05 10:49:42 EST
I have delivered the updated ui labels to the branch 296631_tracing_ui_prefs.
See if the the Browse button looks better now.

The trace file management is completely done by the org.eclipse.osgi. We just set the file name and other parameters in system properties. 

I have noticed that when no file is provided the trace shall get printed to system.out. Current o.e.osgi prints only for the first call. The subsequent trace calls are ignored. There is nothing I can do in this new ui plug-in about it.
Comment 47 Christian Georgi CLA 2011-12-05 10:56:48 EST
BTW: I just noticed that the provided extension point "org.eclipse.equinox.trace.ui.traceComponent" does not adhere to the Eclipse naming conventions as it uses a singular name "traceComponent" where "traceComponents" would be more adequate (see http://wiki.eclipse.org/index.php/Naming_Conventions#Plug-ins_and_Extension_Points).

Just a small issue, but if it's not too late we should change it.
Comment 48 Curtis Windatt CLA 2011-12-12 17:50:07 EST
Committed a fix correcting @since tags, renaming the extension point to be plural and some UI tweaks.  We are close to moving this into master (hopefully we can do this before the holidays).

A few things I noticed:
- The bundle should turn on API tools
- Trace illegally extends DebugTrace.  This is not a good example to set.  It is doing this to 'quotify' NLS'd strings containing {#}. We need a more elegant solution, perhaps even inside OSGi?
- Why is there a minimum of 10 historical files?  The UI needs to inform the user of the minimum and maximum (10 - 100).  Same thing for file size.
- We will need to write documentation explaining the various options.  I'm not sure if the OSGi API we are calling explains exactly what the options do.

Troy, you had mentioned in email you would look at writing some doc we could put up on a blog/wiki/helpdoc to explain how to convert your plug-in.  Any progress?

Ankur, please start looking into what we need to do to add a new bundle to the platform.  At a minimum we will need to add a new entry in our map file and feature (what feature does the error log belong to?)
Comment 49 Dani Megert CLA 2011-12-13 02:59:51 EST
> and
> feature (what feature does the error log belong to?)

http://git.eclipse.org/c/pde/eclipse.pde.git/tree/org.eclipse.pde-feature/feature.xml
Comment 50 Curtis Windatt CLA 2011-12-19 15:46:45 EST
The feature branch has been merged with master.  The new bundle has not been added to the pde feature yet as there are limited builds over the holiday break.
Comment 51 Curtis Windatt CLA 2012-01-04 12:48:08 EST
Test I build is happening at:
https://hudson.eclipse.org/hudson/view/Eclipse%20and%20Equinox/job/eclipse-equinox-test-I/2/
Comment 52 Curtis Windatt CLA 2012-01-04 16:16:34 EST
Test build is complete and is working great for me. I will be on vacation until the 13th. If there is a problem with the I build, Ankur, Dani and Kim will investigate.
Comment 53 Curtis Windatt CLA 2012-01-04 16:17:32 EST
The test build used is actually:
https://hudson.eclipse.org/hudson/view/Eclipse%20and%20Equinox/job/eclipse-equinox-test-N/385/
Comment 54 Dani Megert CLA 2012-01-05 09:50:23 EST
(In reply to comment #52)
> If there is a problem with the I build, Ankur, Dani and Kim will
> investigate.

There was a test failure about missing files in the bundle. Investigation revealed that the 'about.html' was missing.

Fixed in master: fb68d2acfc64b5508fcc216ca3b41c7a08ef2419
Comment 55 Dani Megert CLA 2012-01-06 06:42:38 EST
(In reply to comment #54)
> (In reply to comment #52)
> > If there is a problem with the I build, Ankur, Dani and Kim will
> > investigate.
> 
> There was a test failure about missing files in the bundle. Investigation
> revealed that the 'about.html' was missing.
> 
> Fixed in master: fb68d2acfc64b5508fcc216ca3b41c7a08ef2419

And had to add it to the build properties and also add missing copyright notices: e9de1972a3f1a1ccb05db942617ef40a3cdba24a
Comment 56 Christian Georgi CLA 2012-01-11 03:35:50 EST
The current code does not compile against an Eclipse 3.6 target platform although the manifest claims so.  ServiceTracker is used in its generified version (in DebugOptionsHandler), which only exists from 3.7 on.  While this one is binary-compatible (I guess), InstanceScope.INSTANCE used in PreferenceHandler is a breaking new API.

So either we adjust the manifest or make the code 3.6-compatible.  I would prefer the latter since this one would allow clients to use tracing bundle as a drop-in for older platforms.
Comment 57 Dani Megert CLA 2012-01-11 05:43:41 EST
(In reply to comment #56)
> The current code does not compile against an Eclipse 3.6 target platform
> although the manifest claims so.  ServiceTracker is used in its generified
> version (in DebugOptionsHandler), which only exists from 3.7 on.  While this
> one is binary-compatible (I guess), InstanceScope.INSTANCE used in
> PreferenceHandler is a breaking new API.
> 
> So either we adjust the manifest or make the code 3.6-compatible.  I would
> prefer the latter since this one would allow clients to use tracing bundle as a
> drop-in for older platforms.

If we make it backwards compatible, then it should be against 3.5 to match the date where the feature was introduced in the OSGi layer. For now I've fixed the bundle versions to avoid any confusion.
Comment 58 Christian Georgi CLA 2012-01-11 06:49:06 EST
(In reply to comment #57)
> (In reply to comment #56)
> > The current code does not compile against an Eclipse 3.6 target platform
> > although the manifest claims so.  ServiceTracker is used in its generified
> > version (in DebugOptionsHandler), which only exists from 3.7 on.  While this
> > one is binary-compatible (I guess), InstanceScope.INSTANCE used in
> > PreferenceHandler is a breaking new API.
> > 
> > So either we adjust the manifest or make the code 3.6-compatible.  I would
> > prefer the latter since this one would allow clients to use tracing bundle as a
> > drop-in for older platforms.
> 
> If we make it backwards compatible, then it should be against 3.5 to match the
> date where the feature was introduced in the OSGi layer. For now I've fixed the
> bundle versions to avoid any confusion.

I agree with 3.5 as lowest version.  And I really like to see being backwards-compatible.  We have a concrete scenario where our product needs to be compatible to 3.6 and we are lacking a trace UI, so this would fit in there nicely.  And what I can read from the original postings here it's all about how to get a trace UI in 3.5 ;)  Let me know how I can assist in testing the bundle against 3.5.
Comment 59 Curtis Windatt CLA 2012-01-16 17:50:37 EST
In addition, the 3.5 API for DebugOptions is different.  You cannot get/set a map of all options, you have to do them indvidually. 

I have created Bug 368780 to track changing the dependencies so we can close this bug.  While the changes to make it work are reasonably small, my time is very limited. Christian, please consider contributing a patch.
Comment 60 Christian Georgi CLA 2012-01-27 10:05:08 EST
Created attachment 210187 [details]
Fixes missing updates on change of debug options

ModifiedDebugOptions were not updated correctly in case e.g. a boolean option was changed - the entry was removed but never added to debugOptionsToAdd.
Comment 61 Christian Georgi CLA 2012-01-27 10:11:41 EST
This one is already closed.  Opened https://bugs.eclipse.org/369937.