Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Drop Java 6 Compatibility #511

Closed
isaachier opened this issue Aug 8, 2018 · 42 comments · Fixed by #802
Closed

Drop Java 6 Compatibility #511

isaachier opened this issue Aug 8, 2018 · 42 comments · Fixed by #802

Comments

@isaachier
Copy link
Contributor

Requirement - what kind of business use case are you trying to solve?

Allow use of Java 8 and above in client. This would simplify some of the code.

Problem - what in Jaeger blocks you from solving the requirement?

For some reason, we preserve Java 6 compatibility in the client. @felixbarny for why this was added last year when Java 6 is basically deprecated/ancient technology.

Proposal - what do you suggest to solve the problem or improve the existing situation?

Stop supporting Java 6 in future releases.

@isaachier isaachier changed the title Java 6 Compatibility Drop Java 6 Compatibility Aug 8, 2018
@black-adder
Copy link
Contributor

This would simplify some of the code - can you quantify? As an infra platform, we should support as many versions as possible.

@jpkrohling
Copy link
Collaborator

jpkrohling commented Aug 8, 2018

Java 6 is EOL for years now. I vote for dropping 1.6 support.

can you quantify?

Lambdas and default methods on interfaces are things that I miss most, from this library's perspective.

As an infra platform, we should support as many versions as possible.

IMO, it's only a real problem if we introduce breaking changes in the agent/collector that would require a bump on client versions. Otherwise, people stuck with 1.6 can just keep using an old client version (just like they keep using their old Java version).

@black-adder
Copy link
Contributor

Ok, according to this, java 6 does seem to be on the way out so I can see us dropping it, although I'd like a really good reason for doing so.

Allow use of Java 8 and above in client

Does this mean people using java 7 will be out of luck?

@vprithvi
Copy link
Contributor

vprithvi commented Aug 8, 2018

Another alternative is evaluating a library like retrolambda, which backports lambdas to Java 6.

@isaachier
Copy link
Contributor Author

isaachier commented Aug 8, 2018

Sure @black-adder:

  • Lets us remove Java6CompatibleThreadLocalRandom
  • Java 7 allows use of diamond operators so we don't have to write out Collections.<String, String>emptyMap() every time we need a new baggage map. Instead, we can do Collections.emptyMap() and the compiler figures it out from context.
  • Java 8 adds lambdas and a ton of new methods for collections.
  • Java 6 is EOL as @jpkrohling points out. There is no good reason to support it. Java 8 is the minimum standard at Uber and I imagine almost everywhere else.

Update:

  • We currently use a hack to parse span context strings: new BigInteger(parts[0], 16).longValue(). It is inefficient, but a workaround because Long.parseLong(parts[0], 16) throws an error when parts[0] is a large unsigned long (which it is), due to signed integer overflow. Java 8 fixes this with the new method Long.parseUnsignedLong(parts[0], 16).

@black-adder
Copy link
Contributor

ok sure, but it'd have to be a breaking change to prevent users of java 6 accidentally pulling a non-compatible client version.

I like prithvi's idea and would like to explore it.

@isaachier
Copy link
Contributor Author

The real question is why we added this in the first place. IDK exactly how you quantify a breaking change for a language we never really claimed to support. @felixbarny added this support in #132 (original issue #129). That's why I'd like to see if he still needs this.

@black-adder
Copy link
Contributor

never really claimed to support

The fact that we landed the change means we support it. We have to deal with it now and I'd prefer not do a breaking change.

@isaachier
Copy link
Contributor Author

Ya I get it. Bottom line: we should get rid of it in the next breaking release.

@black-adder
Copy link
Contributor

I added this to this week's Jaeger biweekly meeting agenda, should be some riveting stuff.

@felixbarny
Copy link
Contributor

felixbarny commented Aug 9, 2018

I’m not affiliated with stagemonitor anymore. @liebhaeuser do you still require Java 6 compatibly for Jaeger/stagemonitor?

@isaachier
Copy link
Contributor Author

Thanks @felixbarny for following up.

@jpkrohling
Copy link
Collaborator

If we are aware that someone needs Java 6, then we should definitely not break them. Looks like stagemonitor still uses Java 6:

https://github.com/stagemonitor/stagemonitor/blob/eca3b0cd87b7e11b3ee36d8db144cdb2b1900f57/build.gradle#L113-L114

Another alternative is evaluating a library like retrolambda, which backports lambdas to Java 6.

While lambdas are really helpful, I'm not sure I would add a bytecode transformation tool just to make use of that.

Does this mean people using java 7 will be out of luck?

Java 7 is EOL since 2015. Just to be clear: EOL means that ordinary people do not have any support from Oracle and bugs are not being fixed, not even security ones. So, people should really not be using anything less than Java 8.

it'd have to be a breaking change to prevent users of java 6 accidentally pulling a non-compatible client version

I assume that people using Java 6 are doing so because their code base is old and they can't afford to simply use a newer JVM version. In that case, I can really see them also not touching Jaeger Client Java's version. StageMonitor, for instance, is still on Jaeger Client Java v0.24.0, released in Feb this year:

https://github.com/stagemonitor/stagemonitor/blob/eca3b0cd87b7e11b3ee36d8db144cdb2b1900f57/build.gradle#L59

I cannot see how one would "accidentally" pull a newer Jaeger Client Java version without actively bumping it on Gradle/Maven. And once they do, it should become apparent very quickly that they pulled a version that isn't supported on their environment. We might need to do some testing, but I think their code might not even compile or the tests won't run, due to incompatible class file format version.

@yurishkuro
Copy link
Member

To my memory, stagemonitor was indeed the only project that requested support for 6 in both OpenTracing and Jaeger. This may have been two years ago, when 6 might have been slightly more likely to be found in production than today. I agree that we could move on, but only to 7 as min, as 7 is still quite wildly used. However, I don't think 7 gives us any significant advantages.

@isaachier
Copy link
Contributor Author

At least it gives diamond operators (always thought that was Java 8). Anyway, stagemonitor claims to support Java 6 even today, so I agree it is probably a breaking change. Practically, it isn't a huge deal, just a matter of preference and why not if we can. I realize we can't until the next major version is released.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 10, 2018 via email

@yurishkuro
Copy link
Member

yurishkuro commented Aug 13, 2018

Requirement: Allow use of Java 8 and above in client. This would simplify some of the code.

As a general comment, I am -1 on this being a "requirement". Simplifying the client code (actually more like "pretty-fying") shouldn't come ahead of backwards compatibility and be the reason to break user apps.

According to Won's link, Java 7 is still running quite strong, so we cannot abandon it. And if we're not going to 8+, then going to 7+ does not seem to provide us with a lot of benefits. The main benefit to me would be default methods in interfaces in Java 8 (and even that more on the OpenTracing API side than in Jaeger).

image

@isaachier
Copy link
Contributor Author

I'm not opposed to maintaining an older version if the only sacrifice is complexity. However, more often than not, what ends up happening is that the entire ecosystem moves away from that support. We end up not being able to rely on libraries that drop support. The support can also come at the expense of performance penalties. I think Java 7 is a good compromise.

In Friday's meeting @jpkrohling and I seemed to agree that since we are < 1.0.0 we can still make backwards-incompatible changes when necessary. @jpkrohling suggested upgrading to Java 8 and seeing what response we get. He has a number of good points, especially about the users of EOL versions tending to use old versions of Jaeger as well, so there is no risk of anything breaking them.

@yurishkuro
Copy link
Member

So let's compare:

  • pros: we can type a few characters less in the code
  • cons: 30% of potential users cannot use our software (data as of 2017, but it's certainly not 0% today)

@yurishkuro
Copy link
Member

not being able to rely on libraries that drop support

for example?

@yurishkuro
Copy link
Member

btw, regarding things discussed on the call - the highlights from the discussion should be documented, but given how few interested parties are usually present on the call the final decisions should be made publicly on the github issues, either via consensus or majority vote.

@isaachier
Copy link
Contributor Author

My main counterargument to these Java 6/7 support debates is where do we draw the line? I'm fine if the answer is consistent. For example, why don't we support Java 5, or 4, or even 1? Someone out there is using it!

Java 7 is a reasonable lower bound from my point of view. I just agree with @jpkrohling. Fundamentally, it is easier to ask for forgiveness than permission (as we all know from Python). Sending out a survey won't help, but breaking the code in the newest release would help us identify who out there needs Java 7.

@isaachier
Copy link
Contributor Author

And yes, next time we ought to document it.

@isaachier
Copy link
Contributor Author

Last comment for now, in terms of libraries, I don't see any issues yet. Luckily, gRPC still supports even Java 6.

@jpkrohling
Copy link
Collaborator

To recap my arguments from the meeting (and earlier in this thread) and better document them:

  1. To me, the main point is agreeing on a policy. Both Java 6 and 7 are EOL for years now (like 5, 4, ...). Except for a couple of vendors charging very high sums, nobody has a supported Java 6/7 JVM, meaning that people running them are running non-secure and non-patched software. In the past, library vendors were supporting an EOL Java 6 because of Android, but that's not the case anymore;
  2. Linked to the above: Java versioning is changing to have more frequent releases, with a few LTS releases in between. Java 8 is one such LTS release. IMO, a good policy would be "we support whatever LTS versions are currently supported";
  3. 30% on Java 6+7 was 15 months ago, according to that blog post. We don't have recent numbers, but it's then at most 30%, potentially far lower;
  4. Gut feeling: people who are using older JVMs tend to stick with older versions of software, so, people will not jump into the latest Jaeger client. They'd use whatever they have right now and never change it (unless they get hit by a bug).
  5. As long as we don't break client -> [agent|collector] compatibility, people can just use an older version of the client. We can keep a branch around for 0.30.x, so that interested parties can backport fixes that they need. Louis-Étienne mentioned that he'd be willing to work with this branch and backport bug fixes that are of interest to him.

On the technical side, we don't have many reasons to move. Our code base isn't that big, so, nothing will really change our lives here. Things that are nice, though: some performance improvements, especially around iterators/looping (by using streams), date/time API, lambdas, ...

With all that, the suggestion is then to bump our build to use Java 8 and release the next minor version (v0.31.0) but not use any Java 7+ specific features for a few months. If we don't hear any complaints by Jan 2019, we can start making use of the new features. Otherwise, we just rollback this change and release new minor (v0.32.0).

@yurishkuro
Copy link
Member

Louis-Étienne (@ledor473) - you work for Ticketmaster, correct? This isn't a vendor charging high sums, it's an end user. And I am sure there is plenty of enterprise software out there that is not yet running on Java 8.

Basically, my point is that if we go to Java 8+, we cut off some (unknown, but still sizable) user base. If we only go to Java 7, it doesn't give us a lot of benefits over Java 6, so why bother.

Btw, Zipkin's Brave is on Java 7+.

@isaachier
Copy link
Contributor Author

Actually I like @jpkrohling's idea of using Java 8 for build purposes, but not any of the 7+ features until enough time has passed to guarantee that the vast majority of potential users have already upgraded unharmed.

@isaachier
Copy link
Contributor Author

Also, still would love to hear response to this:

why don't we support Java 5, or 4, or even 1?

@jpkrohling
Copy link
Collaborator

If we only go to Java 7, it doesn't give us a lot of benefits over Java 6, so why bother.

Until when are we going to support Java 6/7?

@ledor473
Copy link
Member

Yes @yurishkuro I'm working at Ticketmaster.
We do have applications that still use Java 6/7. I wouldn't say Java 6 represent a large footprint, but Java 7 is still present. And yes, some of theses applications are instrumented with OT/Jaeger!

I have no doubt that we are not the only one in that situation as Oracle itself still supports Java 6 until Dec 2018 with their Extended Support.

Just to put things in perspective, anything written 4 years ago would have been done in Java 7.

While I'd prefer to see Java 6/7 support, I do understand that it's not ideal to build new features and it makes the development harder. Also, there seems to be a shift in the Java community to support Java 8+. For example, Spring 5 requires it.

I know that eventually Java 6 and 7 support will be dropped (even Java 8 will be old soon), but when that day come I still think that the Jaeger architecture will help make this transition smoother.
As @jpkrohling pointed out, keeping the Client -> Agent compatibility will allow older applications to use an older version of the SDK.
If there's ever an incompatiblity, we could still run an older Agent for as long as there's no incompatiblity at the Agent -> Collector layer.
Ideally we would document that just like Spring documents the JDK support for 4.3 vs 5.x.

@yurishkuro
Copy link
Member

Oracle itself still supports Java 6 until Dec 2018 with their Extended Support

Maybe this is a reasonable deadline for us to set as well for dropping Java 6. Still don't know about Java 7.

@ledor473
Copy link
Member

Maybe Java 7 could be dropped at the same time if an "end to end testing" is done (in a separate CI project maybe?) to ensure a minimal compatibility with recent Agent/Collector?

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Aug 15, 2018 via email

codefromthecrypt pushed a commit to openzipkin/brave that referenced this issue Aug 15, 2018
This is a response to the following issue, where our dependency policy
was unclear: jaegertracing/jaeger-client-java#511 (comment)
@jpkrohling
Copy link
Collaborator

@yurishkuro wrote:

This isn't a vendor charging high sums, it's an end user

I meant that there are only a couple of vendors providing support for the JVM. If @ledor473's company isn't paying one of those vendors, then they are running a non-supported JVM, potentially with non-patched security vulnerabilities.

@ledor473 wrote:

Oracle itself still supports Java 6 until Dec 2018 with their Extended Support.

That's one of the vendors that I mentioned before charging high sums of money to offer commercial support :-) I might be wrong here, but from what I understand, only paying customers are receive this support. Everyone else is running non-patched JVMs.

On the same page, we can see that Java 7 is on the same boat, but one layer before: "premier support", but also means that only paying customers are receiving this support.

Maybe Java 7 could be dropped at the same time if an "end to end testing" is done

I'd still do it in phases: release only Java 8 artifacts from now on (or from Jan 2019) and get people aware that things are changing. Then, 6 months later, we are free to start adopting features available in new versions.

But more importantly, I think we should decide on a policy, especially given that few JVMs from now on will be LTS. For instance, Java 10 was released in March this year and it stops receiving updates in about one month, when Java 11 is released.

@yurishkuro
Copy link
Member

I rather like Zipkin Brave's policy: support whichever minimal version is allowed by the frameworks that Brave instruments explicitly (with 1.6 as the lower bound). And from this PR openzipkin/brave#757 there are a lot of popular frameworks that still support 1.6.

@jpkrohling
Copy link
Collaborator

support whichever minimal version is allowed by the frameworks that Brave instruments explicitly

The suggestion is then to base our policy on whatever version OpenTracing Java sets as base? I'm fine with that.

@jpkrohling
Copy link
Collaborator

I do not want to reopen this debate, but wanted to leave this here for reference:

https://snyk.io/blog/jvm-ecosystem-report-2018

87% is on Java 8 or more recent, 9% on Java 7, 3% are on Java 6 or lower and 1% don't know (sample size > 10,200).

@pavolloffay
Copy link
Member

Even java 11 is out (LTS support after java 8)

@jkwatson
Copy link

jkwatson commented Feb 1, 2021

Does jaeger even work with java 6 any more? I can't use the thrift library for java 6, as it depends on okhttp which doesn't work with java 6. Is there a known configuration that will work for jaeger, the thrift protocol and java 6?

@yurishkuro
Copy link
Member

yurishkuro commented Feb 1, 2021

ha, what a walk down the memory lane this ticket is...

According to https://snyk.io/blog/developers-dont-want-to-leave-java-8-as-64-hold-firm-on-their-preferred-release/, 3% of users are on Java 7 or lower. I think at this point we should be ok to go to 8, and let the holdouts use the older SDK versions (plus, we're not really building much into Java SDK these days, so they won't be missing a lot).

@william-tran
Copy link

I have concurrency improvements in an internal fork that are less than trivial to backport to java 6, see #609 (comment). Would be great to see java 8 the baseline.

@yurishkuro
Copy link
Member

I am tagging this as ready to implement, need a PR to upgrade all build files to 1.8

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.