Re: [collectd] curl_xml plugin

[Florian Forster]

Hi Amit,

On Mon, Jan 18, 2010 at 04:48:21PM +0530, Amit Gupta wrote:
 I have added a sample configuration (Sun Java System Web Server 7.0)
 in the example page.

thanks :)

 I would also want to add a detailed section about cURL-XML in the
 collectd.conf documentation page. How do you want me to submit the
 documentation changes for the same?

Ideally as a patch for src/collectd.conf.pod. But if you send a loosely
structured text file I can add the POD markup, too.

Regards,
—octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/


signature.asc
Description: Digital signature
___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd

Re: [collectd] curl_xml plugin

[Amit Gupta]

On Mon, Jan 11, 2010 at 9:26 PM, Florian Forster o...@verplant.org wrote:

 Hi Amit,

 thank you very much for your patch :)


Thanks for applying the patch :)


 I did the following changes:

  * Re-order all of the functions to get rid of the forward declarations.
  * Replace the string and boolean handling functions for config options
   with functions declared in src/configfile.h. This way the
   functionality is not duplicated.
  * Put the type_instance setting and value parsing into a separate
   functions. This makes “cx_submit_xpath_values” considerably bit
   shorter.

 Please let me know if my changes broke anything – unfortunately I didn't
 have the possibility to test them :/


I did a sanity testing and the plugin seems to be working fine.


  #define CX_KEY_MAGIC 0x43484b59UL /* CHKY */
  #define CX_IS_KEY(key) (key)-magic == CX_KEY_MAGIC

 What do we need this hack for? I've found only one place where
 “c_avl_insert” is called, so the elements in the tree should all be of
 the same type, right? I'd be great if you could remove or document this.


There is no need of  this magic key. It just happens to be there since I
borrowed the code from curl_json :). I will clean this up.


 Maybe the entire AVL tree should be removed: As far as I see it is only
 used to iterate over the values (rather than searching for a specific
 key), so a linked list is probably more appropriate.


Yeah I agree. I have replaced avl tree with a linked list.


 Both, “cx_check_type” and “cx_submit_xpath_values” call “plugin_get_ds”.
 I think one call could be optimized away.

This is also done.

Do find the updated patch attached.

Regards
Amit


 Best regards,
 —octo

 [0] 
 http://git.verplant.org/?p=collectd.git;a=blob_plain;f=src/curl_xml.c;hb=refs/heads/ag/curl_xml
 
 --
 Florian octo Forster
 Hacker in training
 GnuPG: 0x91523C3D
 http://verplant.org/

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.6 (GNU/Linux)

 iD8DBQFLS0olHdggu3Q05IYRAuMXAJ440LTtkMUZaDb7MUq4EdnXw8SXTgCfS3gS
 Pat48nthKsGvE3YMI6rIOh0=
 =VgpJ
 -END PGP SIGNATURE-


--- collectd-latest/src/curl_xml.c	Wed Jan 13 16:23:42 2010
+++ collectd-latest-mine/src/curl_xml.c	Wed Jan 13 15:39:22 2010
@@ -23,7 +23,7 @@
 #include common.h
 #include plugin.h
 #include configfile.h
-#include utils_avltree.h
+#include utils_llist.h
 
 #include libxml/parser.h
 #include libxml/tree.h
@@ -32,8 +32,6 @@
 #include curl/curl.h
 
 #define CX_DEFAULT_HOST localhost
-#define CX_KEY_MAGIC 0x43484b59UL /* CHKY */
-#define CX_IS_KEY(key) (key)-magic == CX_KEY_MAGIC
 
 /*
  * Private data structures
@@ -79,7 +77,7 @@
   size_t buffer_size;
   size_t buffer_fill;
 
-  c_avl_tree_t *tree; /* tree of xpath blocks */
+  llist_t *list; /* list of xpath blocks */
 };
 typedef struct cx_s cx_t; /* }}} */
 
@@ -138,25 +136,26 @@
   sfree (xpath);
 } /* }}} void cx_xpath_free */
 
-static void cx_tree_free (c_avl_tree_t *tree) /* {{{ */
+static void cx_list_free (llist_t *list) /* {{{ */
 {
-  char *name;
-  void *value;
+  llentry_t *le;
 
-  while (c_avl_pick (tree, (void *) name, (void *) value) == 0)
+  le = llist_head (list);
+  while (le != NULL)
   {
-cx_xpath_t *key = (cx_xpath_t *)value;
+llentry_t *le_next;
 
-if (CX_IS_KEY(key))
-  cx_xpath_free (key);
-else
-  cx_tree_free ((c_avl_tree_t *)value);
+le_next = le-next;
 
-sfree (name);
+sfree (le-key);
+cx_xpath_free (le-value);
+
+le = le_next;
   }
 
-  c_avl_destroy (tree);
-} /* }}} void cx_tree_free */
+  llist_destroy (list);
+  list = NULL;
+} /* }}} void cx_list_free */
 
 static void cx_free (void *arg) /* {{{ */
 {
@@ -173,9 +172,8 @@
 curl_easy_cleanup (db-curl);
   db-curl = NULL;
 
-  if (db-tree != NULL)
-cx_tree_free (db-tree);
-  db-tree = NULL;
+  if (db-list != NULL)
+cx_list_free (db-list);
 
   sfree (db-buffer);
   sfree (db-instance);
@@ -190,11 +188,8 @@
   sfree (db);
 } /* }}} void cx_free */
 
-static int cx_check_type (cx_xpath_t *xpath) /* {{{ */
+static int cx_check_type (const data_set_t *ds, cx_xpath_t *xpath) /* {{{ */
 {
-  const data_set_t *ds;
-  
-  ds = plugin_get_ds (xpath-type);
   if (!ds)
   {
 WARNING (curl_xml plugin: DataSet `%s' not defined., xpath-type);
@@ -373,7 +368,7 @@
 if ( (tmp_size == 0)  (is_table) )
 {
   WARNING (curl_xml plugin: 
-  relative xpath expression for 'Instance' \%s\ doesn't match 
+  relative xpath expression for 'InstanceFrom' \%s\ doesn't match 
   any of the nodes. Skipping the node., xpath-instance);
   xmlXPathFreeObject (instance_node_obj);
   return (-1);
@@ -382,7 +377,7 @@
 if (tmp_size  1)
 {
   WARNING (curl_xml plugin: 
-  relative xpath expression for 'Instance' \%s\ is expected 
+  relative xpath expression for 'InstanceFrom' \%s\ is expected 
   to return only one text node. Skipping the node., xpath-instance);
   xmlXPathFreeObject (instance_node_obj);
   return (-1);
@@ -425,7 

Re: [collectd] curl_xml plugin

[Florian Forster]

Hi Amit,

thank you very much for your patch :)

I've applied the patch to my working directory and submitted it (and
some changes of my own) to the “ag/curl_xml” branch of the Git
repository. If you don't want to use Git you can download the latest
version of src/curl_xml.c from [0].

I did the following changes:

 * Re-order all of the functions to get rid of the forward declarations.
 * Replace the string and boolean handling functions for config options
   with functions declared in src/configfile.h. This way the
   functionality is not duplicated.
 * Put the type_instance setting and value parsing into a separate
   functions. This makes “cx_submit_xpath_values” considerably bit
   shorter.

Please let me know if my changes broke anything – unfortunately I didn't
have the possibility to test them :/

 #define CX_KEY_MAGIC 0x43484b59UL /* CHKY */
 #define CX_IS_KEY(key) (key)-magic == CX_KEY_MAGIC

What do we need this hack for? I've found only one place where
“c_avl_insert” is called, so the elements in the tree should all be of
the same type, right? I'd be great if you could remove or document this.

Maybe the entire AVL tree should be removed: As far as I see it is only
used to iterate over the values (rather than searching for a specific
key), so a linked list is probably more appropriate.

Both, “cx_check_type” and “cx_submit_xpath_values” call “plugin_get_ds”.
I think one call could be optimized away.

Best regards,
—octo

[0] 
http://git.verplant.org/?p=collectd.git;a=blob_plain;f=src/curl_xml.c;hb=refs/heads/ag/curl_xml
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/


signature.asc
Description: Digital signature
___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd

Re: [collectd] curl_xml plugin

[Amit Gupta]

On Wed, Dec 30, 2009 at 6:40 PM, Amit Gupta amit.gupta...@gmail.com wrote:

 On Tue, Dec 29, 2009 at 9:23 PM, Sebastian Harl s...@tokkee.org wrote:

 I was thinking about the implementation of the suggested configuration and
 it seems that the implementation can be simpler if we consider the following
 approach:

 The evaluation of the base_xpath (i.e argument to the XPATH element) will
 return either of the following:
a.) Single result: This can be assumed as Table false case. The
 relative xpath expression(s) in Values is/are always expected to return
 one text node/attribute result.
b.) Multiple results : This can be assumed as Table true case.
 Instance xpath expression will be evaluated and result(s) will be stored
 in a list.  Multiple results will be iterated and the relative xpath(s) in
 Values will be evaluated relative to each of the result (node) [the
 result(s) of the relative xpath(s) can be dispatched to the rrd file at the
 same time]. The relative xpath expression(s) in Values is/are always
 expected to return one text node/attribute result.

 Do note that Table won't be required since single/multiple result(s) of
 the base_xpath evaluation can be used for the same as described above.

 Does this make sense?. Let me know if something is unclear.


Do find the curl_xml.c source file attached. The implementation is done as I
suggested in my previous mail.
The supported configuration is as follows:
xpath /path/to/node(s)
   InstancePrefix test # optional
   Instance path/to/textnode  # optional if base xpath expression returns
single result i.e one matching node
   Values path/to/textnode1  path/to/textnode2
   Type magic_level
 /xpath

I have done some level of testing to make sure plugin is working. If someone
is interested in testing/trying the plugin, do the following:
- apply the attached patch on the 4.9.0 branch
- copy the attached curl_xml.c to the collectd-4.9.0/src
- run autconf to generate new configure script containing changes to build
curl_xml plugin
- build and install collectd (make  make install)

Do let me know if anyone has problem building/running the plugin.

Any comments/suggestions are welcome.

Regards
Amit


 Regards
 Amit


 Cheers,
 Sebastian

 --
 Sebastian tokkee Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

 Those who would give up Essential Liberty to purchase a little Temporary
 Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin


 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAks6JgcACgkQEFEKc4UBx/zbtQCfWIIVzQr9nhm1sHZPpedUkmRV
 W7AAmwfaoj4/EhrGveBDh7yCbbJ7EUVv
 =NM8M
 -END PGP SIGNATURE-



/**
 * collectd - src/curl_xml.c
 * Copyright (C) 2009   Amit Gupta
 *
 * This program is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License as published by the
 * Free Software Foundation; only version 2 of the License is applicable.
 *
 * This program is distributed in the hope that it will be useful, but
 * WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License along
 * with this program; if not, write to the Free Software Foundation, Inc.,
 * 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 *
 * Authors:
 *   Amit Gupta amit.gupta221 at gmail.com
 **/

#include collectd.h
#include common.h
#include plugin.h
#include configfile.h
#include utils_avltree.h

#include libxml/parser.h
#include libxml/tree.h
#include libxml/xpath.h

#include curl/curl.h

#define CX_DEFAULT_HOST localhost
#define CX_KEY_MAGIC 0x43484b59UL /* CHKY */
#define CX_IS_KEY(key) (key)-magic == CX_KEY_MAGIC

struct cx_values_s /* {{{ */
{
  char path[DATA_MAX_NAME_LEN];
  size_t path_len;
};
typedef struct cx_values_s cx_values_t;
/* }}} */

struct cx_xpath_s /* {{{ */
{
  char *path;
  char *type;
  cx_values_t *values;
  int values_len;
  char *instance_prefix;
  char *instance;
  int is_table;
  unsigned long magic;
};
typedef struct cx_xpath_s cx_xpath_t;
/* }}} */

struct cx_s /* {{{ */
{
  char *instance;
  char *host;

  char *url;
  char *user;
  char *pass;
  char *credentials;
  int   verify_peer;
  int   verify_host;
  char *cacert;

  CURL *curl;
  char curl_errbuf[CURL_ERROR_SIZE];
  char *buffer;
  size_t buffer_size;
  size_t buffer_fill;

  c_avl_tree_t *tree; /* tree of xpath blocks */
};
typedef struct cx_s cx_t; /* }}} */

static int cx_read (user_data_t *ud);
static int cx_curl_perform (cx_t *db, CURL *curl);
static int cx_parse_stats_xml (xmlChar *ptr, cx_t *db);
static int cx_submit_statistics(xmlDocPtr doc, xmlXPathContextPtr xpath_ctx,
   cx_t *db);
static int cx_submit_xpath_values (char *plugin_instance, xmlXPathContextPtr xpath_ctx, 
char *base_xpath, cx_xpath_t *xpath);
static int 

Re: [collectd] curl_xml plugin

[Amit Gupta]

On Tue, Dec 29, 2009 at 9:23 PM, Sebastian Harl s...@tokkee.org wrote:

 Hi Amit,

 On Tue, Dec 29, 2009 at 07:24:46PM +0530, Amit Gupta wrote:
  On Tue, Dec 29, 2009 at 3:31 PM, Amit Gupta amit.gupta...@gmail.com
 wrote:
   On Mon, Dec 21, 2009 at 11:59 PM, Florian Forster o...@verplant.org
 wrote:
   Just a clarification, Instance here will be type_instance(s) and thus
 can
   be anything (not just xpath), right?
 
  I guess I understood why Instance has to be a xpath expression if
 table
  is true. Instance xpath would return list of values which will be used as
  type_instances for multiple type values returned by evaluating xpaths in
  Values. Is my understanding correct?.

 Yep, that's right.

 If Table is false, a simple string would be sufficient as well, but
 changing the semantics of Instance depending on Table would be
 confusing -- also, people might still want to fetch the instance from
 the XML file.

 Other plugins (e.g., postgresql) handle that by introducing an option
 called InstancePrefix which expects a string as argument. That string
 is prepended to the type_instance.

 If Table is false, Instance would be optional, thus making it
 possible to specify a constant string as type_instance using Instance-
 Prefix.


Thanks for the detailed explanation.

I was thinking about the implementation of the suggested configuration and
it seems that the implementation can be simpler if we consider the following
approach:

The evaluation of the base_xpath (i.e argument to the XPATH element) will
return either of the following:
   a.) Single result: This can be assumed as Table false case. The
relative xpath expression(s) in Values is/are always expected to return
one text node/attribute result.
   b.) Multiple results : This can be assumed as Table true case.
Instance xpath expression will be evaluated and result(s) will be stored
in a list.  Multiple results will be iterated and the relative xpath(s) in
Values will be evaluated relative to each of the result (node) [the
result(s) of the relative xpath(s) can be dispatched to the rrd file at the
same time]. The relative xpath expression(s) in Values is/are always
expected to return one text node/attribute result.

Do note that Table won't be required since single/multiple result(s) of
the base_xpath evaluation can be used for the same as described above.

Does this make sense?. Let me know if something is unclear.

Regards
Amit


 Cheers,
 Sebastian

 --
 Sebastian tokkee Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

 Those who would give up Essential Liberty to purchase a little Temporary
 Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin


 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.9 (GNU/Linux)

 iEYEARECAAYFAks6JgcACgkQEFEKc4UBx/zbtQCfWIIVzQr9nhm1sHZPpedUkmRV
 W7AAmwfaoj4/EhrGveBDh7yCbbJ7EUVv
 =NM8M
 -END PGP SIGNATURE-


___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd

Re: [collectd] curl_xml plugin

[Sebastian Harl]

Hi Amit,

On Tue, Dec 29, 2009 at 07:24:46PM +0530, Amit Gupta wrote:
 On Tue, Dec 29, 2009 at 3:31 PM, Amit Gupta amit.gupta...@gmail.com wrote:
  On Mon, Dec 21, 2009 at 11:59 PM, Florian Forster o...@verplant.orgwrote:
  Where:
 
   * The argumetn to XPath is an xpath which returns a base block in
 which to look further.
   * The arguments to Instance and Values are xpath expressions, too.
 
  Just a clarification, Instance here will be type_instance(s) and thus can
  be anything (not just xpath), right?
 
 I guess I understood why Instance has to be a xpath expression if table
 is true. Instance xpath would return list of values which will be used as
 type_instances for multiple type values returned by evaluating xpaths in
 Values. Is my understanding correct?.

Yep, that's right.

If Table is false, a simple string would be sufficient as well, but
changing the semantics of Instance depending on Table would be
confusing -- also, people might still want to fetch the instance from
the XML file.

Other plugins (e.g., postgresql) handle that by introducing an option
called InstancePrefix which expects a string as argument. That string
is prepended to the type_instance.

If Table is false, Instance would be optional, thus making it
possible to specify a constant string as type_instance using Instance-
Prefix.

Cheers,
Sebastian

-- 
Sebastian tokkee Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin



signature.asc
Description: Digital signature
___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd

Re: [collectd] curl_xml plugin

[Amit Gupta]

Haven't heard from anyone other than Marco.

Florian/Sebastian,

Any comments on the plugin will be appreciated.

Regards
Amit

On Mon, Nov 16, 2009 at 5:45 PM, Amit Gupta amit.gupta...@gmail.com wrote:

 Any comments/suggestions for this proposed plugin?

 Regards
 Amit


 On Fri, Nov 6, 2009 at 4:41 PM, Amit Gupta amit.gupta...@gmail.comwrote:

 Hi,

 I have been working on curl_xml plugin. The idea of this plugin is to
 query the xml data using the curl library and parse it according to the
 user's configuration using libxml2. I have worked on the top of curl-json
 plugin code and got this plugin to work. I am proposing the following
 configuration for curl_xml:

 Plugin curl_xml
URL [some url]
Instance instance-name
xpath xpath1
   Instance instance-name (Optional)
   Type type
/xpath
xpath xpath2
   Instance instance-name (Optional)
   Type type
/xpath
   /URL
   URL .
  ..
/URL
 /Plugin

 libxml2 will parse the xml using xpath specified in the configuration. For
 now, the attached code supports one xpath to one element mapping.

 Does this plugin make sense?. I will be submitting the configure, Makefile
 and the documentation changes once this idea is approved.

 Regards
 Amit



___
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd