From 8649855084b57c3a24260473f4baa1473aff0125 Mon Sep 17 00:00:00 2001 From: kart2bc Date: Tue, 3 Jun 2025 12:41:37 -0400 Subject: [PATCH] cleanup logging format property --- jobs/gorouter/spec | 7 ---- jobs/gorouter/templates/drain.erb | 8 +--- jobs/gorouter/templates/gorouter.yml.erb | 14 ------- jobs/gorouter/templates/post-start.erb | 2 +- jobs/gorouter/templates/pre-start.erb | 2 +- jobs/tcp_router/templates/post-start.erb | 2 +- spec/gorouter_templates_spec.rb | 37 ------------------- .../gorouter/config/config.go | 2 +- .../gorouter/config/config_test.go | 2 - src/routing_utils/syslog_utils.sh | 14 +------ 10 files changed, 7 insertions(+), 83 deletions(-) diff --git a/jobs/gorouter/spec b/jobs/gorouter/spec index 8378da138..a2c61ba94 100644 --- a/jobs/gorouter/spec +++ b/jobs/gorouter/spec @@ -352,13 +352,6 @@ properties: stderr logs. Available fields are: backend_time, dial_time, dns_time, failed_attempts, failed_attempts_time, local_address, tls_time default: [] - router.logging.format.timestamp: - description: | - Format for timestamp in component logs. Valid values are 'rfc3339', 'deprecated', and 'unix-epoch'." - 'rfc3339' is the recommended format. It will result in all timestamps controlled by gorouter to be in RFC3339 format, which is human readable. This includes stdout, pre-start, and post-start logs. This does not include stderr logs from golang libraries. - 'deprecated' will result in all timestamps being in the format they were before the rfc3339 flag was introduced. This format is different for different logs. We do not recommend using this flag unless you have scripts that expect a particular timestamp format. - 'unix-epoch' is an old flag that we do not recommend using, but we are keeping for backwards compatibility. It will result in the gorouter logs to be in unix-epoch format. This does not effect pre-start or post-start logs. This does not effect stderr logs from golang libaries. - default: "rfc3339" router.enable_proxy: description: "Enables support for the popular PROXY protocol, allowing downstream load balancers that do not support HTTP to pass along client information." default: false diff --git a/jobs/gorouter/templates/drain.erb b/jobs/gorouter/templates/drain.erb index de5f6b6da..3156d8dce 100755 --- a/jobs/gorouter/templates/drain.erb +++ b/jobs/gorouter/templates/drain.erb @@ -6,17 +6,11 @@ set -e -x pidfile=/var/vcap/sys/run/bpm/gorouter/gorouter.pid healthchecker_pidfile=/var/vcap/sys/run/bpm/gorouter/gorouter-healthchecker.pid log_file=/var/vcap/sys/log/gorouter/drain.log -log_format=<%= p("router.logging.format.timestamp") %> mkdir -p "$(dirname ${log_file})" log() { log_string=$1 - - if [ "$log_format" == "deprecated" ]; then - echo "$(date): ${log_string}" >> ${log_file} - else - echo "$(date +%Y-%m-%dT%H:%M:%S.%NZ): ${log_string}" >> ${log_file} - fi + echo "$(date +%Y-%m-%dT%H:%M:%S.%NZ): ${log_string}" >> ${log_file} } if [[ ! -f ${healthchecker_pidfile} ]]; then diff --git a/jobs/gorouter/templates/gorouter.yml.erb b/jobs/gorouter/templates/gorouter.yml.erb index deea6c9c5..def14ae9c 100644 --- a/jobs/gorouter/templates/gorouter.yml.erb +++ b/jobs/gorouter/templates/gorouter.yml.erb @@ -409,11 +409,6 @@ if p('routing_api.enabled') } end -if !['rfc3339', 'deprecated', 'unix-epoch'].include?(p('router.logging.format.timestamp')) - raise "'#{p('router.logging.format.timestamp')}' is not a valid timestamp format for the property 'router.logging.format.timestamp'. Valid options are: 'rfc3339', 'deprecated', and 'unix-epoch'." -end - - extra_access_log_fields = [] # Support `enable_log_attempts_details` which was internally migrated to the new dynamic system. if p('router.enable_log_attempts_details') @@ -428,12 +423,6 @@ if_p('router.logging.extra_access_log_fields') do |extra_fields| extra_access_log_fields.concat(extra_fields) end - -timestamp_format=p('router.logging.format.timestamp') -if timestamp_format == 'deprecated' - timestamp_format = 'unix-epoch' -end - params['logging'] = { 'syslog' => p('router.logging.syslog_tag'), 'syslog_truncate' => p('router.logging.syslog_message_limit'), @@ -446,9 +435,6 @@ params['logging'] = { 'disable_log_forwarded_for' => p('router.disable_log_forwarded_for'), 'disable_log_source_ip' => p('router.disable_log_source_ip'), 'redact_query_params' => p('router.redact_query_parameters'), - 'format' => { - 'timestamp' => timestamp_format, - }, } params['enable_ssl'] = p('router.enable_ssl') diff --git a/jobs/gorouter/templates/post-start.erb b/jobs/gorouter/templates/post-start.erb index 4b20d4e75..f79468631 100644 --- a/jobs/gorouter/templates/post-start.erb +++ b/jobs/gorouter/templates/post-start.erb @@ -4,7 +4,7 @@ LOG_DIR=/var/vcap/sys/log/gorouter source /var/vcap/packages/routing_utils/syslog_utils.sh <% lb_delay = p("router.load_balancer_healthy_threshold") %> -tee_output_to_sys_log "${LOG_DIR}" "post-start" <%= p("router.logging.format.timestamp") %> +tee_output_to_sys_log "${LOG_DIR}" "post-start" echo "Waiting <%= lb_delay %> seconds to allow load balancer to consider gorouter healthy..." sleep <%= lb_delay %> diff --git a/jobs/gorouter/templates/pre-start.erb b/jobs/gorouter/templates/pre-start.erb index 44811f6aa..cfc95addc 100644 --- a/jobs/gorouter/templates/pre-start.erb +++ b/jobs/gorouter/templates/pre-start.erb @@ -4,7 +4,7 @@ LOG_DIR=/var/vcap/sys/log/gorouter source /var/vcap/packages/routing_utils/pid_utils.sh source /var/vcap/packages/routing_utils/syslog_utils.sh -tee_output_to_sys_log "${LOG_DIR}" "pre-start" <%= p("router.logging.format.timestamp") %> +tee_output_to_sys_log "${LOG_DIR}" "pre-start" <% diff --git a/jobs/tcp_router/templates/post-start.erb b/jobs/tcp_router/templates/post-start.erb index 97a19f614..83190ced9 100644 --- a/jobs/tcp_router/templates/post-start.erb +++ b/jobs/tcp_router/templates/post-start.erb @@ -4,7 +4,7 @@ LOG_DIR=/var/vcap/sys/log/tcp_router source /var/vcap/packages/routing_utils/syslog_utils.sh <% lb_delay = p("tcp_router.load_balancer_healthy_threshold") %> -tee_output_to_sys_log "${LOG_DIR}" "post-start" rfc3339 +tee_output_to_sys_log "${LOG_DIR}" "post-start" echo "Waiting <%= lb_delay %> seconds to allow load balancer to consider tcp_router healthy..." sleep <%= lb_delay %> diff --git a/spec/gorouter_templates_spec.rb b/spec/gorouter_templates_spec.rb index 1e59c0d8c..0701a8b2a 100644 --- a/spec/gorouter_templates_spec.rb +++ b/spec/gorouter_templates_spec.rb @@ -1474,43 +1474,6 @@ end context 'logging' do - context 'when timestamp format is not provided' do - it 'it defaults to rfc3339' do - expect(parsed_yaml['logging']['format']['timestamp']).to eq('rfc3339') - end - end - - context 'when timestamp format is provided' do - before do - deployment_manifest_fragment['router']['logging'] = { 'format' => { 'timestamp' => 'unix-epoch' } } - end - - it 'it sets the value correctly' do - expect(parsed_yaml['logging']['format']['timestamp']).to eq('unix-epoch') - end - end - - context 'when the timestamp format is set to deprecated' do - before do - deployment_manifest_fragment['router']['logging'] = { 'format' => { 'timestamp' => 'deprecated' } } - end - - it 'sets the value to be unix-epoch' do - expect(parsed_yaml['logging']['format']['timestamp']).to eq('unix-epoch') - end - end - - context 'when an invalid timestamp format is provided' do - before do - deployment_manifest_fragment['router']['logging'] = { 'format' => { 'timestamp' => 'meow' } } - end - - it 'raises error' do - err_msg = "'meow' is not a valid timestamp format for the property 'router.logging.format.timestamp'. Valid options are: 'rfc3339', 'deprecated', and 'unix-epoch'." - - expect { parsed_yaml }.to raise_error(RuntimeError, err_msg) - end - end context 'when enable_detailed_attempts_logging is not provided' do it 'it does not set the correspoding extra access log values' do diff --git a/src/code.cloudfoundry.org/gorouter/config/config.go b/src/code.cloudfoundry.org/gorouter/config/config.go index ab529b600..ad2893eec 100644 --- a/src/code.cloudfoundry.org/gorouter/config/config.go +++ b/src/code.cloudfoundry.org/gorouter/config/config.go @@ -238,7 +238,7 @@ type TLSPem struct { var defaultLoggingConfig = LoggingConfig{ Level: "debug", MetronAddress: "localhost:3457", - Format: FormatConfig{"unix-epoch"}, + Format: FormatConfig{"rfc3339"}, JobName: "gorouter", RedactQueryParams: REDACT_QUERY_PARMS_NONE, SyslogNetwork: "udp", diff --git a/src/code.cloudfoundry.org/gorouter/config/config_test.go b/src/code.cloudfoundry.org/gorouter/config/config_test.go index 7b9d6523e..30ed6a503 100644 --- a/src/code.cloudfoundry.org/gorouter/config/config_test.go +++ b/src/code.cloudfoundry.org/gorouter/config/config_test.go @@ -459,7 +459,6 @@ suspend_pruning_if_nats_unavailable: true Expect(config.Logging.DisableLogForwardedFor).To(Equal(false)) Expect(config.Logging.DisableLogSourceIP).To(Equal(false)) Expect(config.Logging.RedactQueryParams).To(Equal("none")) - Expect(config.Logging.Format.Timestamp).To(Equal("unix-epoch")) }) It("sets default access log config", func() { @@ -536,7 +535,6 @@ logging: Expect(config.Logging.Level).To(Equal("debug2")) Expect(config.Logging.LoggregatorEnabled).To(Equal(true)) Expect(config.Logging.JobName).To(Equal("gorouter")) - Expect(config.Logging.Format.Timestamp).To(Equal("just_log_something")) }) It("sets the rest of config", func() { diff --git a/src/routing_utils/syslog_utils.sh b/src/routing_utils/syslog_utils.sh index fbe45edf7..7dc9585ca 100644 --- a/src/routing_utils/syslog_utils.sh +++ b/src/routing_utils/syslog_utils.sh @@ -6,19 +6,9 @@ function tee_output_to_sys_log() { declare -r log_dir="$1" declare -r log_name="$2" - declare -r log_format="$3" - if [ "$log_format" == "deprecated" ]; then - exec > >(tee -a >(logger -p user.info -t "vcap.${log_name}.stdout") | prepend_datetime >>"${log_dir}/${log_name}.log") - exec 2> >(tee -a >(logger -p user.error -t "vcap.${log_name}.stderr") | prepend_datetime >>"${log_dir}/${log_name}.err.log") - else - exec > >(tee -a >(logger -p user.info -t "vcap.${log_name}.stdout") | prepend_rfc3339_datetime >>"${log_dir}/${log_name}.log") - exec 2> >(tee -a >(logger -p user.error -t "vcap.${log_name}.stderr") | prepend_rfc3339_datetime >>"${log_dir}/${log_name}.err.log") - fi -} - -function prepend_datetime() { - perl -ne 'BEGIN { use Time::HiRes "time"; use POSIX "strftime"; STDOUT->autoflush(1) }; my $t = time; my $fsec = sprintf ".%06d", ($t-int($t))*1000000; my $time = strftime("[%Y-%m-%d %H:%M:%S".$fsec."%z]", localtime $t); print("$time $_")' + exec > >(tee -a >(logger -p user.info -t "vcap.${log_name}.stdout") | prepend_rfc3339_datetime >>"${log_dir}/${log_name}.log") + exec 2> >(tee -a >(logger -p user.error -t "vcap.${log_name}.stderr") | prepend_rfc3339_datetime >>"${log_dir}/${log_name}.err.log") } function prepend_rfc3339_datetime() {