Closing httpclient properly on logback shutdown

[SuramVishal]

Code at present only closes httpclient, if eventsBatch.size() > 0.

But there might be scenarios where logback calls shutdown and there are no events to flush. Therefore sender.stopHttpClient() never being called preventing a main method exit due to non-deamon thread usage of httpClient.

[shakeelmohamed]

@SuramVishal This change seems fine. Have you verified that it addresses your issue?

Please fix the indents so it matches the rest of the file.

[SuramVishal]

@shakeelmohamed Yes, I have tested this and it does address my issue. Would you be able to make this a part of a new release (potentially 1.7.2) now? Would be really helpful, so that we can use this in our work. Thanks for the swift review by the way.

[SuramVishal]

Hi @shakeelmohamed any help? :)

[shakeelmohamed]

@SuramVishal do you have some steps to reproduce the issue?
It's great to hear you've got your issue fixed, we just want to verify it on our end and potentially write a test for this scenario.

This will definitely be in the next release, I'm not exactly sure when that will be though.

[SuramVishal]

Okay to reproduce:

public class Application {

    public static void main(String[] args) {
        final org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(Application.class);
        log.info("Hello World!");
    }
}

with logback appender as

    <appender name="SPLUNK" class="com.splunk.logging.HttpEventCollectorLogbackAppender">
        <url>${SPLUNK_URL}</url>
        <index>${SPLUNK_INDEX}</index>
        <source>${SPLUNK_SOURCE}</source>
        <sourcetype>${SPLUNK_SOURCE_TYPE}</sourcetype>
        <token>${SPLUNK_TOKEN}</token>
        <disableCertificateValidation>true</disableCertificateValidation>
        <layout class="ch.qos.logback.classic.PatternLayout">
            <pattern>%msg</pattern>
        </layout>
        <messageFormat>json</messageFormat>
    </appender>

[SuramVishal]

@shakeelmohamed It would be really helpful if you can help prioritise this issue. This is major when it comes to all short living applications using splunk appender. :) Please do let me know if you need any further details.

[fantavlik]

Hey @SuramVishal thanks for the testcase, I'm going to try to reproduce this today and hopefully get this merged. Thanks for your help!

[SuramVishal]

@fantavlik, much appreciated. Please let me know if you have any difficulties in reproducing. :)

[fantavlik]

Apologies, I ran into issues setting up my environment. I've been able to repro the fact that the httpClient is not closed for your setup. Thanks for your contribution and we will get this into the next release.

[SuramVishal]

Thank you @fantavlik , @shakeelmohamed for reviewing the PR. May I kindly know when the next release would be? Can we get it asap? So that we can continue to use the library in our work. :)

[fantavlik]

I'm cutting a 1.7.2 release today! 🎉