Skip to content

Commit

Permalink
Remove gRPC and gRPC gateway (#225)
Browse files Browse the repository at this point in the history 
* Since we are using the connect protocol and server we can remove gRPC
and gRPC-gateway support

* This means we no longer need to have a server listening for gRPC

* We also don't need to rely on the gRPC gateway to make the Agent
service accessible fromt the VSCodeExtension because that is now using
the connect protocol.

* We can also get rid of the RunMe proxy because the vscode extension is
now using the clients generated from the foyle buf schema registry in
buf.

Related to #173
  • Loading branch information
@jlewi
jlewi authored Sep 2, 2024
1 parent 335e74c commit affb5ee
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 407 deletions.
3 changes: 2 additions & 1 deletion app/pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ type ServerConfig struct {
HttpPort int `json:"httpPort" yaml:"httpPort"`

// GRPCPort is the port for the gRPC service
GRPCPort int `json:"grpcPort" yaml:"grpcPort"`
// Deprecated: GRPCPort is no longer used and should be removed in the next version of the config.
GRPCPort int `json:"grpcPort,omitempty" yaml:"grpcPort,omitempty"`

// CORS contains the CORS configuration
CORS *CorsConfig `json:"cors,omitempty" yaml:"cors,omitempty"`
Expand Down
71 changes: 0 additions & 71 deletions app/pkg/runme/proxy.go
View file

This file was deleted.

165 changes: 10 additions & 155 deletions app/pkg/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ import (

"github.com/cockroachdb/pebble"

"github.com/jlewi/foyle/app/pkg/runme"
aiv1alpha1 "github.com/stateful/runme/v3/pkg/api/gen/proto/go/runme/ai/v1alpha1"

"github.com/jlewi/foyle/app/pkg/eval"
"github.com/jlewi/foyle/protos/go/foyle/v1alpha1/v1alpha1connect"

Expand All @@ -42,37 +39,24 @@ import (
"connectrpc.com/otelconnect"
"github.com/gin-gonic/gin"
"github.com/go-logr/zapr"
"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
"github.com/jlewi/foyle/app/pkg/agent"
"github.com/jlewi/foyle/app/pkg/config"
"github.com/jlewi/foyle/app/pkg/executor"
"github.com/jlewi/foyle/app/pkg/logs"
"github.com/jlewi/foyle/protos/go/foyle/v1alpha1"
"github.com/pkg/errors"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"go.uber.org/zap"
"google.golang.org/grpc"
"google.golang.org/grpc/connectivity"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/health"
"google.golang.org/grpc/health/grpc_health_v1"
"google.golang.org/grpc/reflection"
)

// Server is the main application server for foyle
type Server struct {
config config.Config
engine *gin.Engine
grpcServer *grpc.Server
hServer *http.Server
config config.Config
engine *gin.Engine
hServer *http.Server
// builtinExtensionPaths is a list of serving paths to the built in extensions
builtinExtensionPaths []string

agent *agent.Agent
runmeProxy *runme.Proxy
executor *executor.Executor
conn *grpc.ClientConn
logsCrud *analyze.CrudHandler
shutdownComplete chan bool
}
Expand All @@ -87,21 +71,16 @@ func NewServer(config config.Config, blocksDB *pebble.DB, agent *agent.Agent, tr
if agent == nil {
return nil, errors.New("Agent is required")
}
runmeProxy, err := runme.NewProxy(agent)
if err != nil {
return nil, err
}

logsCrud, err := analyze.NewCrudHandler(config, blocksDB, tracesDB)
if err != nil {
return nil, err
}
s := &Server{
config: config,
executor: e,
agent: agent,
runmeProxy: runmeProxy,
logsCrud: logsCrud,
config: config,
executor: e,
agent: agent,
logsCrud: logsCrud,
}

if err := s.createGinEngine(); err != nil {
Expand All @@ -123,6 +102,7 @@ func (s *Server) createGinEngine() error {

router := gin.Default()

// TODO(jeremy): Should we turn this into a protobuf service and use connect?
router.GET("/healthz", s.healthCheck)
router.NoRoute(func(c *gin.Context) {
log.Info("Request for not found path", "path", c.Request.URL.Path)
Expand Down Expand Up @@ -417,29 +397,10 @@ func (s *Server) setHTMLTemplates(router *gin.Engine) error {

// Run starts the http server
func (s *Server) Run() error {
grpcAddress := fmt.Sprintf("%s:%d", s.config.Server.BindAddress, s.config.Server.GRPCPort)
grpcLis, err := net.Listen("tcp", grpcAddress)
if err != nil {
return errors.Wrapf(err, "failed to listen: %v", err)
}

s.shutdownComplete = make(chan bool, 1)
trapInterrupt(s)

log := zapr.NewLogger(zap.L())
go func() {
err := s.startGRPCServer(grpcLis)
if err != nil {
log.Error(err, "GRPC server exited")
// TODO(jeremy): Should come up with a better way to do a clean shutdown; i.e stopping the http server
os.Exit(1)
}

}()

if err := s.registerGRPCGatewayRoutes(); err != nil {
return err
}
address := fmt.Sprintf("%s:%d", s.config.Server.BindAddress, s.config.Server.HttpPort)
log.Info("Starting http server", "address", address)

Expand Down Expand Up @@ -482,12 +443,6 @@ func (s *Server) shutdown() {
log := zapr.NewLogger(zap.L())
log.Info("Shutting down the Foyle server")

// Shutdown the grpc server
if s.grpcServer != nil {
s.grpcServer.GracefulStop()
log.Info("GRPC Server shutdown complete")
}

if s.hServer != nil {
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()
Expand All @@ -500,100 +455,6 @@ func (s *Server) shutdown() {
s.shutdownComplete <- true
}

// startGRPCServer starts the grpc server.
// Taking the listener as an argument lets us create tests that inject a listener suitable for tests
func (s *Server) startGRPCServer(lis net.Listener) error {
log := zapr.NewLogger(zap.L())

s.grpcServer = grpc.NewServer(grpc.StatsHandler(otelgrpc.NewServerHandler()))

v1alpha1.RegisterExecuteServiceServer(s.grpcServer, s.executor)
v1alpha1.RegisterGenerateServiceServer(s.grpcServer, s.agent)
aiv1alpha1.RegisterAIServiceServer(s.grpcServer, s.runmeProxy)

// So that gRPC curl can be used to inspect it
reflection.Register(s.grpcServer)

// Support health checks
grpc_health_v1.RegisterHealthServer(s.grpcServer, health.NewServer())

log.Info("Starting grpc service", "address", lis.Addr())
return s.grpcServer.Serve(lis)
}

// registerGRPCGateway starts the gRPC gateway which provides a REST proxy to the grpc server.
func (s *Server) registerGRPCGatewayRoutes() error {
// TODO(jeremy): I think we could use a ctx with Cancel and then potentially trigger cancel to shutdown the
// connection.
ctx := context.Background()

// Create a connection to the gRPC server
opts := []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())}

log := zapr.NewLogger(zap.L())

grpcServerEndpoint := fmt.Sprintf("%s:%d", s.config.Server.BindAddress, s.config.Server.GRPCPort)
log.Info("Dialing grpc server", "endpoint", grpcServerEndpoint)
conn, err := grpc.NewClient(grpcServerEndpoint, opts...)
if err != nil {
return err
}
go func() {
<-ctx.Done()
if err := conn.Close(); err != nil {
grpclog.Errorf("failed to close connection to the gRPC server: %v", err)
}
}()
s.conn = conn
log.Info("Connected to grpc server", "connectionState", conn.GetState())

// TODO(jeremy): Should we add a handler for openapi spec; e.g.
// https://github.com/grpc-ecosystem/grpc-gateway/blob/10d49ec19ecab090aa3318245e3fe0d5db666c3f/examples/internal/gateway/main.go#L51C2-L51C49

gwMux := runtime.NewServeMux()

if err := v1alpha1.RegisterExecuteServiceHandler(ctx, gwMux, conn); err != nil {
return err
}

if err := v1alpha1.RegisterGenerateServiceHandler(ctx, gwMux, conn); err != nil {
return err
}

// Configure gin to delegate to the grpc gateway
handleFunc := func(c *gin.Context) {
log.V(logs.Debug).Info("Delegating request to grpc gateway")
gwMux.ServeHTTP(c.Writer, c.Request)
}

// N.B since we want to to server our grpc gateway on the same port as our gin server
// we need to configure the gin server to delegate to the gateway mux for the appropriate routes.
// There currently doesn't seem to be anyway to do this programmatically. So if we add new routes we'd
// have to update the code here.
// TODO(jeremy): Actually I don't think we can use the group method but I think we can use the star method.
// e.g. router.Any("/api/*any", handleFunc)
// api := router.Group("/api", handleFunc)
pathPrefix := "/api/v1alpha1"

type method struct {
Method string
Path string
}

methods := []method{
{Method: http.MethodPost, Path: "execute"},
{Method: http.MethodPost, Path: "generate"},
}

for _, m := range methods {
fullPath := pathPrefix + "/" + m.Path
log.Info("configuring gin to delegate to the grpc gateway", "path", fullPath, "methods", m.Method)
s.engine.Handle(m.Method, fullPath, handleFunc)
}

return nil
}

// trapInterrupt shutdowns the server if the appropriate signals are sent
func trapInterrupt(s *Server) {
log := zapr.NewLogger(zap.L())
Expand All @@ -612,17 +473,11 @@ func trapInterrupt(s *Server) {

func (s *Server) healthCheck(ctx *gin.Context) {
// TODO(jeremy): We should return the version
connState := s.conn.GetState()
d := gin.H{
"server": "foyle",
"status": "healthy",
"grpcConnectionState": connState.String(),
"server": "foyle",
"status": "healthy",
}
code := http.StatusOK
if connState != connectivity.Ready && connState != connectivity.Idle {
d["status"] = "unhealthy"
code = http.StatusServiceUnavailable
}
ctx.JSON(code, d)
}

Expand Down
17 changes: 8 additions & 9 deletions docs/content/en/docs/getting-started/_index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,20 @@ to provide the frontend.
foyle serve
```

* By default foyle uses port 8877 for the http server and port 9080 for gRPC
* By default foyle uses port 8877 for the http server

* If you need to use different ports you can configure this as follows
* If you need to use a different port you can configure this as follows

```sh
foyle config set server.httpPort=<YOUR HTTP PORT>
foyle config set server.grpcPort=<YOUR GRPC PORT>
```

1. Inside VSCode configure RunMe to use Foyle
1. Open the VSCode setting palette
1. Search for `Runme: Foyle Address`
1. Set the address to `localhost:${GRPC_PORT}`
* The default port is 9080
* If you set a non default value then it will be the value of `server.grpcPort`
1. Search for `Runme: Ai Base URL`
1. Set the address to `http://localhost:${HTTP_PORT}/api`
* The default port is 8877
* If you set a non default value then it will be the value of `server.httpPort`

## Try it out!

Expand All @@ -80,8 +79,8 @@ Now that foyle is running you can open markdown documents in VSCode and start in
### Customizing the Foyle Server Address

1. Open the settings panel; you can click the gear icon in the lower left window and then select settings
2. Search for `Foyle`
3. Set `Runme: Foyle Address` to the address of the Foyle server to use as the Agent
2. Search for `Runme: Ai base URL
3. Set `Runme: Ai base URL` to the address of the Foyle server to use as the Agent
* The Agent handles requests to generate completions

### Customizing the keybindings
Expand Down
13 changes: 0 additions & 13 deletions protos/buf.gen.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@ managed:
enabled: true
disable:
- module: buf.build/googleapis/googleapis
# - module: buf.build/stateful/runme
# override:
# - path: runme
# file_option: runme/parser/v1

plugins:
- local: protoc-gen-go
out: go/
Expand All @@ -17,14 +12,6 @@ plugins:
# the option go_package in the proto file. If its source_relative
# then its pased on the path relative to the location of the buf.yaml file.
- paths=source_relative
- local: protoc-gen-go-grpc
out: go/
opt:
- paths=source_relative
- local: protoc-gen-grpc-gateway
out: go/
opt:
- paths=source_relative
- local: protoc-gen-zap-marshaler
out: go/
opt: paths=source_relative
Expand Down
Loading

0 comments on commit affb5ee

Please sign in to comment.