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

Implement query hooks (middleware) #69

Closed
howesteve opened this issue May 30, 2020 · 7 comments
Closed

Implement query hooks (middleware) #69

howesteve opened this issue May 30, 2020 · 7 comments
Labels
enhancement New feature or request

Comments

@howesteve
Copy link

howesteve commented May 30, 2020

What would you like to be added:
Custom logic hooks before a query is run (i.e. middleware)

Why is this needed:
By implementing this, a number of features could be implemented, specially in embedded mode.

  1. Middlewares of all sorts, whose would allow a richer echosystem. With a richer echosystem, the core server wouldn't have to implement so many features (telemetry, logging, redis support, rate limiting, etc.), popularity of the project would raise and future features would be easier to implement.
  2. Data validation in the application server, which is hard to implement currently. Hasura and Postgraphile recommend to do it on the server, but it's very slow, inconvenient and using a terrible language (pl/pgsql). Validation is restrained to database language and environment.
  3. Other forms of authentication (i.e. server id, ldap, ip based, etc.), authorization, logging, metrics, etc. etc.
  4. Query inspecting and rewriting.
  5. Standalone server loadable golang plugins implementing middleware.
  6. What hasura calls "Actions" (custom business logic via webhooks), but it's slow, inconvenient and adds considerable network traffic. In supergraph, this could all be implemented internally in the server, while still supporting webhooks if wanted.

I think this could be another killer feature for supergraph, currently unmatched in other servers, and opening a wide range of features and collaboration.

How could this be implemented?
I suggest using this api, which is extensible in the future, and wouldn't break existing programs:

type QueryOptions struct {
  UserId string
  UserIdProvider string
  // ... (other query/session variables)
  BeforeQuery func(*Ast) error 
  // BeforeRunSql(ast *Ast, sql string)
  // OnMutation()
  // Future, when subscriptions are  #ready:
  //  OnResultsetChange()  (*supergraph.Result, error)
  // ... etc.
} struct 
func supergraph.GraphQL(c context.Context, query string, vars json.RawMessage, options...QueryOptions) (*supergraph.Result, error)

Currently on hasura they implement actions by calling webhooks by sending a struct {type, definition. handler, kind}.

References:
#17
#26
https://discord.com/channels/628796009539043348/628796009539043350/716096650443096164

@howesteve howesteve added the enhancement New feature or request label May 30, 2020
@gonzaloserrano
Copy link
Contributor

In case you want some inspiration from similar abstractions gqlgen (we use it at our company to generate the graphql code): https://github.com/99designs/gqlgen/blob/master/graphql/handler.go

E.g we have an observability middleware that records the time before executing the decorated operation (query or mutation), then after and the substraction is the latency of the operation and we record as a gauge metric for prometheus, with a metric tag being the name of the operation. This way we can know how long an operation is in the graphql layer.

We have another one to filter all non-login queries/mutations so we do auth, etc.

@dosco
Copy link
Owner

dosco commented May 31, 2020

I'm not against adding more functionality. I'm just careful about adding new public APIs and first try to understand the use-case they will satisfy that cannot be done currently.

  1. When using as a library you have full control of the GraphQL and the JSON that you pass into the GraphQL function so why the need to access an AST.

  2. There is no AST in production mode since all queries from the allow.list file are precompiled into SQL prepared statements on start so essentially every request goes directly to database which makes things really fast.

  3. In fact the whole Super Graph standalone service is implemented by importing the Super Graph library. Everything is implemented using the public exposed Graphql() function. For example metrics was done using opencensus sql.DB wrapper and passing that in as the db.

@dosco
Copy link
Owner

dosco commented May 31, 2020

  • Middlewares of all sorts, whose would allow a richer echosystem. With a richer echosystem, the core server wouldn't have to implement so many features (telemetry, logging, redis support, rate limiting, etc.), popularity of the project would raise and future features would be easier to implement.

Core is not a server github.com/dosco/super-graph/core is the library it exposes one primary function as of now GraphQL(context, query, variables). The standalone Super Graph service github.com/dosco/super-graph is a different package that uses this public API. Anyone can build this similar service they'll just have to handle all the defaults this does.

Also I'll be exposing the service itself as a set of public APIs so people can integrate the entire service into their app. I see Hooks as a valuable thing in this case where you can then use them to hook into various parts of the request lifecycle inside the service.

  • Data validation in the application server, which is hard to implement currently. Hasura and Postgraphile recommend to do it on the server, but it's very slow, inconvenient and using a terrible language (pl/pgsql). Validation is restrained to database language and environment.

You can do this right now in your own code.

func httpHandler(w, r) {
  ...read in json
  ...varlidate json
  ...call core.GraphQL(context, query, validated_json)
  ...do something with outputted json
  .. return it to user
}
  • Other forms of authentication (i.e. server id, ldap, ip based, etc.), authorization, logging, metrics, etc. etc.
  • Query inspecting and rewriting.
  • Standalone server loadable golang plugins implementing middleware.

Same as the above. it's your own http service.

  • What hasura calls "Actions" (custom business logic via webhooks), but it's slow, inconvenient and adds considerable network traffic. In supergraph, this could all be implemented internally in the server, while still supporting webhooks if wanted.

I sense the confusion here arises from the fact that Super Graph is a GO library and a standalone service and I feel maybe you are talking about adding hooks to the service and not the library. So you can leverage everything the service does and add more? Correct me if this is not what you're thinking.

@howesteve
Copy link
Author

Yes all core functionality is exported via core.GraphQL(context, query, validated_json), but how are we going to validate the json data (or even modify it) if we don't know if it is a query or a mutation, and what are the subjects? Typically we'll have a single api entry point (e.g. /avi/v1/graphql) and it will receive all queries and their vars. But vars are query dependent.

I may have exaggerated when I said "ast"; I meant have to known something about the graphql query to allow proper validation and middleware.
Put in other words, when my http handler receives a query:

mutation {
  product(insert: $data) {
    id
    name
  }
}

how do I know I will be validating a product instead of an user?

mutation {
  users(insert: $data) {
    id
    name
    email
  }
}

@dosco
Copy link
Owner

dosco commented Jun 1, 2020

We have two other exported functions that could help here. Name(query) and Operation(query) these are very fast zero allocation functions it extract the name and operation type from the GraphQL query. They are meant to be used instead of a full parse.

https://pkg.go.dev/github.com/dosco/super-graph/core?tab=doc#Name

https://pkg.go.dev/github.com/dosco/super-graph/core?tab=doc#Operation

@dosco
Copy link
Owner

dosco commented Jun 1, 2020

The one issue I do see here is that if you manipulate the incoming json then you have to marshal it back into json before passing it to GraphQL this is a minor issue for most people but we could look into optimizing out this extra step by someone taking in a map or struct.

@howesteve
Copy link
Author

Woah, sorry I forgot to answer this one. The exported functions are indeed of great help! And I agree the marshalling is a minor issue.

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

No branches or pull requests

3 participants