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

Conditional expressions evaluated in a weird order causing configuration discrepancies #829

Open
ecourreges-orange opened this issue Jul 5, 2024 · 3 comments

Comments

@ecourreges-orange
Copy link

Hello,
I will show you two configurations, one that works, one that doesn't, and maybe you understand what is wrong.
If it's not a bug, this requires at least clear documentation warnings on conditionals.

In all cases i have SYSLOG_HOST = localhost in my environment, because I want my UdpJsonAppender to always work to have remote logs, and I want to activate logs on the file system only on dev environments

This first logback.xml works for all combinations of API_LOGS_FS_ENABLED, API_LOGS_TXT_ENABLED: specifically If these two variables are false, I have my last UdpJsonAppender working on the root logger, and the JsonAppender and TextAppender create empty files which are never filled, I guess this is expected.

<?xml version="1.0" encoding="UTF-8"?>
 <configuration
  scan="{{API_SCAN_LOG_CONF | default("false")}}"
  scanPeriod="{{API_SCAN_LOG_CONF_PERIOD | default("30 seconds")}}"
  debug="{{API_LOG_CONF_DEBUG | default("false")}}">

  <variable name="TOMCAT_LOGS_DIR" value="{{TOMCAT_LOGS_DIR | default("/ var/ log/" ~ API_NAME ~ "/ apache-tomcat/ log")}}" />
  <variable name="API_LOGS_FS_ENABLED" value="{{API_LOGS_FS_ENABLED | default(" false")}}" />
  <variable name="API_LOGS_TXT_ENABLED" value="{{API_LOGS_TXT_ENABLED | default(" false")}}" />
  <variable name="API_LOGS_PREFIX" value="ws" />
  <variable name="API_LOGS_JSON_SUFFIX" value="log.json" />
  <variable name="API_LOGS_TXT_SUFFIX" value="log.txt" />
  <variable name="API_LOG_LEVEL" value="{{API_LOG_LEVEL | default(" WARN")}}" />


  <appender name="JsonAppender" class="ch.qos.logback.core.rolling.RollingFileAppender">
    <file>${TOMCAT_LOGS_DIR}/${API_LOGS_PREFIX}.${API_LOGS_JSON_SUFFIX}</file>
    <rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy">
      <fileNamePattern>${TOMCAT_LOGS_DIR}/${API_LOGS_PREFIX}.%d{yyyy-MM-dd}.${API_LOGS_JSON_SUFFIX}</fileNamePattern>
      <maxHistory>{{API_LOG_MAX_HISTORY | default(1)}}</maxHistory>
      <totalSizeCap>{{API_LOG_TOTAL_SIZE_CAP | default("100MB")}}</totalSizeCap>
    </rollingPolicy>
    <append>true</append>
    <encoder class="net.logstash.logback.encoder.LoggingEventCompositeJsonEncoder">
      <providers>
        <timestamp />
        <version />
        <logLevel />
        <threadName />
        <loggerName />
        <message />
        <mdc>
          <fieldName>mdc</fieldName>
        </mdc>
        <stackTrace>
          <throwableConverter class="net.logstash.logback.stacktrace.ShortenedThrowableConverter">
            <maxDepthPerThrowable>{{API_LOG_STACKTRACES_DEPTH | default(1)}}</maxDepthPerThrowable>
          </throwableConverter>
        </stackTrace>
      </providers>
    </encoder>
  </appender>


  <appender name="TextAppender" class="ch.qos.logback.core.rolling.RollingFileAppender">
    <file>${TOMCAT_LOGS_DIR}/${API_LOGS_PREFIX}.${API_LOGS_TXT_SUFFIX}</file>
    <rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
      <fileNamePattern>
        ${TOMCAT_LOGS_DIR}/${API_LOGS_PREFIX}.%d{yyyy-MM-dd}.%i.${API_LOGS_TXT_SUFFIX}.gz</fileNamePattern>
      <maxFileSize>{{API_LOG_MAX_FILE_SIZE | default("100MB")}}</maxFileSize>
      <maxHistory>{{API_LOG_MAX_DAYS_HISTORY | default(2)}}</maxHistory>
      <totalSizeCap>{{API_LOG_MAX_TOTAL_SIZE | default("200MB")}}</totalSizeCap>
    </rollingPolicy>
    <append>true</append>
    <encoder>
      <pattern>%boldWhite(%d{HH:mm:ss.SSS}) [%thread] %highlight(%-5level) %green(%logger{36}) -
        %.-{{API_LOG_MSG_MAX_LENGTH | default(256)}}msg%n %ex{{'{'}}{{API_LOG_STACKTRACES_DEPTH |
        default(2)}}{{'}'}}</pattern>
    </encoder>
  </appender>

  <if condition="${API_LOGS_TXT_ENABLED}">
    <then>
      <include file="${API_CONF_DIR}/logback-inc-txt.xml" />
    </then>
  </if>

  <appender name="UdpJsonAppender" class="net.logstash.logback.appender.LogstashUdpSocketAppender">
    <host>{{SYSLOG_HOST | default("localhost")}}</host>
    <port>{{SYSLOG_PORT | default(514)}}</port>
    <layout class="com.orange.skeleton.logback.util.SyslogLayout">
      <senderHost>{{SYSLOG_API_SENDER_HOST | default(API_NAME ~ "_host")}}</senderHost>
      <appName>{{SYSLOG_API_WS_APP_NAME | default("app." ~ API_NAME ~ ".ws")}}</appName>
      <maxSize>{{SYSLOG_MESSAGE_MAX_SIZE | default(65000)}}</maxSize>
      <payloadLayout class="net.logstash.logback.layout.LoggingEventCompositeJsonLayout">
        <providers>
          <timestamp />
          <version />
          <logLevel />
          <threadName />
          <loggerName />
          <message />
          <mdc>
            <fieldName>mdc</fieldName>
          </mdc>
          <stackTrace>
            <throwableConverter class="net.logstash.logback.stacktrace.ShortenedThrowableConverter">
              <maxDepthPerThrowable>{{API_LOG_STACKTRACES_DEPTH | default(1)}}</maxDepthPerThrowable>
            </throwableConverter>
          </stackTrace>
        </providers>
      </payloadLayout>
    </layout>
  </appender>


  <root level="${API_LOG_LEVEL}">
    <if condition="${API_LOGS_FS_ENABLED}">
      <then>
        <appender-ref ref="JsonAppender" />
      </then>
    </if>
    <if condition="${API_LOGS_TXT_ENABLED}">
      <then>
        <appender-ref ref="TextAppender" />
      </then>
    </if>
    <if condition='isDefined("SYSLOG_HOST")'>
      <then>
        <appender-ref ref="UdpJsonAppender" />
      </then>
    </if>
  </root>

  <include file="${API_CONF_DIR}/logback-inc-api.xml" optional="{{API_LOGBACK_INCLUDED_OPTIONAL | default("true")}}" />
  <include file="${API_CONF_DIR}/logback-test.xml" optional="true" />

</configuration>

Whereas in this one, the UdpJsonAppender works only if API_LOGS_FS_ENABLED, API_LOGS_TXT_ENABLED are both true, which is totally unexpected, and makes me think that the root logger evaluates the last if condition only if the appender-ref of JsonAppender and TextAppender are valid, which in this case they are not since their block is also enclosed in a conditional which is false, so the appenders are never declared, but the ifs inside the root are also false so they should be ignored and the one for UdpJsonAppender should still be OK.

This one would have the clear advantage of not creating the resources or the file for the unused appenders, but in practice, it doesn't work.

<?xml version="1.0" encoding="UTF-8"?>

<!-- For assistance related to logback-translator or configuration  -->
<!-- files in general, please contact the logback user mailing list -->
<!-- at http://www.qos.ch/mailman/listinfo/logback-user             -->
<!--                                                                -->
<!-- For professional support please see                            -->
<!--    http://www.qos.ch/shop/products/professionalSupport         -->
<!--                                                                -->
<configuration
  scan="{{API_SCAN_LOG_CONF | default("false")}}"
  scanPeriod="{{API_SCAN_LOG_CONF_PERIOD | default("30 seconds")}}"
  debug="{{API_LOG_CONF_DEBUG | default("false")}}">

  <variable name="TOMCAT_LOGS_DIR" value="{{TOMCAT_LOGS_DIR | default("/var/log/" ~ API_NAME ~ "/apache-tomcat/log")}}" />
  <variable name="API_LOGS_FS_ENABLED" value="{{API_LOGS_FS_ENABLED | default("false")}}" />
  <variable name="API_LOGS_TXT_ENABLED" value="{{API_LOGS_TXT_ENABLED | default("false")}}" />
  <variable name="API_LOGS_PREFIX" value="ws" />
  <variable name="API_LOGS_JSON_SUFFIX" value="log.json" />
  <variable name="API_LOGS_TXT_SUFFIX" value="log.txt" />
  <variable name="API_LOG_LEVEL" value="{{API_LOG_LEVEL | default("WARN")}}" />

  <if condition="${API_LOGS_FS_ENABLED}">
    <then>
      <appender name="JsonAppender" class="ch.qos.logback.core.rolling.RollingFileAppender">
        <file>${TOMCAT_LOGS_DIR}/${API_LOGS_PREFIX}.${API_LOGS_JSON_SUFFIX}</file>
        <rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy">
          <fileNamePattern>${TOMCAT_LOGS_DIR}/${API_LOGS_PREFIX}.%d{yyyy-MM-dd}.${API_LOGS_JSON_SUFFIX}</fileNamePattern>
          <maxHistory>{{API_LOG_MAX_HISTORY | default(1)}}</maxHistory>
          <totalSizeCap>{{API_LOG_TOTAL_SIZE_CAP | default("100MB")}}</totalSizeCap>
        </rollingPolicy>
        <append>true</append>
        <encoder class="net.logstash.logback.encoder.LoggingEventCompositeJsonEncoder">
          <providers>
            <timestamp/>
            <version/>
            <logLevel/>
            <threadName/>
            <loggerName/>
            <message/>
            <mdc><fieldName>mdc</fieldName></mdc>
            <stackTrace>
              <throwableConverter class="net.logstash.logback.stacktrace.ShortenedThrowableConverter">
                <maxDepthPerThrowable>{{API_LOG_STACKTRACES_DEPTH | default(1)}}</maxDepthPerThrowable>
              </throwableConverter>
            </stackTrace>
          </providers>
        </encoder>
      </appender>
    </then>
  </if>

  <if condition="${API_LOGS_TXT_ENABLED}">
    <then>
      <appender name="TextAppender" class="ch.qos.logback.core.rolling.RollingFileAppender">
        <file>${TOMCAT_LOGS_DIR}/${API_LOGS_PREFIX}.${API_LOGS_TXT_SUFFIX}</file>
        <rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
          <fileNamePattern>${TOMCAT_LOGS_DIR}/${API_LOGS_PREFIX}.%d{yyyy-MM-dd}.%i.${API_LOGS_TXT_SUFFIX}.gz</fileNamePattern>
          <maxFileSize>{{API_LOG_MAX_FILE_SIZE | default("100MB")}}</maxFileSize>
          <maxHistory>{{API_LOG_MAX_DAYS_HISTORY | default(2)}}</maxHistory>
          <totalSizeCap>{{API_LOG_MAX_TOTAL_SIZE | default("200MB")}}</totalSizeCap>
        </rollingPolicy>
        <append>true</append>
        <encoder>
          <pattern>%boldWhite(%d{HH:mm:ss.SSS}) [%thread] %highlight(%-5level) %green(%logger{36}) - %.-{{API_LOG_MSG_MAX_LENGTH | default(256)}}msg%n %ex{{'{'}}{{API_LOG_STACKTRACES_DEPTH | default(2)}}{{'}'}}</pattern>
        </encoder>
      </appender>

      <include file="${API_CONF_DIR}/logback-inc-txt.xml" />
    </then>
  </if>

  <if condition='isDefined("SYSLOG_HOST")'>
    <then>
      <appender name="UdpJsonAppender" class="net.logstash.logback.appender.LogstashUdpSocketAppender">
        <host>{{SYSLOG_HOST | default("localhost")}}</host>
        <port>{{SYSLOG_PORT | default(514)}}</port>
        <layout class="com.orange.skeleton.logback.util.SyslogLayout">
          <senderHost>{{SYSLOG_API_SENDER_HOST | default(API_NAME ~ "_host")}}</senderHost>
          <appName>{{SYSLOG_API_WS_APP_NAME | default("app." ~ API_NAME ~ ".ws")}}</appName>
          <maxSize>{{SYSLOG_MESSAGE_MAX_SIZE | default(65000)}}</maxSize>
          <payloadLayout class="net.logstash.logback.layout.LoggingEventCompositeJsonLayout">
            <providers>
              <timestamp/>
              <version/>
              <logLevel/>
              <threadName/>
              <loggerName/>
              <message/>
              <mdc><fieldName>mdc</fieldName></mdc>
              <stackTrace>
                <throwableConverter class="net.logstash.logback.stacktrace.ShortenedThrowableConverter">
                  <maxDepthPerThrowable>{{API_LOG_STACKTRACES_DEPTH | default(1)}}</maxDepthPerThrowable>
                </throwableConverter>
              </stackTrace>
            </providers>
          </payloadLayout>
        </layout>
      </appender>
    </then>
  </if>

  <root level="${API_LOG_LEVEL}">
    <if condition="${API_LOGS_FS_ENABLED}">
      <then>
        <appender-ref ref="JsonAppender" />
      </then>
    </if>
    <if condition="${API_LOGS_TXT_ENABLED}">
      <then>
        <appender-ref ref="TextAppender" />
      </then>
    </if>
    <if condition='isDefined("SYSLOG_HOST")'>
      <then>
        <appender-ref ref="UdpJsonAppender" />
      </then>
    </if>
  </root>

  <include file="${API_CONF_DIR}/logback-inc-api.xml" optional="{{API_LOGBACK_INCLUDED_OPTIONAL | default("true")}}" />
  <include file="${API_CONF_DIR}/logback-test.xml" optional="true" />

</configuration>
@ceki
Copy link
Member

ceki commented Jul 5, 2024

@ecourreges-orange Thank you for this report. Which version of logback are you using?

@ecourreges-orange
Copy link
Author

@ecourreges-orange Thank you for this report. Which version of logback are you using?

logback-classic 1.5.5

@ecourreges-orange
Copy link
Author

With logstash-logback-encoder 6.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants