Skip to content

Add "X-cache: PASS" support in cacheHits module#166

Merged
macbre merged 4 commits into
macbre:masterfrom
kennydee:master
Dec 14, 2013
Merged

Add "X-cache: PASS" support in cacheHits module#166
macbre merged 4 commits into
macbre:masterfrom
kennydee:master

Conversation

@kennydee
Copy link
Copy Markdown
Contributor

Hi,

I don't know if this PR could have some interest for you, but in case ...

This PR add X-cache PASS metrics.

In our Varnish, we have HIT, MISS and PASS.

PASS tell us there is a rule which exclude cache on this request, and tell us which rules.

And PASS request, is a thing that we have to look, because it means that every hit go to the backend !

;) Thanks

Comment thread modules/cacheHits/cacheHits.js Outdated
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

As toLowerCase() is now called twice on header I'd move the call here.

@macbre
Copy link
Copy Markdown
Owner

macbre commented Dec 13, 2013

Makes perfect sense to me :)

Please run JS Lint against your changes by npm run-script lint - it reports mixed tabs and spaces.

Comment thread README.md
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please update information about number of metrics above (it got ++)

@ghost ghost assigned macbre Dec 13, 2013
Comment thread modules/cacheHits/cacheHits.js Outdated
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You may want to use offenders (see #140) for reporting requests being passed to the backend. Simply call phantomas.addOffender('cachePasses', entry.url);

@macbre
Copy link
Copy Markdown
Owner

macbre commented Dec 14, 2013

Way better :) Thanks, @kennydee!

macbre added a commit that referenced this pull request Dec 14, 2013
Add "X-cache: PASS" support in cacheHits module
@macbre macbre merged commit 76adcdd into macbre:master Dec 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants