Skip to content

Commit

Permalink
Add breakdown per org on stats metrics when a date is specified (#517)
Browse files Browse the repository at this point in the history


Signed-off-by: Paolo Di Tommaso <[email protected]>
Co-authored-by: Paolo Di Tommaso <[email protected]>
  • Loading branch information
munishchouhan and pditommaso authored May 31, 2024
1 parent de97575 commit b24894a
Show file tree
Hide file tree
Showing 11 changed files with 209 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,28 +65,25 @@ class MetricsController {
@Get(uri = "/v1alpha2/metrics/builds", produces = MediaType.APPLICATION_JSON)
HttpResponse<?> getBuildsMetrics(@Nullable @QueryValue String date, @Nullable @QueryValue String org) {
if(!date && !org)
return HttpResponse.ok(metricsService.getOrgCount(MetricConstants.PREFIX_BUILDS))
return HttpResponse.ok(metricsService.getAllOrgCount(MetricConstants.PREFIX_BUILDS))
validateQueryParams(date)
final count = metricsService.getBuildsMetrics(date, org)
return HttpResponse.ok(new GetBuildsCountResponse(count))
return HttpResponse.ok(metricsService.getOrgCount(MetricConstants.PREFIX_BUILDS, date, org))
}

@Get(uri = "/v1alpha2/metrics/pulls", produces = MediaType.APPLICATION_JSON)
HttpResponse<?> getPullsMetrics(@Nullable @QueryValue String date, @Nullable @QueryValue String org) {
if(!date && !org)
return HttpResponse.ok(metricsService.getOrgCount(MetricConstants.PREFIX_PULLS))
return HttpResponse.ok(metricsService.getAllOrgCount(MetricConstants.PREFIX_PULLS))
validateQueryParams(date)
final count = metricsService.getPullsMetrics(date, org)
return HttpResponse.ok(new GetPullsCountResponse(count))
return HttpResponse.ok(metricsService.getOrgCount(MetricConstants.PREFIX_PULLS, date, org))
}

@Get(uri = "/v1alpha2/metrics/fusion/pulls", produces = MediaType.APPLICATION_JSON)
HttpResponse<?> getFusionPullsMetrics(@Nullable @QueryValue String date, @Nullable @QueryValue String org) {
if(!date && !org)
return HttpResponse.ok(metricsService.getOrgCount(MetricConstants.PREFIX_FUSION))
return HttpResponse.ok(metricsService.getAllOrgCount(MetricConstants.PREFIX_FUSION))
validateQueryParams(date)
final count = metricsService.getFusionPullsMetrics(date, org)
return HttpResponse.ok(new GetFusionPullsCountResponse(count))
return HttpResponse.ok(metricsService.getOrgCount(MetricConstants.PREFIX_FUSION, date, org))

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package io.seqera.wave.service.counter.impl

import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.atomic.AtomicLong
import java.util.regex.Pattern

import groovy.transform.CompileStatic
import io.micronaut.context.annotation.Requires
Expand Down Expand Up @@ -49,10 +50,11 @@ class LocalCounterProvider implements CounterProvider {

@Override
Map<String, Long> getAllMatchingEntries(String key, String pattern) {
def keyStore = store.get(key)
Pattern compiledPattern = Pattern.compile(pattern.replace('*', '.*'))
Map keyStore = store.get(key)
Map<String, Long> result = [:]
if (keyStore){
def matchingPairs = keyStore.findAll { k, v -> k =~ pattern }
def matchingPairs = keyStore.findAll {entry -> compiledPattern.matcher(entry.key).matches()}
matchingPairs.each { k, v ->
result.put(k, v as Long)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,33 +26,6 @@ import io.seqera.wave.tower.PlatformId
* @author Munish Chouhan <[email protected]>
*/
interface MetricsService {
/**
* get the Wave builds metrics
*
* @param date, date of the required metrics
* @param org, org of the required metrics
* @return Long, builds counts
*/
Long getBuildsMetrics(String date, String org)

/**
* get the Wave pulls metrics
*
* @param date, date of the required metrics
* @param org, org of the required metrics
* @return Long, pulls counts
*/
Long getPullsMetrics(String date, String org)

/**
* get the Wave fusion pulls metrics
*
* @param date, date of the required metrics
* @param org, org of the required metrics
* @return Long, fusion pulls counts
*/
Long getFusionPullsMetrics(String date, String org)

/**
* increment wave fusion pulls count
*
Expand Down Expand Up @@ -80,5 +53,13 @@ interface MetricsService {
* @param metric
* @return GetOrgCountResponse
*/
GetOrgCountResponse getOrgCount(String metric)
GetOrgCountResponse getAllOrgCount(String metric)

/**
* Get counts per organisations or per date or per both
*
* @param metric
* @return GetOrgCountResponse
*/
GetOrgCountResponse getOrgCount(String metric, String date, String org)
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ package io.seqera.wave.service.metric.impl

import java.time.LocalDate
import java.time.format.DateTimeFormatter
import java.util.regex.Matcher
import java.util.regex.Pattern

import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j
Expand All @@ -40,28 +42,15 @@ import jakarta.inject.Singleton
@CompileStatic
class MetricsServiceImpl implements MetricsService {

@Inject
private MetricsCounterStore metricsCounterStore

DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd")

@Override
Long getBuildsMetrics(String date, String org) {
return metricsCounterStore.get(getKey(MetricConstants.PREFIX_BUILDS, date, org)) ?: 0
}
static final private DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd")

@Override
Long getPullsMetrics(String date, String org) {
return metricsCounterStore.get(getKey(MetricConstants.PREFIX_PULLS, date, org)) ?: 0
}
static final private Pattern ORF_DATE_KEY_PATTERN = Pattern.compile('(builds|pulls|fusion)/o/([^/]+)/d/\\d{4}-\\d{2}-\\d{2}')

This comment has been minimized.

Copy link
@munishchouhan

munishchouhan May 31, 2024

Author Member

@pditommaso i think it should be ORG in ORF_DATE_KEY_PATTERN?

This comment has been minimized.

Copy link
@pditommaso

pditommaso May 31, 2024

Author Collaborator

Sure, the change please


@Override
Long getFusionPullsMetrics(String date, String org) {
return metricsCounterStore.get(getKey(MetricConstants.PREFIX_FUSION, date, org)) ?: 0
}
@Inject
private MetricsCounterStore metricsCounterStore

@Override
GetOrgCountResponse getOrgCount(String metric){
GetOrgCountResponse getAllOrgCount(String metric){
final response = new GetOrgCountResponse(metric, 0, [:])
final orgCounts = metricsCounterStore.getAllMatchingEntries("$metric/$MetricConstants.PREFIX_ORG/*")
for(def entry : orgCounts) {
Expand All @@ -75,6 +64,28 @@ class MetricsServiceImpl implements MetricsService {
return response
}

@Override
GetOrgCountResponse getOrgCount(String metric, String date, String org) {
final response = new GetOrgCountResponse(metric, 0, [:])

// count is stored per date and per org, so it can be extracted from get method
response.count = metricsCounterStore.get(getKey(metric, date, org)) ?: 0L

//when org and date is provided, return the org count for given date
if (org) {
response.orgs.put(org, response.count)
}else{
// when only date is provide, scan the store and return the count for all orgs on given date
final orgCounts = metricsCounterStore.getAllMatchingEntries("$metric/$MetricConstants.PREFIX_ORG/*/$MetricConstants.PREFIX_DAY/$date")
for(def entry : orgCounts) {
response.orgs.put(extractOrgFromKey(entry.key), entry.value)
}
}

return response

}

@Override
void incrementFusionPullsCounter(PlatformId platformId){
incrementCounter(MetricConstants.PREFIX_FUSION, platformId?.user?.email)
Expand All @@ -92,14 +103,14 @@ class MetricsServiceImpl implements MetricsService {

protected void incrementCounter(String prefix, String email) {
def org = getOrg(email)
def key = getKey(prefix, LocalDate.now().format(dateFormatter), null)
def key = getKey(prefix, LocalDate.now().format(DATE_FORMATTER), null)
metricsCounterStore.inc(key)
log.trace("increment metrics count of: $key")
if ( org ) {
key = getKey(prefix, null, org)
metricsCounterStore.inc(key)
log.trace("increment metrics count of: $key")
key = getKey(prefix, LocalDate.now().format(dateFormatter), org)
key = getKey(prefix, LocalDate.now().format(DATE_FORMATTER), org)
metricsCounterStore.inc(key)
log.trace("increment metrics count of: $key")
}
Expand Down Expand Up @@ -127,4 +138,8 @@ class MetricsServiceImpl implements MetricsService {
return null
}

protected static String extractOrgFromKey(String key) {
Matcher matcher = ORF_DATE_KEY_PATTERN.matcher(key)
return matcher.matches() ? matcher.group(2) : "unknown"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,23 +75,23 @@ class MetricsControllerTest extends Specification {
def res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 3]
res.body() == [metric:'builds', count:3, orgs:['org1.com': 1, 'org2.com': 1]]
res.status.code == 200

when: 'date and org is provided'
req = HttpRequest.GET("/v1alpha2/metrics/builds?date=$date&org=org1.com").basicAuth("username", "password")
res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 1]
res.body() == [metric:'builds', count:1, orgs:['org1.com': 1]]
res.status.code == 200

when: 'only org is provided'
req = HttpRequest.GET("/v1alpha2/metrics/builds?org=org1.com").basicAuth("username", "password")
res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 1]
res.body() == [metric:'builds', count:1, orgs:['org1.com': 1]]
res.status.code == 200

when: 'no param is provided'
Expand Down Expand Up @@ -119,23 +119,23 @@ class MetricsControllerTest extends Specification {
def res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 3]
res.body() == [metric:'pulls', count:3, orgs:['org1.com': 1, 'org2.com': 1]]
res.status.code == 200

when: 'date and org is provided'
req = HttpRequest.GET("/v1alpha2/metrics/pulls?date=$date&org=org2.com").basicAuth("username", "password")
req = HttpRequest.GET("/v1alpha2/metrics/pulls?date=$date&org=org1.com").basicAuth("username", "password")
res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 1]
res.body() == [metric:'pulls', count:1, orgs:['org1.com': 1]]
res.status.code == 200

when: 'only org is provided'
req = HttpRequest.GET("/v1alpha2/metrics/pulls?org=org2.com").basicAuth("username", "password")
res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 1]
res.body() == [metric:'pulls', count:1, orgs:['org2.com': 1]]
res.status.code == 200

when: 'no param is provided'
Expand Down Expand Up @@ -163,23 +163,23 @@ class MetricsControllerTest extends Specification {
def res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 3]
res.body() == [metric:'fusion', count:3, orgs:['org1.com': 1, 'org2.com': 1]]
res.status.code == 200

when: 'date and org is provided'
req = HttpRequest.GET("/v1alpha2/metrics/fusion/pulls?date=$date&org=org2.com").basicAuth("username", "password")
req = HttpRequest.GET("/v1alpha2/metrics/fusion/pulls?date=$date&org=org1.com").basicAuth("username", "password")
res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 1]
res.body() == [metric:'fusion', count:1, orgs:['org1.com': 1]]
res.status.code == 200

when: 'only org is provided'
req = HttpRequest.GET("/v1alpha2/metrics/fusion/pulls?org=org2.com").basicAuth("username", "password")
res = client.toBlocking().exchange(req, Map)

then: 'should get the correct count'
res.body() == [count: 1]
res.body() == [metric:'fusion', count:1, orgs:['org2.com': 1]]
res.status.code == 200

when: 'no param is provided'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class PullMetricsRequestsFilterTest extends Specification {
def res = httpClient.toBlocking().exchange(req, Map)

then:
res.body() == [count: 1]
res.body() == [metric:'pulls', count: 1]
res.status.code == 200
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,15 @@ class LocalCounterProviderTest extends Specification {
localCounterProvider.inc('metrics/v1', 'pulls/o/foo.it', 1)
localCounterProvider.inc('metrics/v1', 'pulls/o/bar.es', 2)
localCounterProvider.inc('metrics/v1', 'pulls/o/abc.in', 3)
localCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/date/yyyy-mm-dd', 1)
localCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-05-30', 1)
localCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-05-31', 1)

then:
localCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') ==
['pulls/o/abc.com.au/date/yyyy-mm-dd':1, 'pulls/o/abc.in':3, 'pulls/o/bar.es':2, 'pulls/o/foo.it':1]
['pulls/o/abc.in':3, 'pulls/o/bar.es':2, 'pulls/o/foo.it':1, 'pulls/o/abc.com.au/d/2024-05-30':1, 'pulls/o/abc.com.au/d/2024-05-31':1]
and:
localCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') ==
['pulls/o/abc.com.au/d/2024-05-30':1]
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,19 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain
def 'should get correct org count' () {
when:
redisCounterProvider.inc('metrics/v1', 'builds/o/foo.com', 1)
redisCounterProvider.inc('metrics/v1', 'builds/o/bar.org', 1)
redisCounterProvider.inc('metrics/v1', 'builds/o/abc.it', 2)
redisCounterProvider.inc('metrics/v1', 'pulls/o/foo.es', 1)
redisCounterProvider.inc('metrics/v1', 'pulls/o/bar.in', 2)
redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.au', 3)
redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com/date/yyyy-mm-dd', 1)
redisCounterProvider.inc('metrics/v1', 'builds/o/bar.io', 1)
redisCounterProvider.inc('metrics/v1', 'builds/o/abc.org', 2)
redisCounterProvider.inc('metrics/v1', 'pulls/o/foo.it', 1)
redisCounterProvider.inc('metrics/v1', 'pulls/o/bar.es', 2)
redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.in', 3)
redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-05-30', 1)
redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-05-31', 1)

then:
redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') ==
['pulls/o/foo.es':1, 'pulls/o/bar.in':2, 'pulls/o/abc.au':3, 'pulls/o/abc.com/date/yyyy-mm-dd': 1]
['pulls/o/abc.in':3, 'pulls/o/bar.es':2, 'pulls/o/foo.it':1, 'pulls/o/abc.com.au/d/2024-05-30':1, 'pulls/o/abc.com.au/d/2024-05-31':1]
and:
redisCounterProvider.getAllMatchingEntries('metrics/v1', 'fusion/o/*') == [:]
redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') ==
['pulls/o/abc.com.au/d/2024-05-30':1]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ package io.seqera.wave.service.metric

import spock.lang.Specification

import java.time.LocalDate
import java.time.format.DateTimeFormatter

import io.micronaut.test.extensions.spock.annotation.MicronautTest
import jakarta.inject.Inject
/**
Expand All @@ -32,6 +35,8 @@ class MetricsCounterStoreLocalTest extends Specification {
@Inject
MetricsCounterStore metricsCounterStore

DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd")

def 'should get correct count value' () {
when:
metricsCounterStore.inc('foo')
Expand All @@ -48,9 +53,16 @@ class MetricsCounterStoreLocalTest extends Specification {
metricsCounterStore.inc('builds/o/foo.com')
metricsCounterStore.inc('builds/o/bar.org')
metricsCounterStore.inc('pulls/o/bar.in')
metricsCounterStore.inc('pulls/o/foo.com/d/2024-05-29')
metricsCounterStore.inc('builds/o/bar.org/d/2024-05-30')
metricsCounterStore.inc('fusion/o/bar.in/d/2024-05-30')
metricsCounterStore.inc('pulls/o/bar.in/d/2024-05-31')

then:
metricsCounterStore.getAllMatchingEntries('builds/o*') == ['builds/o/foo.com':1, 'builds/o/bar.org':1]
metricsCounterStore.getAllMatchingEntries('pulls/o*') == ['pulls/o/bar.in':1]
metricsCounterStore.getAllMatchingEntries('builds/o/*') == ['builds/o/foo.com':1, 'builds/o/bar.org':1, 'builds/o/bar.org/d/2024-05-30':1]
metricsCounterStore.getAllMatchingEntries('pulls/o/*') == ['pulls/o/bar.in':1, 'pulls/o/foo.com/d/2024-05-29':1, 'pulls/o/bar.in/d/2024-05-31':1]
metricsCounterStore.getAllMatchingEntries('fusion/o/*') == ['fusion/o/bar.in/d/2024-05-30':1]
metricsCounterStore.getAllMatchingEntries('builds/o/*/d/2024-05-30') == ['builds/o/bar.org/d/2024-05-30':1]
metricsCounterStore.getAllMatchingEntries('pulls/o/bar.in/d/2024-05-31') == ['pulls/o/bar.in/d/2024-05-31':1]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,16 @@ class MetricsCounterStoreRedisTest extends Specification implements RedisTestCo
metricsCounterStore.inc('builds/o/foo.com')
metricsCounterStore.inc('builds/o/bar.org')
metricsCounterStore.inc('pulls/o/bar.in')
metricsCounterStore.inc('pulls/o/foo.com/d/2024-05-29')
metricsCounterStore.inc('builds/o/bar.org/d/2024-05-30')
metricsCounterStore.inc('fusion/o/bar.in/d/2024-05-30')
metricsCounterStore.inc('pulls/o/bar.in/d/2024-05-31')

then:
metricsCounterStore.getAllMatchingEntries('builds/o*') == ['builds/o/foo.com':1, 'builds/o/bar.org':1]
metricsCounterStore.getAllMatchingEntries('pulls/o*') == ['pulls/o/bar.in':1]
metricsCounterStore.getAllMatchingEntries('builds/o/*') == ['builds/o/foo.com':1, 'builds/o/bar.org':1, 'builds/o/bar.org/d/2024-05-30':1]
metricsCounterStore.getAllMatchingEntries('pulls/o/*') == ['pulls/o/bar.in':1, 'pulls/o/foo.com/d/2024-05-29':1, 'pulls/o/bar.in/d/2024-05-31':1]
metricsCounterStore.getAllMatchingEntries('fusion/o/*') == ['fusion/o/bar.in/d/2024-05-30':1]
metricsCounterStore.getAllMatchingEntries('builds/o/*/d/2024-05-30') == ['builds/o/bar.org/d/2024-05-30':1]
metricsCounterStore.getAllMatchingEntries('pulls/o/bar.in/d/2024-05-31') == ['pulls/o/bar.in/d/2024-05-31':1]
}
}
Loading

0 comments on commit b24894a

Please sign in to comment.