From 396c10c2511222447785f6b8cd41d54ef34d022e Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Sun, 19 Jan 2025 20:03:45 +0100 Subject: [PATCH 1/6] Add TraceContextFilter logging context propagation Signed-off-by: Paolo Di Tommaso --- .../wave/filter/TraceContextFilter.groovy | 63 +++++++++++++++++++ src/main/resources/logback.xml | 2 +- .../wave/filter/TraceContextFilterTest.groovy | 39 ++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 src/main/groovy/io/seqera/wave/filter/TraceContextFilter.groovy create mode 100644 src/test/groovy/io/seqera/wave/filter/TraceContextFilterTest.groovy diff --git a/src/main/groovy/io/seqera/wave/filter/TraceContextFilter.groovy b/src/main/groovy/io/seqera/wave/filter/TraceContextFilter.groovy new file mode 100644 index 000000000..cc0952856 --- /dev/null +++ b/src/main/groovy/io/seqera/wave/filter/TraceContextFilter.groovy @@ -0,0 +1,63 @@ +/* + * Wave, containers provisioning service + * Copyright (c) 2023-2024, Seqera Labs + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package io.seqera.wave.filter + +import java.util.regex.Pattern + +import groovy.transform.CompileStatic +import io.micronaut.context.propagation.slf4j.MdcPropagationContext +import io.micronaut.core.propagation.MutablePropagatedContext +import io.micronaut.http.HttpRequest +import io.micronaut.http.annotation.RequestFilter +import io.micronaut.http.annotation.ServerFilter +import org.slf4j.MDC +import static io.micronaut.http.annotation.ServerFilter.MATCH_ALL_PATTERN + +/** + * HTTP filter to trace and propagate request metadata in the MDC logging context + * + * @author Paolo Di Tommaso + */ +@CompileStatic +@ServerFilter(MATCH_ALL_PATTERN) +class TraceContextFilter { + + static final Pattern REGEX = ~/^\/v2\/wt\/([a-z0-9]+).*/ + + @RequestFilter + void requestFilter(HttpRequest request, MutablePropagatedContext mutablePropagatedContext) { + try { + final requestId = getRequestId(request.path) + MDC.put("requestId", requestId) + MDC.put("requestPath", request.path) + MDC.put("requestMethod", request.methodName) + mutablePropagatedContext.add(new MdcPropagationContext()) + } finally { + MDC.remove("requestId") + MDC.remove("requestPath") + MDC.remove("requestMethod") + } + } + + static String getRequestId(String path) { + final m = REGEX.matcher(path) + return m.matches() ? m.group(1) : null + } + +} diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml index 695ffc591..785ed30b4 100644 --- a/src/main/resources/logback.xml +++ b/src/main/resources/logback.xml @@ -24,7 +24,7 @@ false - %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n + %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} \(rid=%X{requestId}\) - %msg%n diff --git a/src/test/groovy/io/seqera/wave/filter/TraceContextFilterTest.groovy b/src/test/groovy/io/seqera/wave/filter/TraceContextFilterTest.groovy new file mode 100644 index 000000000..e9fe61cd8 --- /dev/null +++ b/src/test/groovy/io/seqera/wave/filter/TraceContextFilterTest.groovy @@ -0,0 +1,39 @@ +/* + * Wave, containers provisioning service + * Copyright (c) 2023-2024, Seqera Labs + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package io.seqera.wave.filter + +import spock.lang.Specification + +/** + * + * @author Paolo Di Tommaso + */ +class TraceContextFilterTest extends Specification { + + def 'should validate request id regex' () { + expect: + TraceContextFilter.getRequestId(PATH) == EXPECTED + where: + PATH | EXPECTED + '/foo/bar' | null + '/v2/wt/1234' | '1234' + '/v2/wt/12ab/x/y/z' | '12ab' + } + +} From a95153bedfa79145f85c240d6b7b7a5903bba1da Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Sun, 19 Jan 2025 20:05:05 +0100 Subject: [PATCH 2/6] Improve request caching logic Signed-off-by: Paolo Di Tommaso --- .../wave/core/RegistryProxyService.groovy | 32 +++++++++++++------ .../wave/core/RegistryProxyServiceTest.groovy | 5 +++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy b/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy index 26ccda8f6..80a7e4d3f 100644 --- a/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy +++ b/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy @@ -165,6 +165,7 @@ class RegistryProxyService { 'Accept-Encoding', 'Authorization', 'Cache-Control', + 'Connection', 'Content-Type', 'Content-Length', 'Content-Range', @@ -173,9 +174,12 @@ class RegistryProxyService { 'If-None-Match', 'If-None-Match', 'Etag', + 'Host', 'Location', 'Last-Modified', + 'User-Agent', 'Range', + 'X-Registry-Auth' ] static protected boolean isCacheableHeader(String key) { @@ -196,8 +200,13 @@ class RegistryProxyService { if( !headers ) headers = Map.of() for( Map.Entry> entry : headers ) { - if( !isCacheableHeader(entry.key) ) + if( "Cache-Control".equalsIgnoreCase(entry.key) ) { + // ignore caching when cache-control header is provided + return null + } + if( !isCacheableHeader(entry.key) ) { continue + } hasher.putUnencodedChars(entry.key) for( String it : entry.value ) { if( it ) @@ -218,14 +227,19 @@ class RegistryProxyService { } DelegateResponse handleRequest(RoutePath route, Map> headers) { - return cache.getOrCompute( - requestKey(route, headers), - (String k)-> { - final resp = handleRequest0(route, headers) - // when the response is not cacheable, return null as TTL - final ttl = route.isDigest() && resp.isCacheable() ? cache.duration : null - return new Tuple2(resp, ttl) - }) + final key = requestKey(route, headers) + if( !key ) { + log.debug "Bypass cache for requrst route=${route}; headers=${headers}" + return handleRequest0(route, headers) + } + else { + return cache.getOrCompute(key,(String k)-> { + final resp = handleRequest0(route, headers) + // when the response is not cacheable, return null as TTL + final ttl = route.isDigest() && resp.isCacheable() ? cache.duration : null + return new Tuple2(resp, ttl) + }) + } } @TraceElapsedTime(thresholdMillis = '${wave.trace.proxy-service.threshold:1000}') diff --git a/src/test/groovy/io/seqera/wave/core/RegistryProxyServiceTest.groovy b/src/test/groovy/io/seqera/wave/core/RegistryProxyServiceTest.groovy index 161fc8185..bb4f8b5e9 100644 --- a/src/test/groovy/io/seqera/wave/core/RegistryProxyServiceTest.groovy +++ b/src/test/groovy/io/seqera/wave/core/RegistryProxyServiceTest.groovy @@ -97,6 +97,11 @@ class RegistryProxyServiceTest extends Specification { def k5 = RegistryProxyService.requestKey(p1, ['Content-Type': ['text/1', 'text/2'], 'X-Foo': ['bar']]) then: k5 == 'f9fe81aed4d72cba' // <-- the header 'X-Foo' does not alter the cache key + + when: + def k6 = RegistryProxyService.requestKey(p1, ["cache-control":["none"]]) + then: + k6 == null } def 'check is cacheable header' () { From 0ab871648e6a4cb426e6e81ceb66e99c084a155d Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Mon, 20 Jan 2025 11:16:21 +0100 Subject: [PATCH 3/6] Improve logging pattern Signed-off-by: Paolo Di Tommaso --- src/main/resources/logback.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml index 785ed30b4..7bebd1257 100644 --- a/src/main/resources/logback.xml +++ b/src/main/resources/logback.xml @@ -24,7 +24,7 @@ false - %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} \(rid=%X{requestId}\) - %msg%n + %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg >> wt=%X{requestId}%n @@ -43,7 +43,7 @@ 200MB - %d{MMM-dd HH:mm:ss.SSS} [%t] %-5level %logger{36} - %msg%n + %d{MMM-dd HH:mm:ss.SSS} [%t] %-5level %logger{36} - %msg%n >> wt=%X{requestId}%n From 163e605f1927cec970ec7efcdf61058740f12661 Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Mon, 20 Jan 2025 11:37:23 +0100 Subject: [PATCH 4/6] Improve Proxy cache configuration Signed-off-by: Paolo Di Tommaso --- .../configuration/ProxyCacheConfig.groovy | 58 +++++++++++++++++++ .../wave/core/RegistryProxyService.groovy | 3 + .../io/seqera/wave/proxy/ProxyCache.groovy | 22 ++++--- .../seqera/wave/proxy/ProxyCacheTest.groovy | 3 +- 4 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 src/main/groovy/io/seqera/wave/configuration/ProxyCacheConfig.groovy diff --git a/src/main/groovy/io/seqera/wave/configuration/ProxyCacheConfig.groovy b/src/main/groovy/io/seqera/wave/configuration/ProxyCacheConfig.groovy new file mode 100644 index 000000000..0d5021f8b --- /dev/null +++ b/src/main/groovy/io/seqera/wave/configuration/ProxyCacheConfig.groovy @@ -0,0 +1,58 @@ +/* + * Wave, containers provisioning service + * Copyright (c) 2023-2024, Seqera Labs + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package io.seqera.wave.configuration + +import java.time.Duration + +import groovy.transform.CompileStatic +import groovy.transform.ToString +import io.micronaut.context.annotation.Value +import jakarta.inject.Singleton + +/** + * Model {@link io.seqera.wave.proxy.ProxyCache} configuration settings + * + * @author Paolo Di Tommaso + */ +@Singleton +@CompileStatic +@ToString(includeNames = true, includePackage = false) +class ProxyCacheConfig { + + @Value('${wave.proxy-cache.duration:4m}') + private Duration duration + + @Value('${wave.proxy-cache.max-size:10000}') + private int maxSize + + @Value('${wave.proxy-cache.enabled:true}') + private boolean enabled + + Duration getDuration() { + return duration + } + + int getMaxSize() { + return maxSize + } + + boolean getEnabled() { + return enabled + } +} diff --git a/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy b/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy index 80a7e4d3f..5be392f85 100644 --- a/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy +++ b/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy @@ -227,6 +227,9 @@ class RegistryProxyService { } DelegateResponse handleRequest(RoutePath route, Map> headers) { + if( !cache.enabled ) { + return handleRequest0(route, headers) + } final key = requestKey(route, headers) if( !key ) { log.debug "Bypass cache for requrst route=${route}; headers=${headers}" diff --git a/src/main/groovy/io/seqera/wave/proxy/ProxyCache.groovy b/src/main/groovy/io/seqera/wave/proxy/ProxyCache.groovy index a65c6258c..9a8fe84fd 100644 --- a/src/main/groovy/io/seqera/wave/proxy/ProxyCache.groovy +++ b/src/main/groovy/io/seqera/wave/proxy/ProxyCache.groovy @@ -22,8 +22,9 @@ import java.time.Duration import com.squareup.moshi.adapters.PolymorphicJsonAdapterFactory import groovy.transform.CompileStatic -import io.micronaut.context.annotation.Value +import groovy.util.logging.Slf4j import io.micronaut.core.annotation.Nullable +import io.seqera.wave.configuration.ProxyCacheConfig import io.seqera.wave.encoder.MoshiEncodeStrategy import io.seqera.wave.encoder.MoshiExchange import io.seqera.wave.store.cache.AbstractTieredCache @@ -34,18 +35,17 @@ import jakarta.inject.Singleton * * @author Paolo Di Tommaso */ +@Slf4j @Singleton @CompileStatic class ProxyCache extends AbstractTieredCache { - @Value('${wave.proxy-cache.duration:30m}') - private Duration duration + private ProxyCacheConfig config - @Value('${wave.proxy-cache.max-size:10000}') - private int maxSize - - ProxyCache(@Nullable L2TieredCache l2) { + ProxyCache(@Nullable L2TieredCache l2, ProxyCacheConfig config) { super(l2, encoder()) + this.config = config + log.info "+ Creating Proxy-cache - config=${config}" } static MoshiEncodeStrategy encoder() { @@ -69,11 +69,15 @@ class ProxyCache extends AbstractTieredCache { @Override int getMaxSize() { - return maxSize + return config.maxSize } Duration getDuration() { - return duration + return config.duration + } + + boolean getEnabled() { + return config.enabled } } diff --git a/src/test/groovy/io/seqera/wave/proxy/ProxyCacheTest.groovy b/src/test/groovy/io/seqera/wave/proxy/ProxyCacheTest.groovy index 5c32fd95b..b1ab018f9 100644 --- a/src/test/groovy/io/seqera/wave/proxy/ProxyCacheTest.groovy +++ b/src/test/groovy/io/seqera/wave/proxy/ProxyCacheTest.groovy @@ -24,6 +24,7 @@ import spock.lang.Specification import java.time.Duration import io.micronaut.context.ApplicationContext +import io.seqera.wave.configuration.ProxyCacheConfig import io.seqera.wave.store.cache.RedisL2TieredCache import io.seqera.wave.test.RedisTestContainer /** @@ -51,7 +52,7 @@ class ProxyCacheTest extends Specification implements RedisTestContainer { given: def TTL = Duration.ofMillis(150) def store = applicationContext.getBean(RedisL2TieredCache) - def cache = new ProxyCache(store) + def cache = new ProxyCache(store, Mock(ProxyCacheConfig)) and: def k = UUID.randomUUID().toString() def resp = new DelegateResponse( From 5fe4b4cda3e0930b927daf990378afedfa40933f Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Mon, 20 Jan 2025 11:38:19 +0100 Subject: [PATCH 5/6] [release] bump version 1.16.8 Signed-off-by: Paolo Di Tommaso --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 293bfa8f0..75b5dfa4d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.16.7 +1.16.8 From cc79afea67772608232aed2a01e24888028d9620 Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Mon, 20 Jan 2025 17:07:21 +0100 Subject: [PATCH 6/6] Update changelog [ci skip] Signed-off-by: Paolo Di Tommaso --- changelog.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/changelog.txt b/changelog.txt index 59e399f21..e69aa1279 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,4 +1,11 @@ # Wave changelog +1.16.8 - 20 Jab 2025 +- Add TraceContextFilter logging context propagation [396c10c2] +- Improve Proxy cache configuration [163e605f] +- Improve logging pattern [0ab87164] +- Improve request caching logic [a95153be] +- Bump MN 4.7.4 [4ce2a139] + 1.16.7 - 10 Jan 2025 - Improve logging on pairing websocket error [21861dbe]