New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

varnishstat -1 -f field inclusion glob doesn't allow VBE backend fields #2022

Closed
1davidmichael opened this Issue Jul 20, 2016 · 7 comments

Comments

Projects
None yet
6 participants
@1davidmichael

1davidmichael commented Jul 20, 2016

Expected Behavior

When trying to view individual statistics about a backend I would expect to be able to run
sudo varnishstat -1 -f "VBE.boot.default.happy" OR sudo varnishstat -1 -f VBE\.boot\.default\.happy and get the correct results.

This works with similar commands:
sudo varnishstat -1 -f LCK.wstat.locks LCK.wstat.locks 55950 0.32 Lock Operations

Current Behavior

When running these commands now I get the error -f: Wrong number of elements.

Steps to Reproduce (for bugs)

Run varnish with multiple backends, attempt to query individual backend fields with varnishstat -f command.

Context

I am trying to configure zabbix autodiscovery and monitoring of individual backends. The ability to run this for each backend would make it easier.

Your Environment

  • Version used: varnishstat (varnish-4.1.2 revision 0d7404e) and varnishstat (varnish-4.0.3 revision b8c4a34)
  • Operating System and version: Tested on both RHEL 7.1 and Archlinux
@Dridi

This comment has been minimized.

Show comment
Hide comment
@Dridi

Dridi Jul 21, 2016

Member

Confirmed for master.

varnishtest ""

server s1 "" -start
varnish v1 -vcl+backend "" -start

shell {varnishstat -n ${v1_name} -1 | grep VBE.vcl1.s1.happy >/dev/null}

shell {varnishstat -n ${v1_name} -1 -f LCK.wstat.locks >/dev/null}
shell {varnishstat -n ${v1_name} -1 -f VBE.vcl1.s1.happy >/dev/null}
Member

Dridi commented Jul 21, 2016

Confirmed for master.

varnishtest ""

server s1 "" -start
varnish v1 -vcl+backend "" -start

shell {varnishstat -n ${v1_name} -1 | grep VBE.vcl1.s1.happy >/dev/null}

shell {varnishstat -n ${v1_name} -1 -f LCK.wstat.locks >/dev/null}
shell {varnishstat -n ${v1_name} -1 -f VBE.vcl1.s1.happy >/dev/null}
@fgsch

This comment has been minimized.

Show comment
Hide comment
@fgsch

fgsch Jul 21, 2016

Member

field assumes type.name or type.ident.name.
In the case of backends, e.g. VBE.boot.default.happy, type is VBE, ident boot.default and name happy.
This breaks the assumption in vsc.c about having a max of 3 periods (.).
You can still do things like -f "VBE.*.happy" or -f "VBE.*" but you cannot do -f "VBE.bo*" or the full string.

Member

fgsch commented Jul 21, 2016

field assumes type.name or type.ident.name.
In the case of backends, e.g. VBE.boot.default.happy, type is VBE, ident boot.default and name happy.
This breaks the assumption in vsc.c about having a max of 3 periods (.).
You can still do things like -f "VBE.*.happy" or -f "VBE.*" but you cannot do -f "VBE.bo*" or the full string.

@Dridi

This comment has been minimized.

Show comment
Hide comment
@Dridi

Dridi Jul 21, 2016

Member

Workaround: varnishstat -1 -f "VBE?boot.default.happy"

It you want the happy counter for all the backends in your boot VCL:
varnishstat -1 -f "VBE?boot.*.happy"

Member

Dridi commented Jul 21, 2016

Workaround: varnishstat -1 -f "VBE?boot.default.happy"

It you want the happy counter for all the backends in your boot VCL:
varnishstat -1 -f "VBE?boot.*.happy"

@Dridi

This comment has been minimized.

Show comment
Hide comment
@Dridi

Dridi Jul 21, 2016

Member

This breaks the assumption in vsc.c about having a max of 3 periods (.).

@r0b0ticus also reported this problem for 4.0.3 (haven't checked yet) where backends can also contain an IPv4 address. Does it mean that we can't filter backends based on an IP address with -f?

I haven't looked at the details in vsc.c yet, but it sounds like an assumption that shouldn't be made.

Member

Dridi commented Jul 21, 2016

This breaks the assumption in vsc.c about having a max of 3 periods (.).

@r0b0ticus also reported this problem for 4.0.3 (haven't checked yet) where backends can also contain an IPv4 address. Does it mean that we can't filter backends based on an IP address with -f?

I haven't looked at the details in vsc.c yet, but it sounds like an assumption that shouldn't be made.

@fgsch

This comment has been minimized.

Show comment
Hide comment
@fgsch

fgsch Jul 21, 2016

Member

Sorry, not 3 periods but 3 parts, which corresponds to type, name an indent.
I guess having a period within a particular field was not considered / originally supported.

Member

fgsch commented Jul 21, 2016

Sorry, not 3 periods but 3 parts, which corresponds to type, name an indent.
I guess having a period within a particular field was not considered / originally supported.

@nigoroll

This comment has been minimized.

Show comment
Hide comment
@nigoroll

nigoroll Jul 25, 2016

Contributor

bugwash: @Dridi to look after this, change implementation to be just a glob match on the stats string, no special handling of dots

Contributor

nigoroll commented Jul 25, 2016

bugwash: @Dridi to look after this, change implementation to be just a glob match on the stats string, no special handling of dots

fgsch added a commit to fgsch/varnish-cache that referenced this issue Sep 11, 2016

fgsch added a commit to fgsch/varnish-cache that referenced this issue Sep 11, 2016

@fgsch fgsch assigned fgsch and unassigned Dridi Sep 12, 2016

@fgsch fgsch closed this in 8bb7e90 Sep 12, 2016

@hermunn

This comment has been minimized.

Show comment
Hide comment
@hermunn

hermunn Oct 12, 2016

Contributor

Backport review: This will not be backported to 4.1, as the workarounds seem good enough to me.

Contributor

hermunn commented Oct 12, 2016

Backport review: This will not be backported to 4.1, as the workarounds seem good enough to me.

mbgrydeland added a commit that referenced this issue Jun 15, 2017

Switch to globbing on the whole string
Fixes #2022

Conflicts:
	bin/varnishstat/varnishstat_options.h

Backport to 4.1

ibreger added a commit to thomsonreuters/varnish-cache that referenced this issue Jun 27, 2017

Switch to globbing on the whole string
Fixes #2022

Conflicts:
	bin/varnishstat/varnishstat_options.h

Backport to 4.1

wmfgerrit pushed a commit to wikimedia/operations-debs-varnish4 that referenced this issue Jun 29, 2017

4.1.7-1wm1: new upstream, new counters
Package varnish 4.1.7, add counters for transient storage.

Introduce a new counter for shortlived objects creation,
cache_shortlived, and another one for uncacheable responses,
cache_uncacheable. They should provide insights when it comes to
monitoring transient storage usage.

Changes in 4.1.7:

 - Correctly honor nuke_limit parameter
   varnishcache/varnish-cache#1764
 - Prevent storage backends name collisions
   varnishcache/varnish-cache#2321
 - varnishstat -1 -f field inclusion glob doesn't allow VBE backend fields
   varnishcache/varnish-cache#2022
 - Health probes fail when HTTP response does not contain reason phrase
   varnishcache/varnish-cache#2069
 - "varnishstat -f MAIN.sess_conn -1" produces empty output
   varnishcache/varnish-cache#2118
 - Remember to reset workspace
   varnishcache/varnish-cache#2219
 - Rework and fix varnishstat counter filtering
   varnishcache/varnish-cache#2320
 - Docfix: Only root can jail
   varnishcache/varnish-cache#2329
 - Don't panic on a null ban
 - Add extra locking to protect the pools list and refcounts
 - Add -vsl_catchup to varnishtest
 - Add record-prefix support to varnishncsa

Bug: T164768
Ref: https://github.com/varnishcache/varnish-cache/blob/4.1/doc/changes.rst#varnish-cache-417-2017-06-28
Change-Id: I8a8f3a8103feb83b1a55a6788ea6c5d12963b4f5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment