Update LoggingEventJsonPatternParser to fix breaking logback change

[jordanjennings]

Fixes #1060

Although this fixes the compatibility issue with logback 1.5.13, it does have breaking version compatibility changes (due to the underlying breaking change made by logback itself). I've updated the README to indicate new minimum versions, but I didn't change anything with the pom.xml version number, even though this might be worthy of a major version bump since it breaks backwards compatibility with logback versions.

[philsttr]

Thanks @jordanjennings ! I really appreciate your investigation and fix.

I agree that this would require a major version bump in logstash-logback-encoder.

However, I am interested to see if there is a way to support both the old and new logback versions in 8.x. Even if it was "ugly", we could probably live with it until the next major logstash-logback-encoder version, at which point it could be removed.

[jordanjennings]

@philsttr Yes, there is a way using reflection, but it's a bit ugly. Here's how I originally approached the fix:

  @Override
  protected PatternLayoutBase<ILoggingEvent> createLayout() {
    PatternLayoutBase<ILoggingEvent> layout = new PatternLayout();

    try {
      Field field = PatternLayoutBase.class.getDeclaredField("instanceConverterMap");
      ParameterizedType type = (ParameterizedType) field.getGenericType();
      Type valueType = type.getActualTypeArguments()[1];

      if (valueType.getTypeName().equals("java.lang.String")) {
        putPropertyUnchecked(layout.getInstanceConverterMap(), EnhancedPropertyConverter.class.getName());
      } else if (valueType.getTypeName().equals("java.util.function.Supplier<ch.qos.logback.core.pattern.DynamicConverter>")) {
        putPropertyUnchecked(layout.getInstanceConverterMap(), EnhancedPropertyConverter::new);
      } else {
        throw new IncompatibleClassChangeError(
            "Unexpected type found for instanceConverterMap, logback-core dependency version is not compatible");
      }

    } catch (NoSuchFieldException e) {
      throw new IncompatibleClassChangeError(
          "Field instanceConverterMap not found, logback-core dependency version is not compatible");
    }
    return layout;
  }

  @SuppressWarnings({"rawtypes", "unchecked"})
  private void putPropertyUnchecked(Map map, String value) {
    map.put("property", value);
  }

  @SuppressWarnings({"unchecked", "rawtypes"})
  private void putPropertyUnchecked(Map map, Supplier<Object> value) {
    map.put("property", value);
  }

I didn't go with this approach because it felt very brittle and likely to break again. However, it does keep compatibility until a major release of logstash-logback-encoder can be done. What do you think about this approach?

[philsttr]

Hey @jordanjennings ,

Yeah, I agree it's a bit brittle, but this repo has had to do similar things in the past to maintain compatibility with various breaking changes in logback. So, this works for me. And it's easy to remove in the next major version of logstash-logback-encoder, so at least it's not permanent.

[philsttr]

Hey @jordanjennings, I incorporated your approach and merged it in f4e5ff3

Thanks again for the contribution!

[jordanjennings]

@philsttr that’s fantastic news, thank you! What’s the expected timeline for cutting a release?

[philsttr]

Working on it now