Skip to content

Commit

Permalink
chore: remove redundant log messages in language plugin proto (#3212)
Browse files Browse the repository at this point in the history
We will use stdout with the json log format instead, as this matches
what we do elsewhere.
  • Loading branch information
matt2e authored Oct 28, 2024
1 parent 9311f72 commit 7d74fbd
Show file tree
Hide file tree
Showing 11 changed files with 228 additions and 533 deletions.
568 changes: 214 additions & 354 deletions backend/protos/xyz/block/ftl/v1/language/language.pb.go

Large diffs are not rendered by default.

17 changes: 3 additions & 14 deletions backend/protos/xyz/block/ftl/v1/language/language.proto
Original file line number Diff line number Diff line change
Expand Up @@ -233,26 +233,12 @@ message BuildFailure {
bool invalidate_dependencies = 4;
}

// Log message from the language service.
message LogMessage {
enum LogLevel {
DEBUG = 0;
INFO = 1;
WARN = 2;
ERROR = 3;
}

string message = 1;
LogLevel level = 2;
}

// Every type of message that can be streamed from the language plugin for a build.
message BuildEvent {
oneof event {
AutoRebuildStarted auto_rebuild_started = 2;
BuildSuccess build_success = 3;
BuildFailure build_failure = 4;
LogMessage log_message = 5;
}
}

Expand Down Expand Up @@ -320,6 +306,9 @@ service LanguageService {
//
// Each time this call is made, the Build call must send back a corresponding BuildSuccess or BuildFailure
// event with the updated build context id with "is_automatic_rebuild" as false.
//
// If the plugin will not be able to return a BuildSuccess or BuildFailure, such as when there is no active
// build stream, it must fail the BuildContextUpdated call.
rpc BuildContextUpdated(BuildContextUpdatedRequest) returns (BuildContextUpdatedResponse);

// Generate stubs for a module.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 0 additions & 31 deletions backend/protos/xyz/block/ftl/v1/language/mixins.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
structpb "google.golang.org/protobuf/types/known/structpb"

"github.com/TBD54566975/ftl/internal/builderrors"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/moduleconfig"
"github.com/TBD54566975/ftl/internal/projectconfig"
"github.com/TBD54566975/ftl/internal/slices"
Expand Down Expand Up @@ -86,36 +85,6 @@ func PosFromProto(pos *Position) optional.Option[builderrors.Position] {
})
}

func LogLevelFromProto(level LogMessage_LogLevel) log.Level {
switch level {
case LogMessage_INFO:
return log.Info
case LogMessage_DEBUG:
return log.Debug
case LogMessage_WARN:
return log.Warn
case LogMessage_ERROR:
return log.Error
default:
panic(fmt.Sprintf("unhandled log level %v", level))
}
}

func LogLevelToProto(level log.Level) LogMessage_LogLevel {
switch level {
case log.Info:
return LogMessage_INFO
case log.Debug:
return LogMessage_DEBUG
case log.Warn:
return LogMessage_WARN
case log.Error:
return LogMessage_ERROR
default:
panic(fmt.Sprintf("unhandled log level %v", level))
}
}

// ModuleConfigToProto converts a moduleconfig.AbsModuleConfig to a protobuf ModuleConfig.
//
// Absolute configs are used because relative paths may change resolve differently between parties.
Expand Down
2 changes: 1 addition & 1 deletion common/plugin/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func Start[Impl any, Iface any, Config any](
ctx, cancel := context.WithCancel(ctx)
defer cancel()

// Configure logging to JSON on stdout. This will be read by the parent process.
// Configure logging to JSON on stderr. This will be read by the parent process.
logConfig := cli.LogConfig
logConfig.JSON = true
logger := log.Configure(os.Stderr, logConfig)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,6 @@ func waitForAutoRebuildToStart(contextId string) in.Action {
} else {
logger.Warnf("ignoring build failure for unexpected context %q while waiting for auto rebuild started event for %q", event.BuildFailure.ContextId, contextId)
}
case *langpb.BuildEvent_LogMessage:
logger.Debugf("plugin: %s", event.LogMessage.Message)
}
}
}
Expand Down Expand Up @@ -484,9 +482,6 @@ func waitForBuildToEnd(success BuildResultType, contextId string, automaticRebui
additionalChecks(t, ic, e)
}
return

case *langpb.BuildEvent_LogMessage:
logger.Infof("plugin log: %v", event.LogMessage.Message)
}
}
}
Expand All @@ -495,7 +490,6 @@ func waitForBuildToEnd(success BuildResultType, contextId string, automaticRebui
func checkForNoEvents(duration time.Duration) in.Action {
return func(t testing.TB, ic in.TestContext) {
in.Infof("Checking for no events for %v", duration)
logger := log.FromContext(ic.Context)
for {
select {
case result := <-buildChan:
Expand All @@ -508,9 +502,6 @@ func checkForNoEvents(duration time.Duration) in.Action {
panic(fmt.Sprintf("build success event when expecting no events: %v", event))
case *langpb.BuildEvent_BuildFailure:
panic(fmt.Sprintf("build failure event when expecting no events: %v", event))
case *langpb.BuildEvent_LogMessage:
logger.Infof("plugin log: %v", event.LogMessage.Message)
continue
}
case <-time.After(duration):
return
Expand Down
4 changes: 1 addition & 3 deletions internal/buildengine/languageplugin/external_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,7 @@ func (p *externalPlugin) run(ctx context.Context) {
continue
}

switch event := e.Event.(type) {
case *langpb.BuildEvent_LogMessage:
logger.Logf(langpb.LogLevelFromProto(event.LogMessage.Level), "%s", event.LogMessage.Message)
switch e.Event.(type) {
case *langpb.BuildEvent_AutoRebuildStarted:
if _, ok := activeBuildCmd.Get(); ok {
logger.Debugf("ignoring automatic rebuild started during explicit build")
Expand Down
35 changes: 0 additions & 35 deletions python-runtime/python-plugin/log_to_stream.go

This file was deleted.

2 changes: 0 additions & 2 deletions python-runtime/python-plugin/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,6 @@ func (s *Service) Build(ctx context.Context, req *connect.Request[langpb.BuildRe
logger := log.FromContext(ctx)
logger.Infof("Do python build")

ctx = log.ContextWithLogger(ctx, newLoggerForStream(log.Debug, stream))

buildCtx, err := buildContextFromProto(req.Msg.BuildContext)
if err != nil {
return err
Expand Down

0 comments on commit 7d74fbd

Please sign in to comment.