From 1ba5013346ded2a5941d9af9e6bb4ea74cb9647f Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 2 Feb 2023 16:24:57 -0700 Subject: [PATCH 1/3] Fix TR logging when cqhv field is absent --- CHANGELOG.md | 1 + .../core/http/HTTPAccessEventBuilder.java | 14 +++++++++++++- .../core/http/HTTPAccessEventBuilderTest.java | 13 ++++++++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ce1e4d024..5676b81fac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7120](https://github.com/apache/trafficcontrol/pull/7120) *Docs* Update t3c documentation regarding parent.config parent_retry. ### Fixed +- [](https://github.com/apache/trafficcontrol/pull/) *Traffic Router* Fixed TR logging for the `cqhv` field when absent. - [#7252](https://github.com/apache/trafficcontrol/issues/7252) *Traffic Router* Fixed integer overflow for `czCount`, by resetting the count to max value when it overflows. - [#7221](https://github.com/apache/trafficcontrol/issues/7221) *Docs* Fixed request structure spec in cdn locks description in APIv4. - [#7225](https://github.com/apache/trafficcontrol/issues/7225) *Docs* Fixed docs for /roles response description in APIv4. diff --git a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilder.java b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilder.java index cb2f81fad0..d574897fa2 100644 --- a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilder.java +++ b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilder.java @@ -74,6 +74,14 @@ private static String formatRequestHeaders(final Map requestHead return stringBuilder.toString(); } + private static boolean validProtocol(final String protocol) { + if (protocol != null && + !protocol.equals("")) { + return true; + } + return false; + } + @SuppressWarnings({"PMD.UseStringBufferForStringAppends", "PMD.NPathComplexity"}) public static String create(final HTTPAccessRecord httpAccessRecord) { final long start = httpAccessRecord.getRequestDate().getTime(); @@ -84,7 +92,11 @@ public static String create(final HTTPAccessRecord httpAccessRecord) { String chi = formatObject(httpServletRequest.getRemoteAddr()); final String url = formatRequest(httpServletRequest); final String cqhm = formatObject(httpServletRequest.getMethod()); - final String cqhv = formatObject(httpServletRequest.getProtocol()); + String cqhv = "-"; + final String protocol = formatObject(httpServletRequest.getProtocol()); + if (validProtocol(protocol)) { + cqhv = protocol; + } final String resultType = formatObject(httpAccessRecord.getResultType()); final String rerr = formatObject(httpAccessRecord.getRerr()); diff --git a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilderTest.java b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilderTest.java index d999eeaff1..adc0ff957c 100644 --- a/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilderTest.java +++ b/traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilderTest.java @@ -79,10 +79,21 @@ public void itGeneratesAccessEvents() throws Exception { HTTPAccessRecord httpAccessRecord = builder.build(); String httpAccessEvent = HTTPAccessEventBuilder.create(httpAccessRecord); - assertThat(httpAccessEvent, equalTo("144140678.000 qtype=HTTP chi=192.168.7.6 rhi=- url=\"http://example.com/index.html?foo=bar\" cqhm=GET cqhv=HTTP/1.1 rtype=- rloc=\"-\" rdtl=- rerr=\"-\" rgb=\"-\" rurl=\"-\" rurls=\"-\" uas=\"null\" svc=\"-\" rh=\"-\"")); } + @Test + public void itGeneratesAccessEventsWithCorrectCqhvDefaultValues() throws Exception { + HTTPAccessRecord.Builder builder = new HTTPAccessRecord.Builder(new Date(144140678000L), request); + HTTPAccessRecord httpAccessRecord = builder.build(); + when(request.getProtocol()).thenReturn(null); + String httpAccessEvent = HTTPAccessEventBuilder.create(httpAccessRecord); + assertThat(httpAccessEvent, equalTo("144140678.000 qtype=HTTP chi=192.168.7.6 rhi=- url=\"http://example.com/index.html?foo=bar\" cqhm=GET cqhv=- rtype=- rloc=\"-\" rdtl=- rerr=\"-\" rgb=\"-\" rurl=\"-\" rurls=\"-\" uas=\"null\" svc=\"-\" rh=\"-\"")); + when(request.getProtocol()).thenReturn(""); + httpAccessEvent = HTTPAccessEventBuilder.create(httpAccessRecord); + assertThat(httpAccessEvent, equalTo("144140678.000 qtype=HTTP chi=192.168.7.6 rhi=- url=\"http://example.com/index.html?foo=bar\" cqhm=GET cqhv=- rtype=- rloc=\"-\" rdtl=- rerr=\"-\" rgb=\"-\" rurl=\"-\" rurls=\"-\" uas=\"null\" svc=\"-\" rh=\"-\"")); + } + @Test public void itAddsResponseData() throws Exception { Answer nanoTimeAnswer = new Answer() { From 47dbfc2a723eb2a139c8c73fbb130da2db24667c Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 2 Feb 2023 16:27:48 -0700 Subject: [PATCH 2/3] fix changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5676b81fac..5a3f9c6379 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7120](https://github.com/apache/trafficcontrol/pull/7120) *Docs* Update t3c documentation regarding parent.config parent_retry. ### Fixed -- [](https://github.com/apache/trafficcontrol/pull/) *Traffic Router* Fixed TR logging for the `cqhv` field when absent. +- [#7340](https://github.com/apache/trafficcontrol/pull/7340) *Traffic Router* Fixed TR logging for the `cqhv` field when absent. - [#7252](https://github.com/apache/trafficcontrol/issues/7252) *Traffic Router* Fixed integer overflow for `czCount`, by resetting the count to max value when it overflows. - [#7221](https://github.com/apache/trafficcontrol/issues/7221) *Docs* Fixed request structure spec in cdn locks description in APIv4. - [#7225](https://github.com/apache/trafficcontrol/issues/7225) *Docs* Fixed docs for /roles response description in APIv4. From 680931c5b7ba72b1586e4b2455f91163c9830d81 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Thu, 2 Feb 2023 22:39:04 -0700 Subject: [PATCH 3/3] code review --- .../core/http/HTTPAccessEventBuilder.java | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilder.java b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilder.java index d574897fa2..c3f7e7963a 100644 --- a/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilder.java +++ b/traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/http/HTTPAccessEventBuilder.java @@ -43,7 +43,7 @@ private static String formatRequest(final HttpServletRequest request) { } private static String formatObject(final Object o) { - return (o == null) ? "-" : o.toString(); + return (o == null || o.toString().equals("")) ? "-" : o.toString(); } private static String formatRequestHeaders(final Map requestHeaders) { @@ -74,14 +74,6 @@ private static String formatRequestHeaders(final Map requestHead return stringBuilder.toString(); } - private static boolean validProtocol(final String protocol) { - if (protocol != null && - !protocol.equals("")) { - return true; - } - return false; - } - @SuppressWarnings({"PMD.UseStringBufferForStringAppends", "PMD.NPathComplexity"}) public static String create(final HTTPAccessRecord httpAccessRecord) { final long start = httpAccessRecord.getRequestDate().getTime(); @@ -92,11 +84,7 @@ public static String create(final HTTPAccessRecord httpAccessRecord) { String chi = formatObject(httpServletRequest.getRemoteAddr()); final String url = formatRequest(httpServletRequest); final String cqhm = formatObject(httpServletRequest.getMethod()); - String cqhv = "-"; - final String protocol = formatObject(httpServletRequest.getProtocol()); - if (validProtocol(protocol)) { - cqhv = protocol; - } + final String cqhv = formatObject(httpServletRequest.getProtocol()); final String resultType = formatObject(httpAccessRecord.getResultType()); final String rerr = formatObject(httpAccessRecord.getRerr());