From c4fc1a34e4dff495368d151f8c11d4d1642b7537 Mon Sep 17 00:00:00 2001 From: Norbert Hartl Date: Thu, 17 Sep 2026 22:17:23 +0200 Subject: [PATCH] Let the Zinc counter be narrowed to a path The deployed servers count only what arrives under /api - AGBaseUriSpace sets that prefix before installing the counter. Canopy's counter subscribed to every transaction, so after the migration zinc_httpRequests and zinc_httpResponses would have carried materially different numbers, the scrapes of the metrics endpoint among them. pharo-metrics says as much where it filters: left alone, a counter of HTTP requests counts the requests that read it. Two lists rather than the single flag pharo-metrics uses, where addInclude and addExclude overwrite each other's mode and the last call wins: named prefixes narrow what is counted, excluded ones drop out of what is left, and with neither everything counts. That covers what the servers do and has no order in which it means something else. Co-Authored-By: Claude Opus 5 (1M context) --- .../CanopyZincCounterTest.class.st | 28 ++++++++++++++++ .../CanopyZincTestAnnouncement.class.st | 15 ++++++++- .../Canopy-Metrics/CanopyZincCounter.class.st | 32 ++++++++++++++++++- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/source/Canopy-Metrics-Tests/CanopyZincCounterTest.class.st b/source/Canopy-Metrics-Tests/CanopyZincCounterTest.class.st index 9754eec..6ab2b84 100644 --- a/source/Canopy-Metrics-Tests/CanopyZincCounterTest.class.st +++ b/source/Canopy-Metrics-Tests/CanopyZincCounterTest.class.st @@ -5,6 +5,34 @@ Class { #package : 'Canopy-Metrics-Tests' } +{ #category : 'as yet unclassified' } +CanopyZincCounterTest >> testAnExcludedPrefixIsNotCounted [ + "The scrape endpoint is the obvious one to leave out: a counter of HTTP requests that counts + the requests reading it measures partly itself." + | counter | + counter := CanopyZincCounter new. + counter addExcludePrefix: '/metrics'. + counter handleAnnouncement: (CanopyZincTestAnnouncement new url: '/metrics'; responseCode: 200 duration: 5; yourself). + self assert: counter requests equals: 0. + counter handleAnnouncement: (CanopyZincTestAnnouncement new url: '/api/grids'; responseCode: 200 duration: 5; yourself). + self assert: counter requests equals: 1 +] + +{ #category : 'as yet unclassified' } +CanopyZincCounterTest >> testOnlyRequestsUnderAnIncludedPrefixAreCounted [ + "What the deployed servers do: narrow the count to '/api'. Without it Canopy counted every + request the image ever answered, which is not the number the dashboards have been showing." + | counter | + counter := CanopyZincCounter new. + counter addIncludePrefix: '/api'. + counter handleAnnouncement: (CanopyZincTestAnnouncement new url: '/status'; responseCode: 200 duration: 5; yourself). + self assert: counter requests equals: 0. + self assert: (counter responses at: 200) equals: 0. + counter handleAnnouncement: (CanopyZincTestAnnouncement new url: '/api/grids'; responseCode: 200 duration: 5; yourself). + self assert: counter requests equals: 1. + self assert: (counter responses at: 200) equals: 1 +] + { #category : 'as yet unclassified' } CanopyZincCounterTest >> testAFreshCounterAlreadyCarriesTheUsualStatusCodes [ "Parity with what the servers export today: the status series exist from the first scrape at diff --git a/source/Canopy-Metrics-Tests/CanopyZincTestAnnouncement.class.st b/source/Canopy-Metrics-Tests/CanopyZincTestAnnouncement.class.st index 4605389..4e9194f 100644 --- a/source/Canopy-Metrics-Tests/CanopyZincTestAnnouncement.class.st +++ b/source/Canopy-Metrics-Tests/CanopyZincTestAnnouncement.class.st @@ -3,7 +3,8 @@ Class { #superclass : 'Object', #instVars : [ 'responseCode', - 'duration' + 'duration', + 'url' ], #category : 'Canopy-Metrics-Tests', #package : 'Canopy-Metrics-Tests' @@ -19,6 +20,18 @@ CanopyZincTestAnnouncement >> responseCode [ ^ responseCode ] +{ #category : 'accessing' } +CanopyZincTestAnnouncement >> url [ + "Defaults to a path the deployed servers count, so a test that does not care about filtering + does not have to say anything about it." + ^ url ifNil: [ '/api/grids' asZnUrl ] +] + +{ #category : 'accessing' } +CanopyZincTestAnnouncement >> url: aString [ + url := aString asZnUrl +] + { #category : 'accessing' } CanopyZincTestAnnouncement >> responseCode: aCode duration: aDuration [ responseCode := aCode. diff --git a/source/Canopy-Metrics/CanopyZincCounter.class.st b/source/Canopy-Metrics/CanopyZincCounter.class.st index c4ff648..92b906b 100644 --- a/source/Canopy-Metrics/CanopyZincCounter.class.st +++ b/source/Canopy-Metrics/CanopyZincCounter.class.st @@ -5,7 +5,9 @@ Class { 'requests', 'responses', 'duration', - 'registration' + 'registration', + 'includePrefixes', + 'excludePrefixes' ], #classInstVars : [ 'instance' @@ -40,6 +42,20 @@ CanopyZincCounter class >> uninstall [ ^ self instance uninstall ] +{ #category : 'configuring' } +CanopyZincCounter >> addExcludePrefix: aString [ + "Do not count requests to this path. The scrape endpoint itself is the obvious candidate - + left alone, a counter of HTTP requests counts the requests that read it." + excludePrefixes add: aString +] + +{ #category : 'configuring' } +CanopyZincCounter >> addIncludePrefix: aString [ + "Count only requests to this path. As soon as one is named, everything else is ignored - + the deployed servers narrow the count to '/api' this way." + includePrefixes add: aString +] + { #category : 'counting' } CanopyZincCounter >> adjustDuration: aDuration [ duration ifNil: [ ^ duration := aDuration ]. @@ -60,8 +76,20 @@ CanopyZincCounter >> durationCanopy [ value: self duration ] +{ #category : 'counting' } +CanopyZincCounter >> countsRequestTo: aUrl [ + "Named prefixes narrow what is counted, excluded ones drop out of whatever is left. Both + lists are honoured; with neither, everything counts." + | pfad | + pfad := aUrl pathPrintString. + (excludePrefixes anySatisfy: [ :each | pfad beginsWith: each ]) ifTrue: [ ^ false ]. + includePrefixes ifEmpty: [ ^ true ]. + ^ includePrefixes anySatisfy: [ :each | pfad beginsWith: each ] +] + { #category : 'counting' } CanopyZincCounter >> handleAnnouncement: anAnnouncement [ + (self countsRequestTo: anAnnouncement url) ifFalse: [ ^ self ]. self increaseRequests. self increaseStatusCode: anAnnouncement responseCode. self adjustDuration: anAnnouncement duration @@ -85,6 +113,8 @@ CanopyZincCounter >> initialize [ requests := 0. duration := 0. responses := Dictionary new. + includePrefixes := OrderedCollection new. + excludePrefixes := OrderedCollection new. self initializeResponses ]