From 95c8b6df271a42291f8453f5edc57f435a8c095d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 29 Oct 2024 16:05:53 +1100 Subject: [PATCH] fix: language plugin logs have named scope and are parsed as json (#3229) --- frontend/cli/cmd_new.go | 2 +- internal/buildengine/engine.go | 2 +- .../external_integration_test.go | 2 +- .../languageplugin/external_plugin.go | 4 +-- .../languageplugin/external_plugin_client.go | 25 +++++++++++++++---- .../languageplugin/go_plugin_test.go | 4 +-- .../languageplugin/java_plugin_test.go | 2 +- internal/buildengine/languageplugin/plugin.go | 4 +-- .../cmd/ftl-language-python/main.go | 3 ++- 9 files changed, 32 insertions(+), 16 deletions(-) diff --git a/frontend/cli/cmd_new.go b/frontend/cli/cmd_new.go index f01b7ddc83..ca33136e6c 100644 --- a/frontend/cli/cmd_new.go +++ b/frontend/cli/cmd_new.go @@ -69,7 +69,7 @@ func prepareNewCmd(ctx context.Context, k *kong.Kong, args []string) (optionalPl return optionalPlugin, fmt.Errorf("could not create bind allocator: %w", err) } - plugin, err := languageplugin.New(ctx, bindAllocator, language) + plugin, err := languageplugin.New(ctx, bindAllocator, language, "new") if err != nil { return optionalPlugin, fmt.Errorf("could not create plugin for %v: %w", language, err) } diff --git a/internal/buildengine/engine.go b/internal/buildengine/engine.go index 66b4a98faf..20a23a4255 100644 --- a/internal/buildengine/engine.go +++ b/internal/buildengine/engine.go @@ -1028,7 +1028,7 @@ func (e *Engine) gatherSchemas( } func (e *Engine) newModuleMeta(ctx context.Context, config moduleconfig.UnvalidatedModuleConfig) (moduleMeta, error) { - plugin, err := languageplugin.New(ctx, e.bindAllocator, config.Language) + plugin, err := languageplugin.New(ctx, e.bindAllocator, config.Language, config.Module) if err != nil { return moduleMeta{}, fmt.Errorf("could not create plugin for %s: %w", config.Module, err) } diff --git a/internal/buildengine/languageplugin/external_integration_test.go b/internal/buildengine/languageplugin/external_integration_test.go index 4daaaf12f0..e6b8158850 100644 --- a/internal/buildengine/languageplugin/external_integration_test.go +++ b/internal/buildengine/languageplugin/external_integration_test.go @@ -244,7 +244,7 @@ func startPlugin() in.Action { bindURL, err = bindAllocator.Next() assert.NoError(t, err) - client, err = newExternalPluginImpl(ic.Context, bindURL, ic.Language) + client, err = newExternalPluginImpl(ic.Context, bindURL, ic.Language, "test") assert.NoError(t, err) } } diff --git a/internal/buildengine/languageplugin/external_plugin.go b/internal/buildengine/languageplugin/external_plugin.go index d505749e3d..e985d4229c 100644 --- a/internal/buildengine/languageplugin/external_plugin.go +++ b/internal/buildengine/languageplugin/external_plugin.go @@ -49,8 +49,8 @@ type externalPlugin struct { var _ LanguagePlugin = &externalPlugin{} -func newExternalPlugin(ctx context.Context, bind *url.URL, language string) (*externalPlugin, error) { - impl, err := newExternalPluginImpl(ctx, bind, language) +func newExternalPlugin(ctx context.Context, bind *url.URL, language, name string) (*externalPlugin, error) { + impl, err := newExternalPluginImpl(ctx, bind, language, name) if err != nil { return nil, err } diff --git a/internal/buildengine/languageplugin/external_plugin_client.go b/internal/buildengine/languageplugin/external_plugin_client.go index 79f09138c3..1e3f878098 100644 --- a/internal/buildengine/languageplugin/external_plugin_client.go +++ b/internal/buildengine/languageplugin/external_plugin_client.go @@ -45,11 +45,11 @@ type externalPluginImpl struct { cmdError chan error } -func newExternalPluginImpl(ctx context.Context, bind *url.URL, language string) (*externalPluginImpl, error) { +func newExternalPluginImpl(ctx context.Context, bind *url.URL, language, name string) (*externalPluginImpl, error) { impl := &externalPluginImpl{ client: rpc.Dial(langconnect.NewLanguageServiceClient, bind.String(), log.Error), } - err := impl.start(ctx, bind, language) + err := impl.start(ctx, bind, language, name) if err != nil { return nil, err } @@ -57,14 +57,30 @@ func newExternalPluginImpl(ctx context.Context, bind *url.URL, language string) } // Start launches the plugin and blocks until the plugin is ready. -func (p *externalPluginImpl) start(ctx context.Context, bind *url.URL, language string) error { +func (p *externalPluginImpl) start(ctx context.Context, bind *url.URL, language, name string) error { + logger := log.FromContext(ctx).Scope(name) + cmdName := "ftl-language-" + language p.cmd = exec.Command(ctx, log.Debug, ".", cmdName, "--bind", bind.String()) + p.cmd.Env = append(p.cmd.Env, "FTL_NAME="+name) _, err := exec.LookPath(cmdName) if err != nil { return fmt.Errorf("failed to find plugin for %s: %w", language, err) } + // Send the plugin's stderr to the logger. + p.cmd.Stderr = nil + pipe, err := p.cmd.StderrPipe() + if err != nil { + return fmt.Errorf("could not create stderr pipe for %s: %w", name, err) + } + go func() { + err := log.JSONStreamer(pipe, logger, log.Error) + if err != nil { + logger.Errorf(err, "Error streaming plugin logs.") + } + }() + runCtx, cancel := context.WithCancel(ctx) p.cmdError = make(chan error) @@ -72,8 +88,7 @@ func (p *externalPluginImpl) start(ctx context.Context, bind *url.URL, language // run the plugin and wait for it to finish executing go func() { - err := p.cmd.RunBuffered(runCtx) - fmt.Printf("ended!") + err := p.cmd.Run() if err != nil { p.cmdError <- fmt.Errorf("language plugin failed: %w", err) } else { diff --git a/internal/buildengine/languageplugin/go_plugin_test.go b/internal/buildengine/languageplugin/go_plugin_test.go index 084a49b5de..90222852d4 100644 --- a/internal/buildengine/languageplugin/go_plugin_test.go +++ b/internal/buildengine/languageplugin/go_plugin_test.go @@ -32,7 +32,7 @@ func TestExtractModuleDepsGo(t *testing.T) { uncheckedConfig, err := moduleconfig.LoadConfig(dir) assert.NoError(t, err) - plugin, err := New(ctx, nil, uncheckedConfig.Language) + plugin, err := New(ctx, nil, uncheckedConfig.Language, "test") assert.NoError(t, err) customDefaults, err := plugin.ModuleConfigDefaults(ctx, uncheckedConfig.Dir) @@ -85,7 +85,7 @@ func TestGoConfigDefaults(t *testing.T) { dir, err := filepath.Abs(tt.dir) assert.NoError(t, err) - plugin, err := New(ctx, nil, "go") + plugin, err := New(ctx, nil, "go", "test") assert.NoError(t, err) defaults, err := plugin.ModuleConfigDefaults(ctx, dir) diff --git a/internal/buildengine/languageplugin/java_plugin_test.go b/internal/buildengine/languageplugin/java_plugin_test.go index 8cb8b7272e..12f63f3c35 100644 --- a/internal/buildengine/languageplugin/java_plugin_test.go +++ b/internal/buildengine/languageplugin/java_plugin_test.go @@ -63,7 +63,7 @@ func TestJavaConfigDefaults(t *testing.T) { dir, err := filepath.Abs(tt.dir) assert.NoError(t, err) - plugin, err := New(ctx, nil, "java") + plugin, err := New(ctx, nil, "java", "test") assert.NoError(t, err) defaults, err := plugin.ModuleConfigDefaults(ctx, dir) diff --git a/internal/buildengine/languageplugin/plugin.go b/internal/buildengine/languageplugin/plugin.go index 7a8b814e03..20a62e517f 100644 --- a/internal/buildengine/languageplugin/plugin.go +++ b/internal/buildengine/languageplugin/plugin.go @@ -132,7 +132,7 @@ type LanguagePlugin interface { } // PluginFromConfig creates a new language plugin from the given config. -func New(ctx context.Context, bindAllocator *bind.BindAllocator, language string) (p LanguagePlugin, err error) { +func New(ctx context.Context, bindAllocator *bind.BindAllocator, language, name string) (p LanguagePlugin, err error) { switch language { case "go": return newGoPlugin(ctx), nil @@ -145,7 +145,7 @@ func New(ctx context.Context, bindAllocator *bind.BindAllocator, language string if err != nil { return nil, fmt.Errorf("failed to allocate port for external plugin: %w", err) } - return newExternalPlugin(ctx, port, language) + return newExternalPlugin(ctx, port, language, name) } } diff --git a/python-runtime/cmd/ftl-language-python/main.go b/python-runtime/cmd/ftl-language-python/main.go index b901444e14..b665505674 100644 --- a/python-runtime/cmd/ftl-language-python/main.go +++ b/python-runtime/cmd/ftl-language-python/main.go @@ -2,6 +2,7 @@ package main import ( "context" + "os" "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/language/languagepbconnect" "github.com/TBD54566975/ftl/common/plugin" @@ -11,7 +12,7 @@ import ( func main() { plugin.Start( context.Background(), - "ftl-language-python", + os.Getenv("FTL_NAME"), createService, languagepbconnect.LanguageServiceName, languagepbconnect.NewLanguageServiceHandler,