From ad420107852bafa141dcd68120be6d6a24153bf8 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 7 Dec 2020 20:02:44 +0000 Subject: [PATCH] Correlate data buffers to request log messages HttpMessageWriter implementations now attach the request log prefix as a hint to created data buffers when the logger associated with the writer is at DEBUG level. Closes gh-26230 --- .../org/springframework/core/codec/Hints.java | 20 +++++++++++++++++++ .../core/codec/ResourceRegionEncoder.java | 5 ++++- .../core/io/buffer/DataBufferUtils.java | 18 +++++++++++++++++ .../core/io/buffer/NettyDataBuffer.java | 6 ++++++ .../core/io/buffer/PooledDataBuffer.java | 9 ++++++++- .../io/buffer/LeakAwareDataBuffer.java | 19 +++++++++--------- .../http/codec/EncoderHttpMessageWriter.java | 16 +++++++++++++-- .../http/codec/ResourceHttpMessageWriter.java | 5 ++++- .../ServerSentEventHttpMessageWriter.java | 8 +++++++- .../multipart/MultipartHttpMessageWriter.java | 4 ++++ .../multipart/PartHttpMessageWriter.java | 4 ++++ 11 files changed, 99 insertions(+), 15 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/codec/Hints.java b/spring-core/src/main/java/org/springframework/core/codec/Hints.java index 7f167fd509..a731b018bd 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/Hints.java +++ b/spring-core/src/main/java/org/springframework/core/codec/Hints.java @@ -21,6 +21,8 @@ import java.util.Map; import org.apache.commons.logging.Log; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; @@ -148,4 +150,22 @@ public abstract class Hints { } } + /** + * If the hints contain a {@link #LOG_PREFIX_HINT} and the given logger has + * DEBUG level enabled, apply the log prefix as a hint to the given buffer + * via {@link DataBufferUtils#touch(DataBuffer, Object)}. + * @param buffer the buffer to touch + * @param hints the hints map to check for a log prefix + * @param logger the logger whose level to check + * @since 5.3.2 + */ + public static void touchDataBuffer(DataBuffer buffer, @Nullable Map hints, Log logger) { + if (logger.isDebugEnabled() && hints != null) { + Object logPrefix = hints.get(LOG_PREFIX_HINT); + if (logPrefix != null) { + DataBufferUtils.touch(buffer, logPrefix); + } + } + } + } diff --git a/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java index 3e9d19c20d..3330ac6e15 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/ResourceRegionEncoder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -130,6 +130,9 @@ public class ResourceRegionEncoder extends AbstractEncoder { } Flux in = DataBufferUtils.read(resource, position, bufferFactory, this.bufferSize); + if (logger.isDebugEnabled()) { + in = in.doOnNext(buffer -> Hints.touchDataBuffer(buffer, hints, logger)); + } return DataBufferUtils.takeUntilByteCount(in, count); } diff --git a/spring-core/src/main/java/org/springframework/core/io/buffer/DataBufferUtils.java b/spring-core/src/main/java/org/springframework/core/io/buffer/DataBufferUtils.java index 81d0b01844..41a3760fbe 100644 --- a/spring-core/src/main/java/org/springframework/core/io/buffer/DataBufferUtils.java +++ b/spring-core/src/main/java/org/springframework/core/io/buffer/DataBufferUtils.java @@ -487,6 +487,24 @@ public abstract class DataBufferUtils { } } + /** + * Associate the given hint with the data buffer if it is a pooled buffer + * and supports leak tracking. + * @param dataBuffer the data buffer to attach the hint to + * @param hint the hint to attach + * @return the input buffer + * @since 5.3.2 + */ + @SuppressWarnings("unchecked") + public static T touch(T dataBuffer, Object hint) { + if (dataBuffer instanceof PooledDataBuffer) { + return (T) ((PooledDataBuffer) dataBuffer).touch(hint); + } + else { + return dataBuffer; + } + } + /** * Release the given data buffer, if it is a {@link PooledDataBuffer} and * has been {@linkplain PooledDataBuffer#isAllocated() allocated}. diff --git a/spring-core/src/main/java/org/springframework/core/io/buffer/NettyDataBuffer.java b/spring-core/src/main/java/org/springframework/core/io/buffer/NettyDataBuffer.java index 7b37f8bd51..7809c65295 100644 --- a/spring-core/src/main/java/org/springframework/core/io/buffer/NettyDataBuffer.java +++ b/spring-core/src/main/java/org/springframework/core/io/buffer/NettyDataBuffer.java @@ -315,6 +315,12 @@ public class NettyDataBuffer implements PooledDataBuffer { return new NettyDataBuffer(this.byteBuf.retain(), this.dataBufferFactory); } + @Override + public PooledDataBuffer touch(Object hint) { + this.byteBuf.touch(hint); + return this; + } + @Override public boolean release() { return this.byteBuf.release(); diff --git a/spring-core/src/main/java/org/springframework/core/io/buffer/PooledDataBuffer.java b/spring-core/src/main/java/org/springframework/core/io/buffer/PooledDataBuffer.java index d97e299b0f..e3e794214e 100644 --- a/spring-core/src/main/java/org/springframework/core/io/buffer/PooledDataBuffer.java +++ b/spring-core/src/main/java/org/springframework/core/io/buffer/PooledDataBuffer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -38,6 +38,13 @@ public interface PooledDataBuffer extends DataBuffer { */ PooledDataBuffer retain(); + /** + * Associate the given hint with the data buffer for debugging purposes. + * @return this buffer + * @since 5.3.2 + */ + PooledDataBuffer touch(Object hint); + /** * Decrease the reference count for this buffer by one, * and deallocate it once the count reaches zero. diff --git a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/io/buffer/LeakAwareDataBuffer.java b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/io/buffer/LeakAwareDataBuffer.java index f03036f131..32fe8c5e0a 100644 --- a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/io/buffer/LeakAwareDataBuffer.java +++ b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/io/buffer/LeakAwareDataBuffer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package org.springframework.core.testfixture.io.buffer; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DataBufferWrapper; import org.springframework.core.io.buffer.PooledDataBuffer; import org.springframework.util.Assert; @@ -67,19 +68,19 @@ class LeakAwareDataBuffer extends DataBufferWrapper implements PooledDataBuffer @Override public PooledDataBuffer retain() { - DataBuffer delegate = dataBuffer(); - if (delegate instanceof PooledDataBuffer) { - ((PooledDataBuffer) delegate).retain(); - } + DataBufferUtils.retain(dataBuffer()); + return this; + } + + @Override + public PooledDataBuffer touch(Object hint) { + DataBufferUtils.touch(dataBuffer(), hint); return this; } @Override public boolean release() { - DataBuffer delegate = dataBuffer(); - if (delegate instanceof PooledDataBuffer) { - ((PooledDataBuffer) delegate).release(); - } + DataBufferUtils.release(dataBuffer()); return isAllocated(); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java index 5a63145b4b..c429767fbf 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/EncoderHttpMessageWriter.java @@ -57,6 +57,9 @@ import org.springframework.util.StringUtils; */ public class EncoderHttpMessageWriter implements HttpMessageWriter { + private static final Log logger = HttpLogging.forLogName(EncoderHttpMessageWriter.class); + + private final Encoder encoder; private final List mediaTypes; @@ -125,6 +128,7 @@ public class EncoderHttpMessageWriter implements HttpMessageWriter { return message.setComplete().then(Mono.empty()); })) .flatMap(buffer -> { + Hints.touchDataBuffer(buffer, hints, logger); message.getHeaders().setContentLength(buffer.readableByteCount()); return message.writeWith(Mono.just(buffer) .doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release)); @@ -132,10 +136,15 @@ public class EncoderHttpMessageWriter implements HttpMessageWriter { } if (isStreamingMediaType(contentType)) { - return message.writeAndFlushWith(body.map(buffer -> - Mono.just(buffer).doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release))); + return message.writeAndFlushWith(body.map(buffer -> { + Hints.touchDataBuffer(buffer, hints, logger); + return Mono.just(buffer).doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release); + })); } + if (logger.isDebugEnabled()) { + body = body.doOnNext(buffer -> Hints.touchDataBuffer(buffer, hints, logger)); + } return message.writeWith(body); } @@ -166,6 +175,9 @@ public class EncoderHttpMessageWriter implements HttpMessageWriter { return main; } + private static void touch(DataBuffer buffer, Map hints) { + } + private boolean isStreamingMediaType(@Nullable MediaType mediaType) { if (mediaType == null || !(this.encoder instanceof HttpMessageEncoder)) { return false; diff --git a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java index 4376d39d06..af456989e0 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -132,6 +132,9 @@ public class ResourceHttpMessageWriter implements HttpMessageWriter { Mono input = Mono.just(resource); DataBufferFactory factory = message.bufferFactory(); Flux body = this.encoder.encode(input, factory, type, resourceMediaType, hints); + if (logger.isDebugEnabled()) { + body = body.doOnNext(buffer -> Hints.touchDataBuffer(buffer, hints, logger)); + } return message.writeWith(body); }); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java index 90e4f74ba9..6f54d9631b 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/ServerSentEventHttpMessageWriter.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import org.apache.commons.logging.Log; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -35,6 +36,7 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.PooledDataBuffer; +import org.springframework.http.HttpLogging; import org.springframework.http.MediaType; import org.springframework.http.ReactiveHttpOutputMessage; import org.springframework.http.server.reactive.ServerHttpRequest; @@ -57,6 +59,8 @@ public class ServerSentEventHttpMessageWriter implements HttpMessageWriter WRITABLE_MEDIA_TYPES = Collections.singletonList(MediaType.TEXT_EVENT_STREAM); + private static final Log logger = HttpLogging.forLogName(ServerSentEventHttpMessageWriter.class); + @Nullable private final Encoder encoder; @@ -167,9 +171,11 @@ public class ServerSentEventHttpMessageWriter implements HttpMessageWriter) this.encoder).encodeValue(data, factory, dataType, mediaType, hints); + Hints.touchDataBuffer(buffer, hints, logger); return Flux.just(factory.join(Arrays.asList( encodeText(eventContent, mediaType, factory), - ((Encoder) this.encoder).encodeValue(data, factory, dataType, mediaType, hints), + buffer, encodeText("\n\n", mediaType, factory)))); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java index d94104535d..f2f490e1d8 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java @@ -199,6 +199,10 @@ public class MultipartHttpMessageWriter extends MultipartWriterSupport .concatWith(generateLastLine(boundary, bufferFactory)) .doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release); + if (logger.isDebugEnabled()) { + body = body.doOnNext(buffer -> Hints.touchDataBuffer(buffer, hints, logger)); + } + return outputMessage.writeWith(body); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java index aa7819b2d8..31817470ca 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/PartHttpMessageWriter.java @@ -69,6 +69,10 @@ public class PartHttpMessageWriter extends MultipartWriterSupport implements Htt .concatWith(generateLastLine(boundary, outputMessage.bufferFactory())) .doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release); + if (logger.isDebugEnabled()) { + body = body.doOnNext(buffer -> Hints.touchDataBuffer(buffer, hints, logger)); + } + return outputMessage.writeWith(body); }