Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Java Client] Switch from pretty print to compact print for configs #11609

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

michaeljmarshall
Copy link
Member

Motivation

When running the ProducerStatsRecorderImpl and ConsumerStatsRecorderImpl, there are several log lines that print configuration in pretty print formatting. This output can be easier to read, but it adds verbosity to logs, especially if there are many of these configs printing for many topics. Practically speaking, when tailing logs, these pretty printed configs take up much of one's screen and make it harder to see other logging events. Also, pretty printing can be problematic when using log aggregators that do not have good multi-line support, as the logs won't be grouped properly.

I propose switching to a compact output for these log lines to decrease the number of lines output and to make it easier for log aggregators to easily capture the whole log line.

Here is a sample of the current logging output:

20:36:18.628 [pulsar-io-4-2] INFO  org.apache.pulsar.client.impl.ProducerStatsRecorderImpl - Starting Pulsar producer perf with config: {
  "topicName" : "persistent://public/default/example",
  "producerName" : null,
  "sendTimeoutMs" : 30000,
  "blockIfQueueFull" : false,
  "maxPendingMessages" : 1000,
  "maxPendingMessagesAcrossPartitions" : 50000,
  "messageRoutingMode" : "RoundRobinPartition",
  "hashingScheme" : "JavaStringHash",
  "cryptoFailureAction" : "FAIL",
  "batchingMaxPublishDelayMicros" : 1000,
  "batchingPartitionSwitchFrequencyByPublishDelay" : 10,
  "batchingMaxMessages" : 1000,
  "batchingMaxBytes" : 131072,
  "batchingEnabled" : true,
  "chunkingEnabled" : false,
  "compressionType" : "NONE",
  "initialSequenceId" : null,
  "autoUpdatePartitions" : true,
  "autoUpdatePartitionsIntervalSeconds" : 60,
  "multiSchema" : true,
  "accessMode" : "Shared",
  "properties" : { }
}
20:36:18.628 [pulsar-io-4-2] INFO  org.apache.pulsar.client.impl.ProducerStatsRecorderImpl - Pulsar client config: {
  "serviceUrl" : "pulsar+ssl://172.17.0.11:6651",
  "authPluginClassName" : "org.apache.pulsar.client.impl.auth.AuthenticationToken",
  "authParams" : "*****",
  "authParamMap" : null,
  "operationTimeoutMs" : 30000,
  "statsIntervalSeconds" : 60,
  "numIoThreads" : 1,
  "numListenerThreads" : 1,
  "connectionsPerBroker" : 1,
  "useTcpNoDelay" : true,
  "useTls" : true,
  "tlsTrustCertsFilePath" : "/pulsar/certs/ca.crt",
  "tlsAllowInsecureConnection" : false,
  "tlsHostnameVerificationEnable" : false,
  "concurrentLookupRequest" : 5000,
  "maxLookupRequest" : 50000,
  "maxLookupRedirects" : 20,
  "maxNumberOfRejectedRequestPerConnection" : 50,
  "keepAliveIntervalSeconds" : 30,
  "connectionTimeoutMs" : 10000,
  "requestTimeoutMs" : 60000,
  "initialBackoffIntervalNanos" : 100000000,
  "maxBackoffIntervalNanos" : 60000000000,
  "enableBusyWait" : false,
  "listenerName" : null,
  "useKeyStoreTls" : false,
  "sslProvider" : null,
  "tlsTrustStoreType" : "JKS",
  "tlsTrustStorePath" : null,
  "tlsTrustStorePassword" : null,
  "tlsCiphers" : [ ],
  "tlsProtocols" : [ ],
  "memoryLimitBytes" : 0,
  "proxyServiceUrl" : null,
  "proxyProtocol" : null,
  "enableTransaction" : false
}

Further, I think this change is in line with other Pulsar logging. We already print out configuration in a compact format when starting a broker:

LOG.info("messaging service is ready, {}, cluster={}, configs={}", bootstrapMessage,
config.getClusterName(), ReflectionToStringBuilder.toString(config));

Modifications

  • Update 3 places where ObjectWriters are configured as pretty printers by removing the writerWithDefaultPrettyPrinter method call.

Verifying this change

This is a trivial rework. The result of this update is that log lines will be compact json on a single line instead of pretty printed json on many lines.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

Documentation

N/A

@Anonymitaet
Copy link
Member

@michaeljmarshall thanks for your contribution. What does N/Amean? No need doc?

@michaeljmarshall
Copy link
Member Author

@Anonymitaet - correct, no need to change any documentation. The PR is just a change to log format. The content of the logs will be the same. (N/A means not applicable.)

@codelipenghui codelipenghui added this to the 2.9.0 milestone Aug 10, 2021
@codelipenghui codelipenghui added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Aug 10, 2021
@codelipenghui codelipenghui merged commit 9af5e53 into apache:master Aug 10, 2021
@michaeljmarshall michaeljmarshall deleted the remove-pretty-print branch August 10, 2021 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants