varnish-cache icon indicating copy to clipboard operation
varnish-cache copied to clipboard

Add VMOD version information to Panic output

Open nigoroll opened this issue 1 year ago • 7 comments

This commit adds a facility which I had wanted for a very long time, and should have added much earlier: Analyzing bug reports with VMODs involved used to be complicated by the fact that they typically did not contain reliable version information.

We add to vmodtool.py the generic code to create version information from generate.py with minor modifications. It adds the version set for autotools and, if possible, the git revision to the vmodtool-generated interface code. We copy that to struct vrt and output it for panics.

Being at it, we polish the panic output slightly for readability.

As an example, the output from a panic now might look like this:

vmods = {
  curl = {p=0x7ff9e064a230, abi="Varnish trunk 403a4743751bb6ae81abfb185abae13419ee0fb7", vrt=18.1,
    version="libvmod-curl 1.0.4 / 8cf03452ee59ded56fde5fb3175ffec755a6c78b"},
  directors = {p=0x7ff9e064a2a0, abi="Varnish trunk 403a4743751bb6ae81abfb185abae13419ee0fb7", vrt=0.0,
    version="Varnish trunk 403a4743751bb6ae81abfb185abae13419ee0fb7"},
},

nigoroll avatar Mar 07 '24 14:03 nigoroll

Does it make sense to take it one step further and introduce a way for the vmod author add information to the panic? As we are already changing the format, would it be useful to call into a hook for the vmod to add internal state information?

Btw, this change seems to break a couple of CCI jobs:

/bin/python3.6 ../../../lib/libvcc/vmodtool.py --boilerplate -o vcc_debug_if ../../../vmod/vmod_debug.vcc
Traceback (most recent call last):
  File "../../../lib/libvcc/vmodtool.py", line 1315, in <module>
    runmain(i_vcc, opts.rstdir, opts.output)
  File "../../../lib/libvcc/vmodtool.py", line 1280, in runmain
    v.mkcfile()
  File "../../../lib/libvcc/vmodtool.py", line 1263, in mkcfile
    self.vmod_data(fo)
  File "../../../lib/libvcc/vmodtool.py", line 1215, in vmod_data
    fo.write('\t.version =\t%s,\n' % json.dumps(self.version()))
  File "../../../lib/libvcc/vmodtool.py", line 1188, in version
    for pkgstr in open(os.path.join(srcdir, "Makefile")):
FileNotFoundError: [Errno 2] No such file or directory: '../../../vmod/Makefile'

asadsa92 avatar Mar 07 '24 14:03 asadsa92

Btw, this change seems to break a couple of CCI jobs:

It breaks all jobs running make distcheck.

If anything, we should probably have a $Version stanza.

Projects using autoconf can rename their VCC files to vmod_foo.vcc.in and add something like $Version @PACKAGE@-@VERSION@ expanded at configure time (or something more elaborate that includes a git commit hash) after the $Module stanza.

And autoconf or not, we let VMOD authors figure what they want to put in the version information, and how.

dridi avatar Mar 07 '24 16:03 dridi

@asadsa92 I like the idea, and we could take it even further by adding panic callbacks on the vmod object layer. But I would like to do this in follow ups. Thank you for pointing out the issue with out-of-tree builds, this is addressed in b1270b3d4b304294eb209412041c72a14b4e8c40

nigoroll avatar Mar 07 '24 17:03 nigoroll

It breaks all jobs running make distcheck.

Yes, again, this was an oversight, sorry.

If anything, we should probably have a $Version stanza.

We sure can.

Projects using autoconf can rename their VCC files...

Please, no. Let's make this just work. Yes, we can have an override, but let's not push the burden downstream.

Dridi, you did the project a great service improving the m4 macros and with vcdk, which simplify vmod infrastructure, and we should absolutely follow along that route.

As someone diagnosing vmod issues, I do not want to depend on vmod authors to add infrastructure just to get this basic information. I want to know the vmod version and git rev if I can get it. If I can not get it for some cases, ok, we can handle that. As a VMOD author, I do not want more maintenance burdon. If we can provide a simple solution for most cases, please let's do this. And then provide a custom option for anything else.

nigoroll avatar Mar 07 '24 17:03 nigoroll

bugwash feedback summary:

  • git commit info (if any) should be carried over to a dist archive
  • something like a $Version stanza should allow to override the automatically determined version
  • it could be helpful to add a tool to gather version information of installed vmods outside varnishd (e.g. as an addition to contrib/)
  • vcc could add additional information about the original vmod path to the compiled so (external symbol table)

nigoroll avatar Mar 11 '24 14:03 nigoroll

FTR: This comment put this back on top of my table because oob communication clarified that users of pre-built containers do not know which vmod versions were used, exactly.

nigoroll avatar Jul 23 '24 06:07 nigoroll

I have made changes according to the bugwash feedback and force-pushed an update:

git commit info (if any) should be carried over to a dist archive

This is now done via the vmod_vcs_version.txt file, see doc/changes.rst for details.

something like a $Version stanza should allow to override the automatically determined version

Done

it could be helpful to add a tool to gather version information of installed vmods outside varnishd (e.g. as an addition to contrib/)

Done via an ELF section such that readelf -p.vmod_vcs <file> can be used

vcc could add additional information about the original vmod path to the compiled so (external symbol table)

This was already contained.

I have also changed the debug.vmod CLI command and turned the formerly single version string into two, the "user friendly" version and vcs to contain the git hash.

nigoroll avatar Jul 23 '24 14:07 nigoroll

Once again, I ran into a situation where the exact version of a vmod was unclear, so I would really appreciate if we made some progress here. I also rebased the PR.

nigoroll avatar Nov 29 '24 10:11 nigoroll