Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* The GP2GP Adaptor will no longer incorrectly fail schema validation.
* The GP2GP Adaptor now sources the references correctly from within the service code rather than fetching them from the test folder.

### Changed
* Updated logging configuration to route application logs through async appender settings for improved runtime logging performance.

## [3.0.0] - 2025-11-06

### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ private static ExchangeFilterFunction errorHandling(RequestType requestType, Htt

var statusCode = clientResponse.statusCode();
if (statusCode.equals(httpStatus)) {
LOGGER.info(requestType + " request successful status_code: {}", statusCode.value());
LOGGER.info("{} request successful status_code: {}", requestType, statusCode.value());
return Mono.just(clientResponse);
}
if (statusCode.is5xxServerError()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void execute(SendDocumentTaskDefinition taskDefinition) {
}

var binary = outboundMessage.getAttachments().get(0).getPayload();
LOGGER.debug("Attachment size=" + getBytesLengthOfString(binary) + " content-type=" + taskDefinition.getDocumentContentType());
LOGGER.debug("Attachment size={} content-type={}", getBytesLengthOfString(binary), taskDefinition.getDocumentContentType());
if (isLargeAttachment(binary)) {
outboundMessage.getAttachments().clear(); // since it's a large message, chunks will be sent as external attachments
outboundMessage.setExternalAttachments(new ArrayList<>());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void testForValidReferences(Condition condition) {
try {
messageContext.getInputBundleHolder().getResource(idType);
} catch (Exception e) {
LOGGER.warn("Bundle mapping aborted. Reason: " + e.getMessage());
LOGGER.warn("Bundle mapping aborted. Reason: {}", e.getMessage());
throw e;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void handleContinue(String conversationId, String payload) {
ehrExtractStatusService.updateEhrExtractStatusContinue(conversationId)
.ifPresent(ehrExtractStatus -> {
var documents = ehrExtractStatus.getGpcAccessDocument().getDocuments();
LOGGER.info("Sending documents for: ConversationId: " + conversationId);
LOGGER.info("Sending documents for: ConversationId: {}", conversationId);
for (int documentPosition = 0; documentPosition < documents.size(); documentPosition++) {
var document = documents.get(documentPosition);
createSendDocumentTasks(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private List<InputFile> getFiles() throws UnreadableJsonFileException, NoJsonFil
throw new NoJsonFileFound("No json files found");
}

LOGGER.info("Processing " + files.length + " files from location: " + JSON_FILE_INPUT_PATH);
LOGGER.info("Processing {} files from location: {}", files.length, JSON_FILE_INPUT_PATH);

Arrays.stream(files)
.peek(file -> LOGGER.info("Parsing file: {}", file.getName()))
Expand Down
14 changes: 8 additions & 6 deletions service/src/main/resources/logback.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@
</encoder>
</appender>

<appender name="ASYNC" class="ch.qos.logback.classic.AsyncAppender">
<appender-ref ref="TEXT"/>
<queueSize>${GP2GP_LOGGING_ASYNC_QUEUE_SIZE:-2048}</queueSize>
<discardingThreshold>0</discardingThreshold>
<neverBlock>true</neverBlock>
<includeCallerData>false</includeCallerData>
</appender>

<root level="${GP2GP_ROOT_LOGGING_LEVEL:-WARN}">
<appender-ref ref="TEXT}"/>
<appender-ref ref="ASYNC"/>
</root>

<logger name="uk.nhs.adaptors.gp2gp" level="${GP2GP_LOGGING_LEVEL:-INFO}" />

<!-- <logger name="uk.nhs.adaptors.gp2gp.common.service.WebClientFilterService" level="DEBUG" />-->
<!-- <logger name="uk.nhs.adaptors.gp2gp.mhs.MhsRequestBuilder" level="DEBUG" />-->
<!-- <logger name="uk.nhs.adaptors.gp2gp.mhs.InboundMessageHandler" level="DEBUG" />-->

<logger name="reactor.netty.http.client" level="${GP2GP_LOGGING_LEVEL:-WARN}" />
</configuration>
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package uk.nhs.adaptors.gp2gp.common.configuration;

import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.Appender;
import ch.qos.logback.core.read.ListAppender;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;

import static org.assertj.core.api.Assertions.assertThat;

class LogbackConfigurationTest {

public static final int MILLIS_100 = 100;
private Logger rootLogger;
private Logger gp2gpLogger;
private Logger reactorNettyLogger;

@BeforeEach
void setUp() {
rootLogger = (Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
gp2gpLogger = (Logger) LoggerFactory.getLogger("uk.nhs.adaptors.gp2gp");
reactorNettyLogger = (Logger) LoggerFactory.getLogger("reactor.netty.http.client");
}

@Test
void shouldHaveAsyncAppenderOnRootLogger() {

assertThat(rootLogger.getAppender("ASYNC"))
.as("AsyncAppender 'ASYNC' should be attached to root logger")
.isNotNull();

Appender<?> appender = rootLogger.getAppender("ASYNC");
assertThat(appender.getClass().getSimpleName())
.contains("AsyncAppender");
}

@Test
void shouldHaveGp2gpLoggerConfiguredWithProperLevel() {
assertThat(gp2gpLogger.getEffectiveLevel())
.as("GP2GP logger should inherit or override level explicitly")
.isNotNull();
}

@Test
void shouldDecoupleReactorNettyLoggingLevelFromGp2GpLevel() {
Level reactorLevel = reactorNettyLogger.getEffectiveLevel();

assertThat(reactorLevel)
.as("Reactor Netty logger should have explicit level to decouple from GP2GP logger")
.isNotNull();
}

@Test
void shouldPreserveMdcInAsyncLogging() {

ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
listAppender.start();
gp2gpLogger.addAppender(listAppender);
gp2gpLogger.setLevel(Level.INFO);

MDC.put("ConversationId", "test-conv-123");
MDC.put("TaskId", "test-task-456");

gp2gpLogger.info("Test message with MDC");

try {
Thread.sleep(MILLIS_100);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}

assertThat(listAppender.list).isNotEmpty();

ILoggingEvent event = listAppender.list.getFirst();
assertThat(event.getMDCPropertyMap())
.containsEntry("ConversationId", "test-conv-123")
.containsEntry("TaskId", "test-task-456");

MDC.clear();
gp2gpLogger.detachAppender(listAppender);
listAppender.stop();
}

@Test
void shouldNotBlockOnLoggingIo() {

Appender<?> asyncAppender = rootLogger.getAppender("ASYNC");

assertThat(asyncAppender)
.as("AsyncAppender should be configured to never block producer threads")
.isNotNull();

assertThat(asyncAppender.getClass().getSimpleName())
.isEqualTo("AsyncAppender");
}

@Test
void rootLoggerShouldHaveValidTextAppenderReference() {

Appender<?> appender = rootLogger.getAppender("ASYNC");

assertThat(appender)
.as("Root logger should have valid ASYNC appender (wraps corrected TEXT appender)")
.isNotNull();

assertThat(appender.getName())
.as("Appender name should be ASYNC, not a default fallback")
.isEqualTo("ASYNC");
}
}





Loading
Loading