Fix Groovy OpenTelemetrySdk builder loading by ADITYA-CODE-SOURCE · Pull Request #8407 · open-telemetry/opentelemetry-java

ADITYA-CODE-SOURCE · 2026-05-14T16:41:35Z

This avoids eager Groovy linkage of incubator-dependent SDK code by resolving IncubatingUtil.createExtendedOpenTelemetrySdk(...) reflectively in OpenTelemetrySdkBuilder instead of calling it directly.

Also adds a Groovy regression test covering OpenTelemetrySdk.builder().build() without incubator classes on the runtime classpath.

psx95 · 2026-05-14T20:11:38Z

  }

+  @Test
+  void builder_fromGroovyWithoutIncubator() throws Exception {


Nit: Could rename the test to buildFromGroovyWithoutIncubator

Done in — the test is now named .

psx95 · 2026-05-14T21:32:46Z

+      if (cause instanceof RuntimeException) {
+        throw (RuntimeException) cause;
+      }
+      if (cause instanceof Error) {
+        throw (Error) cause;
+      }


Can you add some comments justifying the reasons for this handling?

Added a comment in commit d5ffc54 explaining that reflection wraps the underlying incubator-path failure in InvocationTargetException, and that we rethrow RuntimeException and Error directly to preserve the original behavior.

psx95 · 2026-05-14T21:39:43Z

+    try {
+      return (OpenTelemetrySdk)
+          requireNonNull(CREATE_EXTENDED_OPEN_TELEMETRY_SDK_METHOD)
+              .invoke(null, openTelemetrySdk, configProvider);


Nit: Looking at the javadoc for invoke, it throws 3 checked exceptions:

IllegalAccessException

IllegalArgumentException

InvocationTargetException

Might be good to add handling for IllegalArgumentException (although it is less likely since arguments are controlled).

Good catch. I added explicit IllegalArgumentException handling in commit d5ffc54 as well.

psx95 · 2026-05-14T21:43:31Z

+   * io.opentelemetry.sdk.internal.SdkConfigProvider)}.
   */
-  OpenTelemetrySdkBuilder setConfigProvider(SdkConfigProvider configProvider) {
+  OpenTelemetrySdkBuilder setConfigProvider(Object configProvider) {


Question: Why was this changed to Object?

I changed it to Object so the reflective path in OpenTelemetrySdkBuilder no longer depends on a narrower incubator-linked method signature while Groovy is eagerly inspecting the builder. The actual object passed is still the same internal config provider; this just keeps the optional incubator path behind reflection.

codecov · 2026-05-15T04:16:45Z

Codecov Report

❌ Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.88%. Comparing base (7b72998) to head (6479a05).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
.../io/opentelemetry/sdk/OpenTelemetrySdkBuilder.java	82.60%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #8407   +/-   ##
=========================================
  Coverage     90.88%   90.88%           
- Complexity     7985     7989    +4     
=========================================
  Files           898      898           
  Lines         24107    24129   +22     
  Branches       2406     2408    +2     
=========================================
+ Hits          21909    21930   +21     
- Misses         1455     1458    +3     
+ Partials        743      741    -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ADITYA-CODE-SOURCE · 2026-05-15T04:35:13Z

Follow-up: I added focused coverage for the reflective error-handling branches in OpenTelemetrySdkBuilder (runtime exception, error, checked exception, illegal argument, and illegal access paths) and pushed that in commit aa2fdc1. This should address the remaining patch coverage gap from Codecov.

ADITYA-CODE-SOURCE · 2026-05-15T06:39:47Z

Follow-up: the broad CI failures were coming from an Error Prone overload-order warning on the new helper in OpenTelemetrySdkBuilder, which was then treated as an error under the normal build settings. I aligned the overload parameter order and updated the new branch-coverage tests accordingly in commit e0d4b87.

psx95 · 2026-05-15T15:13:21Z

You need to run spotlessApply to fix some of the failures.

jack-berg · 2026-05-18T17:22:30Z

Hey I want to take a step back on this. This PR represents a precedent that we're going to jump through additional hoops to support groovy.

We don't say anything about groovy or other JVM based languages in our versioning policy. And its not that a don't want to support these. But I don't understand what supporting entails. Groovy for example, has different class loading semantics that has runs into issues with the patterns we use to activate different capabilities based on whether or not certain dependencies are present. For example, the metrics, logs, and trace SDKs all behave differently based on whether opentelemetry-api-incubator is present, using techniques like this to detect if the module is present, and helps like this that interact with the standard java classloader in such a way that the incubating classes are only loaded after we verify they are present.

I know the techniques we need to use to make these patterns work with the java class loader. I don't the ways in which groovy class loading is different. I do know that there are differences that are impactful though.

Before establishing a precedent about groovy, I want more analysis to be done about those differences and what it means for the patterns we employ.

I know that its a bigger scope than just solving an issue that appears to be self contained. But some seemingly simple issues have a big can of worms hiding under the surface and this is one of them.

Groovy users can always get around these issues by including the compile only dependencies on their classpath.

ADITYA-CODE-SOURCE · 2026-05-18T23:15:59Z

Hey I want to take a step back on this. This PR represents a precedent that we're going to jump through additional hoops to support groovy.

We don't say anything about groovy or other JVM based languages in our versioning policy. And its not that a don't want to support these. But I don't understand what supporting entails. Groovy for example, has different class loading semantics that has runs into issues with the patterns we use to activate different capabilities based on whether or not certain dependencies are present. For example, the metrics, logs, and trace SDKs all behave differently based on whether opentelemetry-api-incubator is present, using techniques like this to detect if the module is present, and helps like this that interact with the standard java classloader in such a way that the incubating classes are only loaded after we verify they are present.

I know the techniques we need to use to make these patterns work with the java class loader. I don't the ways in which groovy class loading is different. I do know that there are differences that are impactful though.

Before establishing a precedent about groovy, I want more analysis to be done about those differences and what it means for the patterns we employ.

I know that its a bigger scope than just solving an issue that appears to be self contained. But some seemingly simple issues have a big can of worms hiding under the surface and this is one of them.

Groovy users can always get around these issues by including the compile only dependencies on their classpath.

@jack-berg That’s a fair point. I’m happy to close this PR if you think the compile-only dependency workaround is sufficient for now, and that any broader JVM-language compatibility discussion should happen separately.

laurit · 2026-05-19T06:10:16Z

Did you try using @CompileStatic? As far as I understand the only relevant change is in the signature of setConfigProvider. Is calling IncubatingUtil via reflection necessary?

Fix Groovy OpenTelemetrySdk builder loading

18d8762

ADITYA-CODE-SOURCE requested a review from a team as a code owner May 14, 2026 16:41

ADITYA-CODE-SOURCE mentioned this pull request May 14, 2026

Can't build OpenTelemetrySdk in groovy #8198

Open

github-actions Bot mentioned this pull request May 14, 2026

Pull Request Dashboard #8375

Open

Fix Groovy builder test formatting

d0880e4

psx95 reviewed May 14, 2026

View reviewed changes

Refine Groovy builder fix comments

d5ffc54

Add Groovy builder branch coverage

aa2fdc1

Fix Groovy builder helper overload

e0d4b87

ADITYA-CODE-SOURCE added 2 commits May 17, 2026 21:01

Fix Groovy builder spotless formatting

db1c7ae

Remove reflective access branch test

6479a05

Conversation

ADITYA-CODE-SOURCE commented May 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ADITYA-CODE-SOURCE commented May 15, 2026

Uh oh!

ADITYA-CODE-SOURCE commented May 15, 2026

Uh oh!

psx95 commented May 15, 2026

Uh oh!

jack-berg commented May 18, 2026

Uh oh!

ADITYA-CODE-SOURCE commented May 18, 2026

Uh oh!

laurit commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented May 15, 2026 •

edited

Loading