Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.1.1, 3.1.2, 4.0
    • Fix Version/s: 3.1.2_b16
    • Component/s: monitoring
    • Labels:
      None

      Description

      (1) Aren't TODO comments useless in actual practice? There is no reminding like in C #pragma's

      From FlashlightProbeProviderFactory:

      public <T> T getProbeProvider(Class<T> providerClazz, String invokerId)
      throws InstantiationException, IllegalAccessException {
      //TODO: check for null and generate default names

      // JIRA note: KABOOM!!!!!
      ProbeProvider provAnn = providerClazz.getAnnotation(ProbeProvider.class);

      String moduleProviderName = provAnn.moduleProviderName();
      String moduleName = provAnn.moduleName();
      String probeProviderName = provAnn.probeProviderName();

      (2)
      In that same file there is a confusing mess of 4 getProbeProvider methods. 3 of them are implementation methods from the interface. Why isn't the fourth? In order to us that method you have to cast from the interface to the concrete class.

        Activity

        Hide
        Byron Nevins added a comment -

        Asking for help from Jennifer

        Show
        Byron Nevins added a comment - Asking for help from Jennifer
        Hide
        Jennifer Chou added a comment -

        For fixing (1)

        • What is the impact on the customer of the bug?

        How likely is it that a customer will see the bug and how serious is the bug?

        Not as likely, since all the GlassFish Probe Providers have been written appropriately. The bug would only appear if another Probe Provider is added where one of the attributes, moduleProviderName, moduleName, or probeProviderName is not specified. Then there would be an NPE.

        Is it a regression? No. Does it meet other bug fix criteria (security, performance, etc.)? No.

        • What is the cost/risk of fixing the bug?

        How risky is the fix? Low risk. How much work is the fix? Low cost. Is the fix complicated? No.

        • Is there an impact on documentation or message strings? No.
        • Which tests should QA (re)run to verify the fix did not destabilize GlassFish? Monitoring tests.
        • Which is the targeted build of 3.1.2 for this fix? b16
        Show
        Jennifer Chou added a comment - For fixing (1) What is the impact on the customer of the bug? How likely is it that a customer will see the bug and how serious is the bug? Not as likely, since all the GlassFish Probe Providers have been written appropriately. The bug would only appear if another Probe Provider is added where one of the attributes, moduleProviderName, moduleName, or probeProviderName is not specified. Then there would be an NPE. Is it a regression? No. Does it meet other bug fix criteria (security, performance, etc.)? No. What is the cost/risk of fixing the bug? How risky is the fix? Low risk. How much work is the fix? Low cost. Is the fix complicated? No. Is there an impact on documentation or message strings? No. Which tests should QA (re)run to verify the fix did not destabilize GlassFish? Monitoring tests. Which is the targeted build of 3.1.2 for this fix? b16
        Hide
        Jennifer Chou added a comment -

        Fixed on 3.1.2 branch.

        ==[IDE]== Dec 23, 2011 4:06:15 PM Committing...
        commit -m "GLASSFISH-17174 NPE waiting to fire..." /Users/jenchou/scratch/gf/3.1.2/flashlight/framework/src/main/java/org/glassfish/flashlight/impl/provider/FlashlightProbeProviderFactory.java
        Sending /Users/jenchou/scratch/gf/3.1.2/flashlight/framework/src/main/java/org/glassfish/flashlight/impl/provider/FlashlightProbeProviderFactory.java
        Transmitting file data ...
        Committed revision 51753.
        Revision: 51753
        Author : jc129909
        Date : Dec 23, 2011 4:07:30 PM
        GLASSFISH-17174 NPE waiting to fire
        Added null checks and default values for ProbeProvider moduleProviderName, moduleName, probeProviderName.

        Reviewed by: Byron Nevins
        Tests: QL, monitoring dev tests

        ==[IDE]== Dec 23, 2011 4:07:33 PM Committing... finished.

        Fixed on trunk.

        Resource URL https://svn.java.net/svn/glassfish~svn/trunk/main/nucleus/flashlight/framework/src/main/java/org/glassfish/flashlight/impl/provider/FlashlightProbeProviderFactory.java
        Repository Root URL https://svn.java.net/svn/glassfish~svn
        Revision 51743
        Last Changed Author jc129909
        Last Changed Date Dec 22, 2011 7:59:32 PM
        Last Changed Revision 51743

        Show
        Jennifer Chou added a comment - Fixed on 3.1.2 branch. == [IDE] == Dec 23, 2011 4:06:15 PM Committing... commit -m " GLASSFISH-17174 NPE waiting to fire..." /Users/jenchou/scratch/gf/3.1.2/flashlight/framework/src/main/java/org/glassfish/flashlight/impl/provider/FlashlightProbeProviderFactory.java Sending /Users/jenchou/scratch/gf/3.1.2/flashlight/framework/src/main/java/org/glassfish/flashlight/impl/provider/FlashlightProbeProviderFactory.java Transmitting file data ... Committed revision 51753. Revision: 51753 Author : jc129909 Date : Dec 23, 2011 4:07:30 PM GLASSFISH-17174 NPE waiting to fire Added null checks and default values for ProbeProvider moduleProviderName, moduleName, probeProviderName. Reviewed by: Byron Nevins Tests: QL, monitoring dev tests == [IDE] == Dec 23, 2011 4:07:33 PM Committing... finished. Fixed on trunk. Resource URL https://svn.java.net/svn/glassfish~svn/trunk/main/nucleus/flashlight/framework/src/main/java/org/glassfish/flashlight/impl/provider/FlashlightProbeProviderFactory.java Repository Root URL https://svn.java.net/svn/glassfish~svn Revision 51743 Last Changed Author jc129909 Last Changed Date Dec 22, 2011 7:59:32 PM Last Changed Revision 51743

          People

          • Assignee:
            Jennifer Chou
            Reporter:
            Byron Nevins
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: