-
Notifications
You must be signed in to change notification settings - Fork 66
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
GraphQL Interceptor Execution Logic Improvement #4199
Comments
Interesting, I see it a bit differently. I wonder if we really need to change the behaviour on the basis of having resource level interceptor. In my head, it was just an extension to interceptor array. For instance, if you want to log the execution time for reach resource function we can easily do it with current architecture, and if we have any other common thing, we can still do it. But with the proposed model we have to add it each and every resource if I am not mistaken. |
ConclusionHad an offline discussion with @shafreenAnfar and @DimuthuMadushan The following are points discussed:
Semantically, there are two different types of interceptors.
Type Interceptors (Service-Level Interceptors)The current interceptor is somewhat similar to a type interceptor. This means the interceptor can be applied to a type. Although the current implementation only supports adding an interceptor to the root import ballerina/graphql;
import ballerina/log;
readonly service class LogInterceptor {
*graphql:Interceptor;
isolated remote function execute(graphql:Context context, graphql:Field 'field) returns anydata|error {
log:printInfo(string `Field "${fieldName}" execution started!`);
var data = context.resolve('field);
log:printInfo(string `Field "${fieldName}" execution completed!`);
return data;
}
}
service on new graphql:Listener(9090) {
resource function get profile() returns Profile {
return new ("John Doe", 30);
}
}
@graphql:ServiceConfig {
interceptors: [new LogInterceptor()]
}
service class Profile {
private final string name;
private final int age;
function init(string name, int age) {
self.name = name;
self.age = age;
}
resource function get name() returns string {
return self.name;
}
resource function get age() returns int {
return self.age;
}
} In the above code, the interceptor is applied to the Field Interceptors (Resource-Level Interceptors)Field interceptors are applied to a certain field of an object in the GraphQL schema (with the limitation of the field must be represented using a Consider the following code: import ballerina/graphql;
import ballerina/log;
readonly service class LogInterceptor {
*graphql:Interceptor;
isolated remote function execute(graphql:Context context, graphql:Field 'field) returns anydata|error {
log:printInfo(string `Field "${fieldName}" execution started!`);
var data = context.resolve('field);
log:printInfo(string `Field "${fieldName}" execution completed!`);
return data;
}
}
service on new graphql:Listener(9090) {
@graphql:ResourceConfig {
interceptors: [new LogInterceptor()]
}
resource function get greeting() returns string {
return "Hello, World!";
}
resource function get profile() returns Profile {
return new ("John Doe", 30);
}
}
service class Profile {
private final string name;
private final int age;
function init(string name, int age) {
self.name = name;
self.age = age;
}
@graphql:ResourceConfig {
interceptors: [new LogInterceptor()]
}
resource function get name() returns string {
return self.name;
}
resource function get age() returns int {
return self.age;
}
} In the above example, the interceptor is applied to the This answers the first part of the question where the type interceptors (service-level interceptors) are applied globally (meaning the interceptor is applied to all the fields and the subfields of the type), while the field interceptors (resource-level interceptors) are applied locally (meaning the interceptor is applied only for the given field). But if a developer needs to apply an interceptor to the entry point, there is no way of doing this using the above-mentioned methods. Defining Entry Point InterceptorsThere are 2 approaches to doing this: Using Different Types of InterceptorsIn this approach, we can introduce three types of interceptors: Service InterceptorsThis works as same as the current interceptor implementation. Can be applied to the service and it will be applied to all the fields. Resource InterceptorsThis is as same as the one proposed in the resource-level interceptors. Can be applied to one or more fields in the GraphQL schema. Listener InterceptorThis can be applied at the listener level, meaning the interceptor will be applied to each request at the interceptor level. This is similar to the HTTP listener interceptors. Implementation of this might be tricky given that this should handle all the requests coming through the listener, and also adhere to the onion principle followed by the GraphQL interceptors. We might be able to avoid the breaking change by introducing different names for resource-level interceptors and listener-level interceptors while keeping the current Using an Annotation for the InterceptorIn this approach, the interceptor will also have an annotation or a flag to state whether the interceptor is applied globally or locally. By default, an interceptor applied to a field will be local. But when applied to a type, it can be global (meaning the interceptor will be applied to each and every field and subfield), or local (meaning the interceptor will be applied to that particular type). Following is an example: @graphql:InterceptorConfig {
global: true // This can be changed based on the requirement.
}
readonly service class LogInterceptor {
*graphql:Interceptor;
isolated remote function execute(graphql:Context context, graphql:Field 'field) returns anydata|error {
log:printInfo(string `Field "${fieldName}" execution started!`);
var data = context.resolve('field);
log:printInfo(string `Field "${fieldName}" execution completed!`);
return data;
}
} With this approach, it is intuitive to think that the resource-level interceptors too would be applied globally (meaning the interceptor should be applied to the field, and the type of the field - similar to a type interceptor). In that case, we might have to implement that as well. This annotation may be replaced with a flag when creating the interceptor. We can avoid breaking the functionality of this by making the Going forward, I think we should finalize on an approach and implement that before releasing the resource-level interceptors because partial implementation would result in future breaking changes. @shafreenAnfar @DimuthuMadushan @MohamedSabthar Please add your thoughts and anything I missed. |
Meeting notes: The following decisions were taken:
|
@shafreenAnfar @ThisaruGuruge There is an issue with this approach. We cannot provide the |
I think we can park the listener interceptor for now. Our initial problem was to find a solution where a developer needs to apply an interceptor to an entry point of the GraphQL schema. This can be achieved using the service-level interceptors with the Therefore, I think the listener-level interceptor does not add more value, at least for now. The only usage I can think of is to handle each and every request that comes to the listener, but that's more of an HTTP thing than a GraphQL thing, and we shouldn't couple those functionalities with GraphQL. If someone needs to handle such cases, they can either use an HTTP listener to listen to all the requests and handle them, or they can use an HTTP gateway before the GraphQL service. We can think about introducing them later if a requirement arises. But in most cases, we can solve this with service and resource interceptors. |
Description:
Problem
Currently, the Ballerina GraphQL package has Service-Level Interceptor support.
In the current implementation, when a service-level interceptor is added to a service, it executes per each field of the GraphQL schema. Consider the following as an example:
In the above example, the interceptor is applied to all the fields, such as
Query.profile
,Profile.name
, andProfile.age
. Consider the following GraphQL document:For the above query, the server will log something like this (ignore the log format, just the functionality):
This was an okay approach back then since there was no way to intercept a field execution otherwise. But one drawback of this approach was that if a developer needed to intercept a field, they have to filter the field name inside the interceptor logic using string manipulation and execute the interceptor. This is not an optimal solution. We had seen this before and that's why we planned for the resource-level (or in GraphQL terms, field-level) interceptors.
Since now we are working on implementing the resource-level interceptors, applying the service-level interceptors to all the fields will become obsolete and it also does not mean anything anymore.
So the plan is to apply the service-level interceptors to the service level, which means any of the entry points of a GraphQL schema. This can be the
Query
type,Mutation
type, orSubscription
type. The interceptor will be applied to the root operation only. If an interceptor is needed to be applied for specific fields, it can be done using the resource-level interceptors.This change can be a breaking change for projects where interceptors are used to intercept fields.
Solution
Going forward, we can do the following:
With this release, we can:
@shafreenAnfar @DimuthuMadushan @MohamedSabthar WDYT?
The text was updated successfully, but these errors were encountered: