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

chore: remove redundant log messages in language plugin proto #3212

Merged
merged 1 commit into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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