Skip to content

Implement vhost_traffic_status_upstream_no_cache filter - #194

Closed
heikkiorsila wants to merge 3 commits into
vozlt:masterfrom
Seravo:master
Closed

Implement vhost_traffic_status_upstream_no_cache filter#194
heikkiorsila wants to merge 3 commits into
vozlt:masterfrom
Seravo:master

Conversation

@heikkiorsila

Copy link
Copy Markdown

This enables monitoring upstream backend events that are configured not to
use cache.

Add the following line to nginx config to create a group:

    vhost_traffic_status_filter_upstream_no_cache "NO_CACHE";

@vozlt

vozlt commented Sep 4, 2022

Copy link
Copy Markdown
Owner

Thanks for the pull request for improvement. But do you need to put non-cached statistics inside serverZones? As far as I know you can already use the information(miss, bypass, expired, stale, updating, revalidated, hit, scarce) in serverZones to calculate statistics for non-cached items. I think it is correct to calculate it through a 3rd party program.

@vozlt
vozlt requested review from SuperQ and u5surf September 4, 2022 16:04

@u5surf u5surf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @heikkiorsila
Thanks feature PR. We didn’t find obviously what to expect on this feature. Because I also couldn’t find any differences between vhost_traffic_status_zone and this feature.

Can you add some tests or share your scenarios to explain your expectation on this feature? It is necessary for us and our users to make sense this feature properly.

vozlt and others added 3 commits October 3, 2022 09:40
…g the ngx_http_vhost_traffic_status_display_get_size() function
* To reduce the ci build time
This enables monitoring upstream backend events that are configured not to
use cache.

Add the following line to nginx config to create a group:

        vhost_traffic_status_filter_upstream_no_cache "NO_CACHE";
@u5surf

u5surf commented Oct 18, 2022

Copy link
Copy Markdown
Collaborator

@ypcs Hi, Thanks following about this PR.

We almost agree with what @vozlt said the below

But do you need to put non-cached statistics inside serverZones? As far as I know you can already use the information(miss, bypass, expired, stale, updating, revalidated, hit, scarce) in serverZones to calculate statistics for non-cached items. I think it is correct to calculate it through a 3rd party program.

Do you have any another reasons that the feature is necessary for this modules?
And can you add some tests or share your expected scenarios to explain the necessity on this feature?
We'll judge based on it whether this PR should merge or not.

@ypcs

ypcs commented Oct 19, 2022

Copy link
Copy Markdown

Hi. We're currently evaluating if this is necessary, I'll post update later, and we'll provide tests then.

@u5surf

u5surf commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Thank you for this, and I am sorry it sat unanswered for so long — the last
word here was ours in 2022 asking for a use case, and then nothing from
either side for years. That is on us.

Coming back to it now, I think @vozlt's original question was the right one,
and I can put a concrete answer behind it: what this directive produces can
already be configured, and the configured version gives more.

The existing mechanism

$upstream_cache_status is empty when the cache module was not involved, and
this module drops nodes with an empty key — which is, I suspect, why it did
not look like an option. Giving that empty value a name is enough:

map $upstream_cache_status $vts_cache {
    ""      "NO_CACHE";
    default $upstream_cache_status;
}

location /nocache {
    vhost_traffic_status_filter_by_set_key $vts_cache cachestatus::$server_name;
    proxy_pass http://backend;
}

Three requests to a cached location, five to an uncached one, two to a static
file:

'HIT'        requests=2 in=  168 out=  644 2xx=2
'MISS'       requests=1 in=   84 out=  322 2xx=1
'NO_CACHE'   requests=5 in=  425 out= 1610 2xx=5
serverZone front requests = 10

The NO_CACHE figure is the one this pull request adds. The breakdown by
cache status comes with it at no extra cost, and because
vhost_traffic_status_filter_by_set_key takes effect per location, the two
static requests stay out of it — the patch counts by r->upstream != NULL,
which cannot make that distinction where a location proxies some requests and
serves others itself.

On the patch itself

Two smaller things, in case the approach is ever revisited. The directive is
named filter_... but the node it creates is a server zone
(NGX_HTTP_VHOST_TRAFFIC_STATUS_UPSTREAM_NO), so NO_CACHE appears in
serverZones beside real host names and would collide with a server actually
named that. And when it is enabled, every request does shm_add_server()
twice — two rbtree searches and two acquisitions of the zone mutex instead of
one.

Also worth noting that the branch has aged past two of its seven files: the
cpanm --notest line and the ngx_shmtx_lock() around shm_info() in
display_get_size() are both in master already.

Closing

Since the result is reachable from configuration today, I would rather not
add a directive for it, so I am closing this. That is not a judgement on the
need — monitoring uncached backend traffic is a reasonable thing to want, and
if the configuration above does not cover your case, please say so and I will
reopen. Thanks again for taking the time to write it.

@u5surf u5surf closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants