Limit result size and execution time in TransModel GraphQL API

[vpaturet]

Summary

This PR builds upon #5881 and provides a GraphQL instrumentation that limits both the result size and the execution time of a GraphQL query.

• the execution time is configured by the parameter apiProcessingTimeout in router-config.json.
• the result size is counted as the number of fields in the GraphQL result. The maximum value is configured by the parameter maxNumberOfResultFields in router-config.json (default value: 1 000 000)

This instrumentation is meant to replace both the OTPRequestTimeoutInstrumentation introduced in #5881 and the MaxQueryComplexityInstrumentation.

Note: MaxQueryComplexityInstrumentation does not check the runtime complexity of a query execution. It performs only a static analysis on the query text. By default it checks only the number of fields in the query text, which has limited value.

Note: the default max size is intentionally high and might be subsequently tuned in follow-up PRs.

Note: in the TransModel API, resolvers are run sequentially in the calling thread (that is: the web server thread). Thus the instrumentation object that is shared by all resolvers is accessed by only one thread and the use of an AtomicLong is actually not required.
However if this instrumentation were to be ported to another API that uses multi-threaded resolving (GTFS API?), the code should work out-of-the-box and concurrently abort the resolvers (see org.opentripplanner.apis.transmodel.support.AbortOnTimeoutExecutionStrategy)
This is however not tested and not in the scope of this PR.

Issue

No.

Unit tests

No.

Documentation

No.

[vpaturet]

@t2gran @leonardehrenfried I intend to use a router-config parameter instead of a header for parameterizing the result size. Do you know the reason why this is currently exposed as a header?

[codecov]

Codecov Report

Attention: Patch coverage is 28.00000% with 18 lines in your changes missing coverage. Please review.

Project coverage is 68.61%. Comparing base (ba67f17) to head (8e5873a).
Report is 161 commits behind head on dev-2.x.

Files	Patch %	Lines
...s/transmodel/MaxFieldsInResultInstrumentation.java	0.00%	14 Missing ⚠️
...entripplanner/apis/transmodel/TransmodelGraph.java	0.00%	2 Missing ⚠️
...opentripplanner/apis/transmodel/TransmodelAPI.java	0.00%	1 Missing ⚠️
...standalone/config/sandbox/TransmodelAPIConfig.java	87.50%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #5883      +/-   ##
=============================================
+ Coverage      68.48%   68.61%   +0.12%     
- Complexity     16703    16823     +120     
=============================================
  Files           1916     1927      +11     
  Lines          72660    72942     +282     
  Branches        7448     7476      +28     
=============================================
+ Hits           49762    50047     +285     
+ Misses         20333    20321      -12     
- Partials        2565     2574       +9     

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

[vpaturet]

@t2gran @leonardehrenfried I intend to use a router-config parameter instead of a header for parameterizing the result size. Do you know the reason why this is currently exposed as a header?

Discussed during the developer meeting: the intent of this header was to let a proxy/bff relax the limit on the GraphQL complexity. This proxy/bff is not in use anymore, thus the header can be safely removed.

[t2gran]

What should we do about the "since" field on the config? Should we remove it or enforce it?

[vpaturet]

I added anyway the version that was missing on the new parameter.

[leonardehrenfried]

What should we do about the "since" field on the config? Should we remove it or enforce it?

I think the current model where it's optional works quite well. I think having this information is often useful.

[t2gran]

Reply: We can discuss it on the next developer meeting. The goal was to get rid of the NA at some point. But, omitting the since tag have the same effect - except it is not visible any more. Having it as optional case people to forget it.