Allow setting a custom EventListener in ApiClientBuilder

[kabigon-sung]

Hi Team,

Thank you for your great work on developing the SDK for LINE Bot in Java.

I noticed that you have already implemented HttpInterceptor interface, which allows the SDK users to add a custom Interceptor to the underlying OKHttpClient. (https://square.github.io/okhttp/features/interceptors/) With this support, we can easily add our own custom HttpInterceptor or reuse existing OKHttpClient's Interceptor by wrapping it accordingly.

I was wondering if it would be possible to also allow setting a custom EventListener on the underlying OkHttpClient.(https://square.github.io/okhttp/features/events/)

Based on our discussion, rather than introducing a new interface like HttpInterceptor, it seems preferable to expose the EventListener directly, as it is easier for users to use and also leads to a simpler implementation.

This pull request introduces a proof of concept for adding such support. The implementation closely mirrors that of the custom interceptor mechanism , although EventListener is a bit more involved than Interceptor.

With this feature, SDK users will be able to set a custom EventListener to suit their needs when creating the API client. For Example:

MessagingApiClient.builder("your access token")
                  .apiEndPoint(URI.create(lineApiUrl))
                  .addInterceptor(new MyHttpInterceptor)
                  .setEventListener(new MyEventListener())
                  .build();

My use case for OKHttpClient's EventListener.
In our system, we also use OkHttpClient along with Retrofit. To enhance the observability of API calls in our system, we leverage the OkHttpMetricsEventListener provided by Micrometer to record HTTP request metrics. (https://docs.micrometer.io/micrometer/reference/reference/okhttpclient.html).

We would like to attach this metrics to LINE Bot SDK's client by adapting a Micrometer's OkHttpMetricsEventListener, like this:

MessagingApiClient.builder("your access token")
                  .apiEndPoint(URI.create(lineApiUrl))
                  .addInterceptor(new MyHttpInterceptor)
                  .setEventListener(new OkHttpMetricsEventListener())
                  .build();

Thank you for considering this suggestion. I look forward to your feedback!

[CLAassistant]

All committers have signed the CLA.

[eucyt]

I apologize for the delayed response 🙇

[eucyt]

I believe you implemented HttpEventListener by wrapping okHttp's EventListener with reference to the Interceptor implementation, considering consistency with existing code.

However, with this approach, sdk users need to implement a class that inherits from the wrapped HttpEventListener class each time, which is cumbersome. Therefore, how about directly acceptting okHttp's EventListener as follows...? 🙏

Suggested change

	public ApiClientBuilder<T> setEventListener(HttpEventListener httpEventListener) {
	this.eventListener = new EventListenerAdapter(httpEventListener);
	return this;
	}
	public ApiClientBuilder<T> setEventListener(EventListener eventListener) {
	this.eventListener = eventListener;
	return this;
	}

Alternatively, I think it would also be good to return a builder so that users can set EventListener and other components as they prefer.

[kabigon-sung]

Thanks for your suggestion.

If directly accepting an HttpEventListener is not a concern from your perspective, I think it’s a simple and convenient approach.

I also noticed that you mentioned another approach — returning a builder so that users can set the EventListener and other components. Could you explain this approach in more detail? (It seems that the OkHttpClientBuilder is created in build method and pass to the Retrofit.Builder )

[eucyt]

Thank you for your confirmation!

I was imagining a structure that wouldn't require additional implementation when adding not just EventListener but other options as well. Specifically, I was thinking about making the OkHttpClientBuilder directly accessible to users so they could handle the client themselves. However, as you mentioned, it would need to be passed to the Retrofit.Builder afterward, so it probably wouldn't result in a clean implementation.

It seems better to simply allow setting the EventListener directly. 🙏

[kabigon-sung]

Thank you for your detail explanation. I've modified the implementation to directly allow setting a EventListener. Kindly review the PR and let me know if there are any issues.

[eucyt]

Thank you!
I confirmed that it's possible to set okHttpMetricsEventListener in my local.