armeria icon indicating copy to clipboard operation
armeria copied to clipboard

Allow customizing `ThriftFieldMaskerSelectorProvider`

Open ikhoon opened this issue 4 months ago • 2 comments

In ContentSanitizer, Thrift messages are currently masked and serialized in the Thrift TJSON protocol format. https://github.com/line/armeria/blob/883e8bf6024ddbc9090cc9e4b1586630ac11ee5b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/common/thrift/logging/TBaseSerializer.java#L36 However, since TJSON includes only field IDs, it significantly reduces human readability.

content={"service":"...","method":"hello","params":[{"2":{"str":""},"13":{"i32":0}}]}}

There may be a need to log messages in a more human-readable format, such as the Thrift TText protocol or TText with named enums. To support this, I propose providing a builder for ThriftFieldMaskerSelectorProvider and allowing it to be set in the ContentSanitizerBuilder.

var maskerSelectorProvider = 
  ThriftFieldMaskerSelectorProvider
    .builder()
    .tProtocolFactory(ThriftProtocolFactories.text())
    // This method is mutually exclusive with tProtocolFactory()
    .serializer(...)
    .deserializers(...)
    .build();

ContentSanitizer
  .builder()
  .fieldMaskerSelectorProvider(maskerSelectorProvider)

Additionally, I suggest removing useDefaultPrettyPrinter() option for TText protocol. This change would help reduce network and log payload size and improve consistency with the other log message formats, which use a single-line format. https://github.com/line/armeria/blob/0960d091bfc7f350c17e68f57cc627de584b9705/thrift/thrift0.13/src/main/java/com/linecorp/armeria/common/thrift/text/TTextProtocol.java#L834-L835

ikhoon avatar Aug 06 '25 03:08 ikhoon

Hello, if no one is currently assigned to this task, I’d be happy to work on it. Would that be possible?

hyunw9 avatar Aug 11 '25 12:08 hyunw9

The masking API was added recently and is still under active development by @jrhee17, so it may not be suitable for external contributors at this time.

ikhoon avatar Aug 12 '25 01:08 ikhoon