Closed codepitbull closed 2 years ago
@codepitbull there's also a test failing here that isn't failing locally. I guess the shading is taking us past a test timeout 🤔
Yep, not an easy decision to make :)
About my reasoning: I treat posthog like I treat other agent tools (jaeger/senrry/...)
They should seamlessly integrate.
We currently also use Sentry, and this is the sentry-dep-tree:
+--- io.sentry:sentry:6.1.3
Zero dependencies to worry about.
The same with Jaeger (not currently in use by us, but I know their agent is shading the deps).
This is the current dep-tree of posthog:
+--- com.posthog.java:posthog:+ -> 1.0.2
| +--- org.json:json:20210307
| \--- com.squareup.okhttp3:okhttp:4.9.1 -> 4.10.0
| +--- com.squareup.okio:okio:3.0.0 -> 3.2.0
| | \--- com.squareup.okio:okio-jvm:3.2.0
| | +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.6.21
| | | +--- org.jetbrains.kotlin:kotlin-stdlib:1.6.21
| | | | +--- org.jetbrains.kotlin:kotlin-stdlib-common:1.6.21
| | | | \--- org.jetbrains:annotations:13.0 -> 22.0.0
| | | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.6.21
| | | \--- org.jetbrains.kotlin:kotlin-stdlib:1.6.21 (*)
| | \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.6.20 -> 1.6.21
| \--- org.jetbrains.kotlin:kotlin-stdlib:1.6.20 -> 1.6.21 (*)
This already produced a clash wit okhttp3 and the kotlin libs. I solved this by swapping out one library and then simply having your deps updated via some resolving rules.
The issue I see is that okhttp3 is rather widely used and you might cause clashes in more applications.
About CVEs: Well, they should already be handled by you since delegating that job to users of your lib won't really help your product.
We are currently using the OWASP-plugin to continuously check for CVEs in our dependencies. And there is also dependabot which solves this issue quite nicely.
I could provide you with a PR for also integrating OWASP into your build if you want to :)
I could provide you with a PR for also integrating OWASP into your build if you want to :)
The more you want to contribute the better 💖
Just the failing test and then I'm happy to merge this 💪
@codepitbull there's also a test failing here that isn't failing locally. I guess the shading is taking us past a test timeout 🤔
I just ran the tests on my mac an everything is green.
Which one is failing and could you give me the full stacktrace?
Yep, they pass locally for me. Only fail in GitHub CI https://github.com/PostHog/posthog-java/actions/runs/3015694697/jobs/5028592218
I'm guessing that the timeout is too low on low-oomph runners compared to our dev machines?
@pauldambra could you do me a favor and run the build on the main-branch? I have a feeling this has nothing to do with my changes since they shouldn't affect the tests at all since no deps are changed.
I will also provide the OWASP checks in this PR.
Hey @codepitbull ,
We're all in Rome for an offsite this week so may be slow to respond.
Ideally, can we add a separate PR to add OWASP? Helps keep the review from spiralling and we can merge things sooner (hopefully)
I'm just creating a new PR to see how the tests are
The tests pass in 7.x seconds on an unchanged branch from main https://github.com/PostHog/posthog-java/actions/runs/3128731382/jobs/5076940041
Reading that code... I can't immediately see why your PR could cause these tests to fail.
The worry is that we're breaking something else and don't have a test covering the broken thing and this is a symptom of that something broken elsewhere.
(Or it's a total red herring :))
Can you please allow the build-action to run again? I increased a timeout but I primarily would like to see if it was a fluke.
@pauldambra looks like that fixed it :)
Sorry for the delay @codepitbull
(had to lock myself in a room to finish some other work 🤣 )
Just sorting out my permissions so I can publish the new version
I'm just navigating sonatype setup so I can publish https://issues.sonatype.org/browse/OSSRH-85282
Why ?
In issue #22 I mentioned that it would be great to have transitive dependencies shaded. The reason behind this is that posthog is an add-on and shouldn't influence the dependencies in an application which simply wants to use posthog. Currently using posthog brings in several tranitive deps which we have been using in different versions in our projects.
What ?
To avoid dependency clashes I used the Maven-Shade-Plugin. This moves all transitive deps into a dedicated package (com.postman.shaded) and puts them into a Uber-jar. This DOESN'T influence how you are working with the project since this happens as part of the packaging phase so the changed packages are only ever visible when pulling the resulting jar.