From 8aebe7680334278194228728abdf43d9f583333a Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Fri, 24 May 2024 20:17:06 -0400 Subject: [PATCH] [Go] move logging out of internal Move logging support out of internal so third-party plugins can use it. We can't put it in core because tracing needs it, so I made a package for it. At that point I felt that `logger.FromContext` read better than `logging.Logger`. --- go/core/action.go | 7 ++++--- go/core/flow.go | 7 ++++--- go/{internal/logging.go => core/logger/logger.go} | 11 +++++++---- go/core/metrics.go | 4 ++-- go/core/servers.go | 6 +++--- go/core/tracing/tracing.go | 3 ++- go/plugins/localvec/localvec.go | 4 ++-- 7 files changed, 24 insertions(+), 18 deletions(-) rename go/{internal/logging.go => core/logger/logger.go} (77%) diff --git a/go/core/action.go b/go/core/action.go index b7361db4bc..31e94e8e96 100644 --- a/go/core/action.go +++ b/go/core/action.go @@ -22,8 +22,9 @@ import ( "reflect" "time" - "github.com/firebase/genkit/go/internal" + "github.com/firebase/genkit/go/core/logger" "github.com/firebase/genkit/go/core/tracing" + "github.com/firebase/genkit/go/internal" "github.com/invopop/jsonschema" ) @@ -102,11 +103,11 @@ func (a *Action[I, O, S]) setTracingState(tstate *tracing.State) { a.tstate = ts func (a *Action[I, O, S]) Run(ctx context.Context, input I, cb func(context.Context, S) error) (output O, err error) { // TODO: validate input against JSONSchema for I. // TODO: validate output against JSONSchema for O. - internal.Logger(ctx).Debug("Action.Run", + logger.FromContext(ctx).Debug("Action.Run", "name", a.name, "input", fmt.Sprintf("%#v", input)) defer func() { - internal.Logger(ctx).Debug("Action.Run", + logger.FromContext(ctx).Debug("Action.Run", "name", a.name, "output", fmt.Sprintf("%#v", output), "err", err) diff --git a/go/core/flow.go b/go/core/flow.go index 1457906b1a..7ecb7b3b0c 100644 --- a/go/core/flow.go +++ b/go/core/flow.go @@ -24,6 +24,7 @@ import ( "sync" "time" + "github.com/firebase/genkit/go/core/logger" "github.com/firebase/genkit/go/core/tracing" "github.com/firebase/genkit/go/gtime" "github.com/firebase/genkit/go/internal" @@ -346,7 +347,7 @@ func (f *Flow[I, O, S]) execute(ctx context.Context, state *flowState[I, O], dis defer func() { if err := fctx.finish(ctx); err != nil { // TODO(jba): do something more with this error? - internal.Logger(ctx).Error("flowContext.finish", "err", err.Error()) + logger.FromContext(ctx).Error("flowContext.finish", "err", err.Error()) } }() ctx = flowContextKey.NewContext(ctx, fctx) @@ -374,14 +375,14 @@ func (f *Flow[I, O, S]) execute(ctx context.Context, state *flowState[I, O], dis latency := time.Since(start) if err != nil { // TODO(jba): handle InterruptError - internal.Logger(ctx).Error("flow failed", + logger.FromContext(ctx).Error("flow failed", "path", tracing.SpanPath(ctx), "err", err.Error(), ) writeFlowFailure(ctx, f.name, latency, err) tracing.SetCustomMetadataAttr(ctx, "flow:state", "error") } else { - internal.Logger(ctx).Info("flow succeeded", "path", tracing.SpanPath(ctx)) + logger.FromContext(ctx).Info("flow succeeded", "path", tracing.SpanPath(ctx)) writeFlowSuccess(ctx, f.name, latency) tracing.SetCustomMetadataAttr(ctx, "flow:state", "done") diff --git a/go/internal/logging.go b/go/core/logger/logger.go similarity index 77% rename from go/internal/logging.go rename to go/core/logger/logger.go index 2e84256493..22e8fb7bca 100644 --- a/go/internal/logging.go +++ b/go/core/logger/logger.go @@ -12,12 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -package internal +// Package logger provides a context-scoped slog.Logger. +package logger import ( "context" "log/slog" "os" + + "github.com/firebase/genkit/go/internal" ) func init() { @@ -29,11 +32,11 @@ func init() { slog.SetDefault(h) } -var loggerKey = NewContextKey[*slog.Logger]() +var loggerKey = internal.NewContextKey[*slog.Logger]() -// Logger returns the Logger in ctx, or the default Logger +// FromContext returns the Logger in ctx, or the default Logger // if there is none. -func Logger(ctx context.Context) *slog.Logger { +func FromContext(ctx context.Context) *slog.Logger { if l := loggerKey.FromContext(ctx); l != nil { return l } diff --git a/go/core/metrics.go b/go/core/metrics.go index 13ca3b2376..117f53a12e 100644 --- a/go/core/metrics.go +++ b/go/core/metrics.go @@ -19,7 +19,7 @@ import ( "sync" "time" - "github.com/firebase/genkit/go/internal" + "github.com/firebase/genkit/go/core/logger" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" @@ -38,7 +38,7 @@ var fetchInstruments = sync.OnceValue(func() *metricInstruments { insts, err := initInstruments() if err != nil { // Do not stop the program because we can't collect metrics. - internal.Logger(context.Background()).Error("metric initialization failed; no metrics will be collected", "err", err) + logger.FromContext(context.Background()).Error("metric initialization failed; no metrics will be collected", "err", err) return nil } return insts diff --git a/go/core/servers.go b/go/core/servers.go index 93d37cc2e9..188188dc4f 100644 --- a/go/core/servers.go +++ b/go/core/servers.go @@ -38,7 +38,7 @@ import ( "syscall" "time" - "github.com/firebase/genkit/go/internal" + "github.com/firebase/genkit/go/core/logger" "github.com/firebase/genkit/go/core/tracing" "go.opentelemetry.io/otel/trace" ) @@ -114,7 +114,7 @@ func (s *devServer) handleRunAction(w http.ResponseWriter, r *http.Request) erro return err } } - internal.Logger(ctx).Debug("running action", + logger.FromContext(ctx).Debug("running action", "key", body.Key, "stream", stream) var callback streamingCallback[json.RawMessage] @@ -400,7 +400,7 @@ func writeJSON(ctx context.Context, w http.ResponseWriter, value any) error { } _, err = w.Write(data) if err != nil { - internal.Logger(ctx).Error("writing output", "err", err) + logger.FromContext(ctx).Error("writing output", "err", err) } return nil } diff --git a/go/core/tracing/tracing.go b/go/core/tracing/tracing.go index cc11da7093..4464bdb1f9 100644 --- a/go/core/tracing/tracing.go +++ b/go/core/tracing/tracing.go @@ -19,6 +19,7 @@ import ( "context" "sync" + "github.com/firebase/genkit/go/core/logger" "github.com/firebase/genkit/go/internal" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -88,7 +89,7 @@ func RunInNewSpan[I, O any]( f func(context.Context, I) (O, error), ) (O, error) { // TODO(jba): support span links. - log := internal.Logger(ctx) + log := logger.FromContext(ctx) log.Debug("span start", "name", name) defer log.Debug("span end", "name", name) sm := &spanMetadata{ diff --git a/go/plugins/localvec/localvec.go b/go/plugins/localvec/localvec.go index 79305fd43b..cc2374bb05 100644 --- a/go/plugins/localvec/localvec.go +++ b/go/plugins/localvec/localvec.go @@ -31,7 +31,7 @@ import ( "slices" "github.com/firebase/genkit/go/ai" - "github.com/firebase/genkit/go/internal" + "github.com/firebase/genkit/go/core/logger" ) // New returns a new local vector database. This will register a new @@ -110,7 +110,7 @@ func (r *retriever) Index(ctx context.Context, req *ai.IndexerRequest) error { } if _, ok := r.data[id]; ok { - internal.Logger(ctx).Debug("localvec skipping document because already present", "id", id) + logger.FromContext(ctx).Debug("localvec skipping document because already present", "id", id) continue }