Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rationale for not supporting Java 7 #1068

Merged
merged 7 commits into from
Aug 22, 2020
Merged

Rationale for not supporting Java 7 #1068

merged 7 commits into from
Aug 22, 2020

Conversation

trask
Copy link
Member

@trask trask commented Aug 20, 2020

Summarizing decision and rationale from yesterday's meeting on this topic. Please review and provide feedback.

docs/java-7-rationale.md Show resolved Hide resolved
docs/java-7-rationale.md Outdated Show resolved Hide resolved
Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
@codefromthecrypt
Copy link

rationale looks to cover the reasons why. if you want to trump our RATIONALEs, add what you are willing to not do.

ex. "modern" is fair enough, but this concedes implicitly that open-telemetry cannot be used for things not modern, so something else will.

Maybe @CodingFabian or others know the fuller impact of how many sites from an existing vendor require Java 1.7 instrumentation.

@CodingFabian
Copy link

Only commenting since @adriancole pinged me.
From a vendor perspective this makes me happy. We have 1.6 / 1.7 customers and we happily support them with our auto instrumentation.
From a technologist perspective, I would say that not being able to test against 1.7 is just an excuse. It is possible (as a vendor I do that!). One might argue that this is too much effort, and this could be true depending on many factors. I can not, and do not want to judge this.
The true question is what Java 8 library provided class is worth dropping the support. I doubt that any bytecode level or runtime behaviour differences would justify the decision. But I assume this has been clarified in the meeting.

Is that the input you were looking for, Adrian?

@codefromthecrypt
Copy link

Is that the input you were looking for, Adrian?

it seems github doesn't have a 🍿 reaction, so I need to make a comment instead

@trask
Copy link
Member Author

trask commented Aug 20, 2020

@adriancole @CodingFabian appreciate the feedback!

how many sites from an existing vendor require Java 1.7 instrumentation.

Oh yes, I will add this to the doc. We know from New Relic's (very large) user base that 2.5% of apps are still on Java 7:

https://blog.newrelic.com/technology/state-of-java/

From a vendor perspective this makes me happy. We have 1.6 / 1.7 customers and we happily support them with our auto instrumentation.

Yup, we actually discussed this, and it makes us feel less bad about not supporting Java 7 users, knowing that those users have other options for codeless monitoring. I will add this to the doc also.

I would say that not being able to test against 1.7 is just an excuse

I'm not following this, are you saying it's ok to --release 7 and only test against Java 8? Or are you saying we should stick to JUnit 4 and not use Testcontainers? EDIT: or probably you mean something else like running our test code inside a separate JVM from where we run our test harness / verification?

@CodingFabian
Copy link

I am not proposing anything for OT. I just want to say that Instana runs Java 1.6 tests, and they do include testcontainers based tests. So I guess it really just depends on the effort you want to invest.

Are you saying NewRelic will recommend Instana? quite an honor.

@trask
Copy link
Member Author

trask commented Aug 20, 2020

Are you saying NewRelic will recommend Instana? quite an honor.

You would have to ask them, but I doubt it 😁

@jkwatson
Copy link
Contributor

New Relic's own non-OpenTelemetry agent support java 7, so no. :)

@trask
Copy link
Member Author

trask commented Aug 20, 2020

Instana runs Java 1.6 tests, and they do include testcontainers based tests. So I guess it really just depends on the effort you want to invest.

Yes, good point, I will add this option to the doc.

@codefromthecrypt
Copy link

codefromthecrypt commented Aug 21, 2020 via email

@trask
Copy link
Member Author

trask commented Aug 21, 2020

updated the rationale doc based on feedback

@trask trask merged commit 87170b1 into open-telemetry:master Aug 22, 2020
@trask trask deleted the java-7-rationale branch August 22, 2020 00:42
Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @trask! And thanks everyone for the feedback! I don't think there's any perfect answer here and we'll continue to digest this, we still have until GA to commit to a decision either way. Hoping it makes people happy :)

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

Successfully merging this pull request may close these issues.

7 participants