Update LDMS Variorum sampler to support Variorum v0.8

[tpatki]

Work in Progress, not ready for review.

Fixes #1431.

This PR updates the JSON API and the parsing of power data based on Variorum 0.8.

ToDo:

• Code Formatting
• Documentation updates
• Squash commits
• Autoconf checks
• Testing (see below)

Testing:

• IBM Alehouse node, CPU+GPU build
• IBM Alehouse node, GPU-build only
• IBM Alehouse node, CPU-build only
• Other LLNL systems with @kshoga1 and @slabasan.

[morrone]

@tpatki Could you also please this squash this PR together into just a small set of clean commits?

You will also need to include appropriate autoconf checks since this will now only work with variorum 0.8 or newer.

[tpatki]

@morrone
Thanks for the quick feedback! :)

Yes, I will squash into clean commits when we're done testing and when I mark this as ready for review (will be a few weeks until @kshoga1 can test on Mutt.)

You will also need to include appropriate autoconf checks since this will now only work with variorum 0.8 or newer.

Could you please clarify this? We were not checking for a specific version in configure.ac here earlier. Where/how should we include the autoconf checks (is there an example we can follow)?

[morrone]

Could you please clarify this? We were not checking for a specific version in configure.ac here earlier. Where/how should we include the autoconf checks (is there an example we can follow)?

Better than a specific version check is to test for features in the library that the code requires. For instance, are there some unique new function in the new version of the library? You could add a test for that function in the library.

LDMS's configure.ac has an AC_LIB_HAVE_LINKFLAGS test for variorum. There is an additional "testcode" field for that macro where you could put a tiny bit of C code that calls a function that is unique to the newer version of the code.

https://www.gnu.org/software/gnulib/manual/html_node/Searching-for-Libraries.html

[tom95858]

@tpatki, @morrone what is the status of this PR?

[tpatki]

Hi @tom95858 : This is draft PR at the moment. Kathleen (@kshoga1) will need to test this at scale internally. We plan to do that in this month. Once we have tested at scale, we'll address Chris' comments, update it as ready for review, and it can be merged.

[tom95858]

@tpatki Ok great, thanks for the update.

[tom95858]

@morrone, please confirm that this is not a target for 4.4.5.

[morrone]

@morrone, please confirm that this is not a target for 4.4.5.

Except for reviewing here, I'm not really involved in that work. But my understanding is that it'll happen when it happens, and we should not hold up the release for this work.

[tom95858]

@tpatki please reopen this pull request when it is ready for review.

[tpatki]

Hi @morrone :

@kshoga1 tested this on an LC system this afternoon, and we can confirm that the plugin builds correctly on LC systems and the power data reported is correct.

We also discussed your suggestion of adding a small testcode with AC_LIB_HAVE_LINKFLAGS -- but we realized that this doesn't make sense for Variorum as we don't have a small-enough check that can specifically test for version 0.8. What has changed is not the Variorum API, but how to data is reported (nested JSON as opposed to a non-nested format; the older format is not supported any more). This changes how the data is parsed in any tools that Variorum integrates with, e.g. the LDMS Variorum sampler.

If you have other suggestions on how we can do a version check, let us know.

This PR is ready for your review and ready to go. We don't have permissions to reopen this PR. I have squashed the commits on the branch here, but the PR is not updating as it is closed.

[morrone]

@tpatki This PR was opened against OVIS-4, which we've done away with. We now use the more standard "main" branch name for the main development branch. github won't let me reopen either becasue OVIS-4 doesn't exist.

I would recommend just opening a new PR.

On the topic of version checking:

It looks like variorum supplies a variorum.pc file, which is great! But at least as of 0.6.0 it looks like that file is installed in the wrong path on the system. It looks like currrently it is going in /usr/share/, and I think it needs to be in ${libpath}/pkgconfig/ or /usr/share/pkgconfig/

If that can be fixed in variorum 0.8's rpm, then the following patch might meet our needs.

diff --git a/configure.ac b/configure.ac
index 4eb068da0..9b4a3ea14 100644
--- a/configure.ac
+++ b/configure.ac
@@ -388,6 +388,11 @@ AC_ARG_ENABLE([variorum],
     [],
     [enable_variorum="check"])
 AS_IF([test "x$enable_variorum" != xno],[
+    PKG_CHECK_MODULES([variorum], [variorum >= 0.8], [variorum_version_check=yes], [variorum_version_check=no])
+    AS_IF([test "x$enable_variorum" != xcheck],[
+        AS_IF([test "x$variorum_version_check" = xno],
+            [AC_MSG_ERROR([variorum pkgconfig file not found, or variorum version is not >= 0.8])])
+    ])
     AC_LIB_HAVE_LINKFLAGS([variorum], [], [#include <variorum.h>
                            #include <variorum_topology.h>])
     AS_IF([test "x$enable_variorum" != xcheck],[
@@ -399,7 +404,7 @@ AS_IF([test "x$enable_variorum" != xno],[
             [AC_MSG_ERROR([libjansson or headers not found])])
     ])
 ])
-AM_CONDITIONAL([ENABLE_VARIORUM_SAMPLER], [test "x$enable_variorum" != no -a "x$HAVE_LIBVARIORUM" = xyes -a "x$HAVE_LIBJANSSON" = xyes])
+AM_CONDITIONAL([ENABLE_VARIORUM_SAMPLER], [test "x$enable_variorum" != no -a "x$HAVE_LIBVARIORUM" = xyes -a "x$variorum_version_check" = xyes -a "x$HAVE_LIBJANSSON" = xyes])

 dnl check for libcurl if influx is configured
 OPTION_DEFAULT_DISABLE([influx], [ENABLE_INFLUX])