The graphql-java instrumentation is going to break in the next graphl-java release

[bbakerman]

Is your feature request related to a problem? Please describe.

The next version of graphql-java is very likely to remove deprecated callbacks in graphql.execution.instrumentation.Instrumentation

The current code uses them

public InstrumentationContext<ExecutionResult> beginExecution(
      InstrumentationExecutionParameters parameters) {

For performance reasons these got changed to a form that means the state is passed directly in.

The current https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/graphql-java-12.0/library code is based on a very old version of grpahql-jaava

V12 was released in March 2019 - 5 years ago

The next version will be graphql-java V22

V20 is the first version has the alternative methods in it and was the version that deprecated the older methods.

Describe the solution you'd like

I think you should create a graphql-java-20.0 library - it can share almost all of the same code as the current one exception the method signature of the graphql.execution.instrumentation.Instrumentation callbacks will have changed

Describe alternatives you've considered

You could create a common module for the common code (since most of it will work as is) and then just have different graphql.execution.instrumentation.Instrumentation shells.

Additional context

No response

[steverao]

Do you mean the instrumentation is alright, but for future version of graphql, we need to reconstruct the codes?

[bbakerman]

We have confusing terms because the graphql callback system is called instrumentation and so its the OTel code.

What I mean is that the graphql.execution.instrumentation.Instrumentation methods implemented here will be removed and the code will not run on v22 once it comes out.

Your internal OTel inside the graphql.execution.instrumentation.Instrumentation callbacks is all ok. so to support versions 20+ - use the new non deprecated graphql.execution.instrumentation.Instrumentation methods and you will be ok.

[bbakerman]

For example old code

    @Deprecated
    @DeprecatedAt("2022-07-26")
    @NotNull
    default InstrumentationContext<ExecutionResult> beginExecution(InstrumentationExecutionParameters parameters) {
        return noOp();
    }

And you should use this version of the method

    @Nullable
    default InstrumentationContext<ExecutionResult> beginExecution(InstrumentationExecutionParameters parameters, InstrumentationState state) {
        return beginExecution(parameters.withNewState(state));
    }

These new methods turned up in v20 and the deprecated methods will be removed in the upcoming v22.

So if you make a new v20+ version of your code with the new non deprecated methods - it will work post v22 and beyond.

if you do not, then your code wont run on a system with graphql-java v22

[laurit]

@bbakerman thanks for reporting. We run tests against the earliest supported version and the latest available so we catch such breakages when a new version is released.

I think you should create a graphql-java-20.0 library - it can share almost all of the same code as the current one exception the method signature of the graphql.execution.instrumentation.Instrumentation callbacks will have changed

Sounds reasonable.