You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2014/12/17 23:44:59 UTC

qpid-proton git commit: PROTON-772: New proton logging code

Repository: qpid-proton
Updated Branches:
  refs/heads/master 85af446f2 -> a72202d4f


PROTON-772: New proton logging code

Based on list discussion:
- moved the log statement api into src/log_private
- simplified the public API to 2 functions: pn_log_enable and pn_log_logger

The minimal public API is required to let applications using proton control
where proton log output goes, which is the point of all this.

The present state of the logging achieves the following goals:
- allow applications to disable or redirect ALL proton library output, nothing is forced to stderr.
- leaves the previous behavior of transport tracers unaffected.
- introduces as little disturbance as possible (one new env. var PN_TRACE_LOG and 2 new API functions)

This is definitely not the final word on logging for proton, but a more
sophisticated, unified logging system requires more discussion in the proton
community.


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/a72202d4
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/a72202d4
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/a72202d4

Branch: refs/heads/master
Commit: a72202d4f4c02221c6555a9e76480d383c7398c8
Parents: 85af446
Author: Alan Conway <ac...@redhat.com>
Authored: Wed Dec 17 17:44:00 2014 -0500
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Dec 17 17:44:00 2014 -0500

----------------------------------------------------------------------
 proton-c/include/proton/log.h      | 49 +++++++-----------------------
 proton-c/src/codec/codec.c         |  2 +-
 proton-c/src/log.c                 | 25 ++++++++-------
 proton-c/src/log_private.h         | 54 +++++++++++++++++++++++++++++++++
 proton-c/src/messenger/messenger.c |  2 +-
 proton-c/src/posix/driver.c        |  2 +-
 proton-c/src/transport/transport.c |  3 +-
 7 files changed, 81 insertions(+), 56 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a72202d4/proton-c/include/proton/log.h
----------------------------------------------------------------------
diff --git a/proton-c/include/proton/log.h b/proton-c/include/proton/log.h
index 73b9b6d..a194e8c 100644
--- a/proton-c/include/proton/log.h
+++ b/proton-c/include/proton/log.h
@@ -21,57 +21,30 @@
 
 /**@file
  *
- * Debug logging for messages that are not associated with a transport.
+ * Control log messages that are not associated with a transport.
  * See pn_transport_trace for transport-related logging.
  */
 
 #include <proton/import_export.h>
 #include <proton/type_compat.h>
-#include <stdarg.h>
 
-/** Callback for customized logging */
+/** Callback for customized logging. */
 typedef void (*pn_logger_t)(const char *message);
 
-/** Initialize the logging module, enables logging based on the PN_LOG_TRACE envionment variable.
+/** Enable/disable global logging.
  *
- * Must be called before any other pn_log_* functions (e.g. from main()).
- * Not required unless you use pn_log_enable.
- */
-PN_EXTERN void pn_log_init(void);
-
-/** Enable/disable logging.
- *
- * Note that if pn_log_init() was not called then this is not
- * thread safe. If called concurrently with calls to other pn_log functions
- * there is a chance that the envionment variable settings will win.
+ * By default, logging is enabled by envionment variable PN_TRACE_LOG.
+ * Calling this function overrides the environment setting.
  */
 PN_EXTERN void pn_log_enable(bool enabled);
 
-/** Return true if logging is enabled. */
-PN_EXTERN bool pn_log_enabled(void);
-
-/** Log a printf style message */
-#define pn_logf(...)                            \
-    do {                                        \
-        if (pn_log_enabled())                   \
-            pn_logf_impl(__VA_ARGS__);          \
-    } while(0)
-
-/** va_list version of pn_logf */
-#define pn_vlogf(fmt, ap)                       \
-    do {                                        \
-        if (pn_log_enabled())                   \
-            pn_vlogf_impl(fmt, ap);             \
-    } while(0)
-
-/** Set the logger. By default a logger that prints to stderr is installed.
- *@param logger Can be 0 to disable logging regardless of pn_log_enable.
+/** Set the logger.
+ *
+ * By default a logger that prints to stderr is installed.
+ *  
+ * @param logger is called with each log messsage if logging is enabled.
+ * Passing 0 disables logging regardless of pn_log_enable() or environment settings.
  */
 PN_EXTERN void pn_log_logger(pn_logger_t logger);
 
-/**@internal*/
-PN_EXTERN void pn_logf_impl(const char* fmt, ...);
-/**@internal*/
-PN_EXTERN void pn_vlogf_impl(const char *fmt, va_list ap);
-
 #endif

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a72202d4/proton-c/src/codec/codec.c
----------------------------------------------------------------------
diff --git a/proton-c/src/codec/codec.c b/proton-c/src/codec/codec.c
index d9c38b5..1c4cbeb 100644
--- a/proton-c/src/codec/codec.c
+++ b/proton-c/src/codec/codec.c
@@ -22,7 +22,6 @@
 #include <proton/object.h>
 #include <proton/codec.h>
 #include <proton/error.h>
-#include <proton/log.h>
 #include <assert.h>
 #include <stdio.h>
 #include <string.h>
@@ -38,6 +37,7 @@
 #include "decoder.h"
 #include "encoder.h"
 #include "data.h"
+#include "../log_private.h"
 
 const char *pn_type_name(pn_type_t type)
 {

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a72202d4/proton-c/src/log.c
----------------------------------------------------------------------
diff --git a/proton-c/src/log.c b/proton-c/src/log.c
index 1140561..b92a99a 100644
--- a/proton-c/src/log.c
+++ b/proton-c/src/log.c
@@ -28,20 +28,23 @@ static void stderr_logger(const char *message) {
 }
 
 static pn_logger_t logger = stderr_logger;
-static int enabled = -1;        /* Undefined, use environment variables. */
+static int enabled_env  = -1;   /* Set from environment variable. */
+static int enabled_call = -1;   /* set by pn_log_enable */
 
-void pn_log_init() {
-    enabled = pn_env_bool("PN_TRACE_LOG");
+void pn_log_enable(bool value) {
+    enabled_call = value;
 }
 
-void pn_log_enable(bool new_enabled) {
-    enabled = new_enabled;
+bool pn_log_enabled(void) {
+    if (enabled_call != -1) return enabled_call; /* Takes precedence */
+    if (enabled_env == -1) 
+        enabled_env = pn_env_bool("PN_TRACE_LOG");
+    return enabled_env;
 }
 
-bool pn_log_enabled() {
-    if (!logger) return false;
-    if (enabled == -1) pn_log_init();
-    return enabled;
+void pn_log_logger(pn_logger_t new_logger) {
+    logger = new_logger;
+    if (!logger) pn_log_enable(false);
 }
 
 void pn_vlogf_impl(const char *fmt, va_list ap) {
@@ -58,7 +61,6 @@ void pn_vlogf_impl(const char *fmt, va_list ap) {
  * complicated messages.) It is important that a disabled log statement results
  * in nothing more than a call to pn_log_enabled().
  */
-
 void pn_logf_impl(const char *fmt, ...) {
   va_list ap;
   va_start(ap, fmt);
@@ -66,6 +68,3 @@ void pn_logf_impl(const char *fmt, ...) {
   va_end(ap);
 }
 
-void pn_log_logger(pn_logger_t new_logger) {
-    logger = new_logger;
-}

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a72202d4/proton-c/src/log_private.h
----------------------------------------------------------------------
diff --git a/proton-c/src/log_private.h b/proton-c/src/log_private.h
new file mode 100644
index 0000000..828e189
--- /dev/null
+++ b/proton-c/src/log_private.h
@@ -0,0 +1,54 @@
+#ifndef LOG_PRIVATE_H
+#define LOG_PRIVATE_H
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**@file
+ *
+ * Log messages that are not associated with a transport.
+ */
+
+#include <proton/log.h>
+#include <stdarg.h>
+
+/** Log a printf style message */
+#define pn_logf(...)                            \
+    do {                                        \
+        if (pn_log_enabled())                   \
+            pn_logf_impl(__VA_ARGS__);          \
+    } while(0)
+
+/** va_list version of pn_logf */
+#define pn_vlogf(fmt, ap)                       \
+    do {                                        \
+        if (pn_log_enabled())                   \
+            pn_vlogf_impl(fmt, ap);             \
+    } while(0)
+
+/** Return true if logging is enabled. */
+PN_EXTERN bool pn_log_enabled(void);
+
+/**@internal*/
+PN_EXTERN void pn_logf_impl(const char* fmt, ...);
+/**@internal*/
+PN_EXTERN void pn_vlogf_impl(const char *fmt, va_list ap);
+
+
+
+#endif

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a72202d4/proton-c/src/messenger/messenger.c
----------------------------------------------------------------------
diff --git a/proton-c/src/messenger/messenger.c b/proton-c/src/messenger/messenger.c
index be7c1b1..4cdd5e9 100644
--- a/proton-c/src/messenger/messenger.c
+++ b/proton-c/src/messenger/messenger.c
@@ -24,7 +24,6 @@
 #include <proton/connection.h>
 #include <proton/delivery.h>
 #include <proton/event.h>
-#include <proton/log.h>
 #include <proton/object.h>
 #include <proton/sasl.h>
 #include <proton/session.h>
@@ -43,6 +42,7 @@
 #include "transform.h"
 #include "subscription.h"
 #include "selectable.h"
+#include "../log_private.h"
 
 typedef struct pn_link_ctx_t pn_link_ctx_t;
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a72202d4/proton-c/src/posix/driver.c
----------------------------------------------------------------------
diff --git a/proton-c/src/posix/driver.c b/proton-c/src/posix/driver.c
index 40486c0..5d513a4 100644
--- a/proton-c/src/posix/driver.c
+++ b/proton-c/src/posix/driver.c
@@ -38,7 +38,7 @@
 #include <proton/sasl.h>
 #include <proton/ssl.h>
 #include <proton/object.h>
-#include <proton/log.h>
+#include "../log_private.h"
 #include "util.h"
 #include "platform.h"
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/a72202d4/proton-c/src/transport/transport.c
----------------------------------------------------------------------
diff --git a/proton-c/src/transport/transport.c b/proton-c/src/transport/transport.c
index dbbf27c..ce8c7e3 100644
--- a/proton-c/src/transport/transport.c
+++ b/proton-c/src/transport/transport.c
@@ -19,8 +19,6 @@
  *
  */
 
-#include <proton/log.h>
-
 #include "engine/engine-internal.h"
 #include "framing/framing.h"
 #include "sasl/sasl-internal.h"
@@ -32,6 +30,7 @@
 #include "proton/event.h"
 #include "platform.h"
 #include "platform_fmt.h"
+#include "../log_private.h"
 
 #include <stdlib.h>
 #include <string.h>


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org