You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by sebb <se...@gmail.com> on 2014/11/25 20:59:19 UTC

Re: svn commit: r1641467 - in /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite: PickleGraphiteMetricsSender.java TextGraphiteMetricsSender.java

On 24 November 2014 at 20:20,  <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Mon Nov 24 20:20:22 2014
> New Revision: 1641467
>
> URL: http://svn.apache.org/r1641467
> Log:
> Remove wrong logging code as per Felix review
>
> Modified:
>     jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/PickleGraphiteMetricsSender.java
>     jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/TextGraphiteMetricsSender.java
>
> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/PickleGraphiteMetricsSender.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/PickleGraphiteMetricsSender.java?rev=1641467&r1=1641466&r2=1641467&view=diff
> ==============================================================================
> --- jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/PickleGraphiteMetricsSender.java (original)
> +++ jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/PickleGraphiteMetricsSender.java Mon Nov 24 20:20:22 2014
> @@ -121,11 +121,7 @@ class PickleGraphiteMetricsSender extend
>                          LOG.warn("Exception invalidating socketOutputStream connected to graphite server '"+socketConnectionInfos.getHost()+"':"+socketConnectionInfos.getPort(), e1);
>                      }
>                  }
> -                if (LOG.isDebugEnabled()) {
> -                    LOG.debug("Error writing to Graphite", e);

There was no need to protect the log call above, as it is not
expensive to store a parameter on the stack

> -                } else {
> -                    LOG.warn("Error writing to Graphite:"+e.getMessage());
> -                }
> +                LOG.error("Error writing to Graphite:"+e.getMessage());

I think error logs should generally include the stacktrace, i.e.

LOG.error("Error writing to Graphite", e);

They should not often happen, but when they do, one needs all the
relevant information.

>              }
>
>              // if there was an error, we might miss some data. for now, drop those on the floor and
>
> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/TextGraphiteMetricsSender.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/TextGraphiteMetricsSender.java?rev=1641467&r1=1641466&r2=1641467&view=diff
> ==============================================================================
> --- jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/TextGraphiteMetricsSender.java (original)
> +++ jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/graphite/TextGraphiteMetricsSender.java Mon Nov 24 20:20:22 2014
> @@ -105,11 +105,7 @@ class TextGraphiteMetricsSender extends
>                                  socketConnectionInfos.getHost()+"':"+socketConnectionInfos.getPort(), e1);
>                      }
>                  }
> -                if (LOG.isDebugEnabled()) {
> -                    LOG.debug("Error writing to Graphite", e);
> -                } else {
> -                    LOG.warn("Error writing to Graphite:"+e.getMessage());
> -                }
> +                LOG.error("Error writing to Graphite:"+e.getMessage());
>              }
>              // We drop metrics in all cases
>              metrics.clear();
>
>