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

Add support for request tracing. #21

Closed
dosco opened this issue Dec 1, 2019 · 8 comments
Closed

Add support for request tracing. #21

dosco opened this issue Dec 1, 2019 · 8 comments
Labels
enhancement New feature or request

Comments

@dosco
Copy link
Owner

dosco commented Dec 1, 2019

What would you like to be added:

Integrate with a popular request tracing framework like Jaeger. There maybe other options I'm not super familiar. If possible request tracing should go all the way to the DB and Remote endpoints called for Remote Joins.

Why is this needed:

Observability is a big part of good infrastructure. This will allow people using Super Graph to automatically get insight into request flowing through our software.

@dosco dosco added the enhancement New feature or request label Dec 1, 2019
@howesteve
Copy link

howesteve commented Dec 5, 2019

That could be implemented internally using a hook on an event as well. Looks better to me then locking into a single implementation; the user could use anything he wants.
If you really want to use Jaeger, how about implementing it internally as events such "OnRequest()/BeforeQuery()/AfterQuery()/OnSession()" hook functions, that could be directly used on the embedded server, but allowing customizing externally as loadable plugins for the default server?
The same concept could be applied to api rating and many other functionalities users might want.

Howe

@dosco
Copy link
Owner Author

dosco commented Dec 7, 2019

I'm not familiar with Jaeger it was suggested to me by someone who uses it for request tracking across services on their K8s cluster. The hooks would work for those who want to embed Super Graph into their own app however the design of Super Graph does not lean towards the embedding option as of now. Super Graph is designed to be deployed as is and configured only through the config file.

I'm also not familiar with how loadable plugins can be implemented in a go app. Jaeger is almost an industry standard for tracing. I don't mind Super Graph being opinionated in this way.

@howesteve
Copy link

I see, I don't mind using Jaeger as well, but I think probably a more modular, event-based architecture would be more open? Allowing users to choose Opentracing, zipkin, etc.
I think the standalone server should just be an opinionated implementation of the embedded server.

Of course you know Hasura and agree it's a very nice piece of software; its pitfalls are exactly that it should be deployed, upgraded, run etc. in separate. Customization is hard. And it's haskell. None of these would happen with embedded supergraph.

Plugins in are straightforward enough in go: it's just runtime loadable functions. [1].

So I think implementation could be more or less like this.

  1. Move the server into it's own struct making it more modular and less coupled.
  2. Server implements some event registering functions:
    OnRequest()/BeforeQuery()/AfterQuery()/OnSession() etc.
  3. There should be a config option for loadable plugins (folders or individual files); something like:
plugin_folders:
    - /home/howe/supergraph_plugins1
    - /home/howe/supergraph_plugins2
plugins:
  -  /home/howe/supergraph_plugins1/jaeger.so
  -  /home/howe/supergraph_plugins1/memcached_sessions.so
  -  /home/howe/supergraph_plugins1/db_sessions.so
  1. Server loads plugins folder/individual files and tries to find the pre-defined exported symbols (OnRequest, BeforeQuery, etc.).
  2. Found plugins functions are hooked into the event system.
    func OnRequest(server Server, query string)

Note that instead of external plugins, it could be just be internal functions for the most common functionalities as well; it's much more decoupled and easier to contribute.

[1] https://medium.com/learning-the-go-programming-language/writing-modular-go-programs-with-plugins-ec46381ee1a9

@howesteve
Copy link

All of these are a great change to the codebase that I'm not sure if you're commited to make and I wonder if I should do it. Afraid of taking too much at once and perhaps you'd not like the new changes...

@dosco
Copy link
Owner Author

dosco commented Dec 10, 2019

I'm all for embedded Super Graph and technically it has always been on my roadmap. On an app I'm currently working on I have two services running on CloudRun one for authentication and creating the JWT token and the other is Super Graph. I've wanted to consolidate it and have Super Graph run an http handler instead of a whole separate app.

It would not even be a major change it could be done with relatively minimum code changes. I have a separate issue open #12 that will help move the code base in that direction. This could be done in multiple passes first just publish a package and a clean api for people to embed Super Graph into their own GO apps and then as a second pass add the hooks api.

@howesteve
Copy link

Sounds great to me. I'm not sure if you prefer to do it yourself, both passes. Perhaps you could fix #12 and then I mess around the events system? Btw the events system could make it easier to hook into other web frameworks such as echo, iris, internal go http, etc. so looks good in all aspects to me.
This project will for sure be super popular after embedding is possible...

@dosco
Copy link
Owner Author

dosco commented Dec 11, 2019

I've created a separate issue to track this idea #26. I prefer to guide and encourage others to work on meaty and interesting ideas within projects. Happy to jump in and add value wherever needed. I agree your description of the eventing system sounds cool. I can look into fixing #12 if that helps but I don't think it's a blocker and this work can go on in parallel.

@dosco
Copy link
Owner Author

dosco commented Apr 20, 2021

Done

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

2 participants