search

segmentio / analytics-java

The hassle-free way to integrate analytics into any java application.
https://segment.com/libraries/java
120 stars 92 forks source link

Allow GsonBuilder to be passed in and used through Analytics and AnalyticsClient #446

Closed louislepper closed 11 months ago

louislepper commented 1 year ago

Hi there,

I'm getting some issues when upgrading services to Java 17, where previously Gson was able to automatically deserialise things like java.util.Optional, and java.util.Duration, but now gets exceptions due to disallowed reflection.

Can we change Analytics / AnalyticsClient, to allow the GsonBuilder to be passed in, so that consumers can provide TypeAdapters and TypeAdapter factories as needed, for deserialisation of any classes that they'd like to send in analytics messages?

louislepper commented 1 year ago

@pbassut - Does this make sense? Can we do this?

I also wasn't sure why gsonInstance was static - not sure if I'm missing something here.

louislepper commented 1 year ago

All works fine! could you help us tu run this command to fix format issues on Analytics.java, please? mvn spotless:apply

Done 👍

louislepper commented 1 year ago

@edsonjab / @pbassut - Can we merge this?

saidur2k commented 1 year ago

@edsonjab can we merge this branch? It's also blocking us from moving to Java 17.

louislepper commented 1 year ago

@bsneed / @MichaelGHSeg Would either of you be able to review?

kim5566 commented 11 months ago

Hey @edsonjab do you have any update on this PR? 🥺

bsneed commented 11 months ago

Following up internally, stay tuned. Will get an answer this week.

louislepper commented 11 months ago

Hi @bsneed / @wenxi-zeng / @edsonjab

Thanks for the approval. Unfortunately I don't have permission to hit merge - Would one of you be able to?

Screenshot 2023-09-27 at 9 48 47 am
edsonjab commented 11 months ago

Hi @louislepper we will request to merge your PR

MichaelGHSeg commented 11 months ago

Released in 3.5.0