zeroshade commented on code in PR #2729:
URL: https://github.com/apache/arrow-adbc/pull/2729#discussion_r2070382275
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -99,8 +154,17 @@ func (base *DatabaseImplBase) SetOptionInt(key string, val
int64) error {
return base.ErrorHelper.Errorf(adbc.StatusNotImplemented, "%s '%s'",
DatabaseMessageOptionUnknown, key)
}
+func (base *database) Close() error {
+ return base.Base().Close()
+}
+
func (base *DatabaseImplBase) Close() error {
- return nil
+ var err error = nil
+ if base.Base().tracerShutdownFunc != nil {
+ err = base.Base().tracerShutdownFunc(context.Background())
+ base.Base().tracerShutdownFunc = nil
+ }
+ return err
}
Review Comment:
we can increase the readability here
```go
func (base *DatabaseImplBase) Close() (err error) {
if base.Base().tracerShutdownFunc != nil {
err = base.Base().tracerShutdownFunc(context.Background())
base.Base().tracerShutdownFunc = nil
}
return
}
```
Also, will this actually work correctly by using `context.Background()` as
opposed to a context that actually contains the span/tracer information?
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
Review Comment:
should we check the env var on every call here? maybe use a `sync.Once` or
otherwise cache it? Or is it likely that the env var might change during
operation of a single process?
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
+ exporterType, _ := tryParseTraceExporterType(exporterName)
+ switch exporterType {
+ case TraceExporterConsole:
+ exporter, _ = newStdoutTraceExporter()
+ case TraceExporterOtlp:
+ exporter, _ = newOtlpTraceExporter(context.Background())
Review Comment:
We should probably have `InitTracing` take a context argument and pass it
through rather than using a `context.Background()` here so that contextual
information can be potentially passed through by the caller instead of making
that decision here for them.
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
+ exporterType, _ := tryParseTraceExporterType(exporterName)
+ switch exporterType {
+ case TraceExporterConsole:
+ exporter, _ = newStdoutTraceExporter()
+ case TraceExporterOtlp:
+ exporter, _ = newOtlpTraceExporter(context.Background())
+ case TraceExporterAdbcFile:
+ exporter, _ = newAdbcFileExporter(driverName)
+ }
Review Comment:
should we log or otherwise denote the failure if any of these return an
error or something? Why ignore the errors?
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
+ exporterType, _ := tryParseTraceExporterType(exporterName)
+ switch exporterType {
+ case TraceExporterConsole:
+ exporter, _ = newStdoutTraceExporter()
+ case TraceExporterOtlp:
+ exporter, _ = newOtlpTraceExporter(context.Background())
+ case TraceExporterAdbcFile:
+ exporter, _ = newAdbcFileExporter(driverName)
+ }
+
+ fullyQualifiedDriverName := driverNamespace + "." + driverName
+ if exporter != nil {
+ tracerProvider, err := newTracerProvider(exporter)
+ if err != nil {
+ panic(err)
+ }
+ base.Base().tracerShutdownFunc = tracerProvider.Shutdown
+ tracer = tracerProvider.Tracer(
+ fullyQualifiedDriverName,
+ trace.WithInstrumentationVersion(driverVersion),
+ trace.WithSchemaURL(semconv.SchemaURL),
+ )
+ } else {
+ tracer = otel.Tracer(fullyQualifiedDriverName)
+ }
+ base.Base().Tracer = tracer
+}
+
+func tryParseTraceExporterType(value string) (traceExporterType, bool) {
+ if te, ok := traceExporterNames[value]; ok {
+ return te, true
+ }
+ return TraceExporterNone, false
+}
+
+func getDriverVersion(driverInfo *DriverInfo) string {
+ var unknownDriverVersion = "unknown"
+ value, ok := driverInfo.GetInfoForInfoCode(adbc.InfoDriverVersion)
+ if !ok || value == nil {
Review Comment:
you don't need the `value == nil` check here since you do the checked type
assertion below
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
Review Comment:
the zero value for a string is the empty string, so I don't think this is
necessary
##########
go/adbc/driver/internal/driverbase/connection.go:
##########
@@ -222,7 +232,47 @@ func (base *ConnectionImplBase) ReadPartition(ctx
context.Context, serializedPar
return nil, base.ErrorHelper.Errorf(adbc.StatusNotImplemented,
"ReadPartition")
}
+func MaybeAddTraceParent(ctx context.Context, cnxn adbc.OTelTracing, st
adbc.OTelTracing) context.Context {
+ var hasTraceParent = false
+ var traceParentStr = ""
+ if st != nil && st.GetTraceParent() != "" {
+ traceParentStr = st.GetTraceParent()
+ hasTraceParent = true
+ } else if cnxn != nil && cnxn.GetTraceParent() != "" {
+ traceParentStr = cnxn.GetTraceParent()
+ hasTraceParent = true
+ }
+ if hasTraceParent {
+ spanContext, err := parseTraceparent(ctx, traceParentStr)
+ if err != nil {
+ return ctx
+ }
+ ctx = trace.ContextWithRemoteSpanContext(ctx, spanContext)
+ }
+ return ctx
+}
+
+func parseTraceparent(ctx context.Context, traceParentStr string)
(trace.SpanContext, error) {
Review Comment:
let's be consistent, `parseTraceParent` (since everywhere else we use camel
case and capitalize `Parent`)
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
Review Comment:
the standard the standard pattern here would be the more concise and simpler
`type RotatingFileWriterOption func(*config)`
with the options being like
```go
func WithTracingFolderPath(tracingFolderPath string)
RotatingFileWriterOption {
return func(cfg *config) {
cfg.TracingFolderPath = tracingFolderPath
}
}
```
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
Review Comment:
Do you forsee us needing multiple implementations of this? If not, let's
avoid pre-emptively making it an interface and instead just use the concrete
object until we actually need another implementation. It's best practice to
avoid pre-emptively constructing interfaces unless you forsee users wanting to
provide their own implementations
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
+
+ // Test if we can create/write a file in the traces folder
+ tempFile, err := os.CreateTemp(tracingFolderPath, logNamePrefix)
+ if err != nil {
+ return nil, err
+ }
+ defer func() {
+ _ = tempFile.Close()
+ _ = os.Remove(tempFile.Name())
+ }()
+ _, err = tempFile.WriteString("file started")
+ if err != nil {
+ return nil, err
+ }
+
+ return &rotatingFileWriterImpl{
+ TracingFolderPath: tracingFolderPath,
+ LogNamePrefix: logNamePrefix,
+ FileSizeMaxKb: fileSizeMaxKb,
+ FileCountMax: fileCountMax,
+ CurrentWriter: nil,
+ }, nil
+}
+
+func getDefaultTracingFolderPath() (string, error) {
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ fullPath := filepath.Join(userHome, ".adbc", "traces")
+ return fullPath, nil
+}
+
+// Closes the rotating file write
+func (base *rotatingFileWriterImpl) Close() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ base.CurrentWriter = nil
+ return err
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Clear() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ }
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return err
+ }
+ for _, file := range logFiles {
+ err := os.Remove(filepath.Join(base.TracingFolderPath,
file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Stat() (fs.FileInfo, error) {
+ if base.CurrentWriter != nil {
+ return base.CurrentWriter.Stat()
+ }
+ return nil, errors.New("no trace file is open")
+}
+
+func (base *rotatingFileWriterImpl) GetTracingFolderPath() string {
+ return base.TracingFolderPath
+}
+
+func (base *rotatingFileWriterImpl) GetLogNamePrefix() string {
+ return base.LogNamePrefix
+}
+
+func (base *rotatingFileWriterImpl) GetFileSizeMaxKb() int64 {
+ return base.FileSizeMaxKb
+}
+
+func (base *rotatingFileWriterImpl) GetFileCountMax() int {
+ return base.FileCountMax
+}
+
+func (base *rotatingFileWriterImpl) Write(p []byte) (int, error) {
+ err := (error)(nil)
+
+ // Check to see if file needs to rotate
+ if err = maybeCloseCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Ensure we have a current writer
+ if err = ensureCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Perform the write
+ return base.CurrentWriter.Write(p)
+}
+
+func maybeCloseCurrentWriter(base *rotatingFileWriterImpl) error {
+ if base.CurrentWriter != nil {
+ fileInfo, err := base.CurrentWriter.Stat()
+ if err != nil {
+ return err
+ }
+ if fileInfo.Size() >= int64(base.FileSizeMaxKb)*1024 {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = nil
+ return removeOldFiles(base)
+ }
+ }
+ return nil
+}
+
+func ensureCurrentWriter(base *rotatingFileWriterImpl) error {
+ const (
+ permissions = 0666 // Required to be full writable on Windows
we can open it again
+ createFlags = os.O_APPEND | os.O_CREATE | os.O_WRONLY
+ appendFlags = os.O_APPEND | os.O_WRONLY
+ )
+
+ if base.CurrentWriter == nil {
+ // check for a candidate file that is not full
+ ok, fullPathLastFile, err := getCandidateLogFileName(base)
Review Comment:
what scenario would `err == nil` but `ok == false`?
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
+
+ // Test if we can create/write a file in the traces folder
+ tempFile, err := os.CreateTemp(tracingFolderPath, logNamePrefix)
+ if err != nil {
+ return nil, err
+ }
+ defer func() {
+ _ = tempFile.Close()
+ _ = os.Remove(tempFile.Name())
+ }()
+ _, err = tempFile.WriteString("file started")
+ if err != nil {
+ return nil, err
+ }
+
+ return &rotatingFileWriterImpl{
+ TracingFolderPath: tracingFolderPath,
+ LogNamePrefix: logNamePrefix,
+ FileSizeMaxKb: fileSizeMaxKb,
+ FileCountMax: fileCountMax,
+ CurrentWriter: nil,
+ }, nil
+}
+
+func getDefaultTracingFolderPath() (string, error) {
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ fullPath := filepath.Join(userHome, ".adbc", "traces")
+ return fullPath, nil
+}
+
+// Closes the rotating file write
+func (base *rotatingFileWriterImpl) Close() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ base.CurrentWriter = nil
+ return err
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Clear() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ }
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return err
+ }
+ for _, file := range logFiles {
+ err := os.Remove(filepath.Join(base.TracingFolderPath,
file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Stat() (fs.FileInfo, error) {
+ if base.CurrentWriter != nil {
+ return base.CurrentWriter.Stat()
+ }
+ return nil, errors.New("no trace file is open")
+}
+
+func (base *rotatingFileWriterImpl) GetTracingFolderPath() string {
+ return base.TracingFolderPath
+}
+
+func (base *rotatingFileWriterImpl) GetLogNamePrefix() string {
+ return base.LogNamePrefix
+}
+
+func (base *rotatingFileWriterImpl) GetFileSizeMaxKb() int64 {
+ return base.FileSizeMaxKb
+}
+
+func (base *rotatingFileWriterImpl) GetFileCountMax() int {
+ return base.FileCountMax
+}
+
+func (base *rotatingFileWriterImpl) Write(p []byte) (int, error) {
+ err := (error)(nil)
+
+ // Check to see if file needs to rotate
+ if err = maybeCloseCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Ensure we have a current writer
+ if err = ensureCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Perform the write
+ return base.CurrentWriter.Write(p)
+}
+
+func maybeCloseCurrentWriter(base *rotatingFileWriterImpl) error {
+ if base.CurrentWriter != nil {
+ fileInfo, err := base.CurrentWriter.Stat()
+ if err != nil {
+ return err
+ }
+ if fileInfo.Size() >= int64(base.FileSizeMaxKb)*1024 {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = nil
+ return removeOldFiles(base)
+ }
+ }
+ return nil
+}
+
+func ensureCurrentWriter(base *rotatingFileWriterImpl) error {
+ const (
+ permissions = 0666 // Required to be full writable on Windows
we can open it again
+ createFlags = os.O_APPEND | os.O_CREATE | os.O_WRONLY
+ appendFlags = os.O_APPEND | os.O_WRONLY
+ )
+
+ if base.CurrentWriter == nil {
+ // check for a candidate file that is not full
+ ok, fullPathLastFile, err := getCandidateLogFileName(base)
+ if err != nil {
+ return err
+ }
+ // attempt to open exising candidate file
+ if ok {
+ currentWriter, err := os.OpenFile(fullPathLastFile,
appendFlags, permissions)
+ if err == nil {
+ base.CurrentWriter = currentWriter
+ return nil
+ }
+ // unable to open candidate file (locked?)
+ }
+
+ // open a new file
+ fullPath := buildNewFileName(base)
+ currentWriter, err := os.OpenFile(fullPath, createFlags,
permissions)
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = currentWriter
+ }
+ return nil
+}
+
+func buildNewFileName(base *rotatingFileWriterImpl) string {
+ now := time.Now().UTC()
+ safeTimeStamp :=
strings.ReplaceAll(strings.ReplaceAll(strings.Replace(strings.Replace(now.Format(time.RFC3339Nano),
"Z", "", 1), "T", "-", 1), ":", "-"), ".", "-")
Review Comment:
why not use a custom format with `now.Format` instead of performing all
these replaces?
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
+
+ // Test if we can create/write a file in the traces folder
+ tempFile, err := os.CreateTemp(tracingFolderPath, logNamePrefix)
+ if err != nil {
+ return nil, err
+ }
+ defer func() {
+ _ = tempFile.Close()
+ _ = os.Remove(tempFile.Name())
+ }()
+ _, err = tempFile.WriteString("file started")
+ if err != nil {
+ return nil, err
+ }
+
+ return &rotatingFileWriterImpl{
+ TracingFolderPath: tracingFolderPath,
+ LogNamePrefix: logNamePrefix,
+ FileSizeMaxKb: fileSizeMaxKb,
+ FileCountMax: fileCountMax,
+ CurrentWriter: nil,
+ }, nil
+}
+
+func getDefaultTracingFolderPath() (string, error) {
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ fullPath := filepath.Join(userHome, ".adbc", "traces")
+ return fullPath, nil
+}
+
+// Closes the rotating file write
+func (base *rotatingFileWriterImpl) Close() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ base.CurrentWriter = nil
+ return err
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Clear() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ }
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return err
+ }
+ for _, file := range logFiles {
+ err := os.Remove(filepath.Join(base.TracingFolderPath,
file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Stat() (fs.FileInfo, error) {
+ if base.CurrentWriter != nil {
+ return base.CurrentWriter.Stat()
+ }
+ return nil, errors.New("no trace file is open")
+}
+
+func (base *rotatingFileWriterImpl) GetTracingFolderPath() string {
+ return base.TracingFolderPath
+}
+
+func (base *rotatingFileWriterImpl) GetLogNamePrefix() string {
+ return base.LogNamePrefix
+}
+
+func (base *rotatingFileWriterImpl) GetFileSizeMaxKb() int64 {
+ return base.FileSizeMaxKb
+}
+
+func (base *rotatingFileWriterImpl) GetFileCountMax() int {
+ return base.FileCountMax
+}
+
+func (base *rotatingFileWriterImpl) Write(p []byte) (int, error) {
+ err := (error)(nil)
+
+ // Check to see if file needs to rotate
+ if err = maybeCloseCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Ensure we have a current writer
+ if err = ensureCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Perform the write
+ return base.CurrentWriter.Write(p)
+}
+
+func maybeCloseCurrentWriter(base *rotatingFileWriterImpl) error {
+ if base.CurrentWriter != nil {
+ fileInfo, err := base.CurrentWriter.Stat()
+ if err != nil {
+ return err
+ }
+ if fileInfo.Size() >= int64(base.FileSizeMaxKb)*1024 {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = nil
+ return removeOldFiles(base)
+ }
+ }
+ return nil
+}
+
+func ensureCurrentWriter(base *rotatingFileWriterImpl) error {
+ const (
+ permissions = 0666 // Required to be full writable on Windows
we can open it again
+ createFlags = os.O_APPEND | os.O_CREATE | os.O_WRONLY
+ appendFlags = os.O_APPEND | os.O_WRONLY
+ )
+
+ if base.CurrentWriter == nil {
+ // check for a candidate file that is not full
+ ok, fullPathLastFile, err := getCandidateLogFileName(base)
+ if err != nil {
+ return err
+ }
+ // attempt to open exising candidate file
+ if ok {
+ currentWriter, err := os.OpenFile(fullPathLastFile,
appendFlags, permissions)
+ if err == nil {
+ base.CurrentWriter = currentWriter
+ return nil
+ }
+ // unable to open candidate file (locked?)
+ }
+
+ // open a new file
+ fullPath := buildNewFileName(base)
+ currentWriter, err := os.OpenFile(fullPath, createFlags,
permissions)
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = currentWriter
+ }
+ return nil
+}
+
+func buildNewFileName(base *rotatingFileWriterImpl) string {
+ now := time.Now().UTC()
+ safeTimeStamp :=
strings.ReplaceAll(strings.ReplaceAll(strings.Replace(strings.Replace(now.Format(time.RFC3339Nano),
"Z", "", 1), "T", "-", 1), ":", "-"), ".", "-")
+ fileName := base.LogNamePrefix + "-" + safeTimeStamp +
defaultTraceFileExt
+ fullPath := filepath.Join(base.TracingFolderPath, fileName)
+ return fullPath
+}
+
+func getCandidateLogFileName(base *rotatingFileWriterImpl) (bool, string,
error) {
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil || len(logFiles) < 1 {
+ return false, "", err
+ }
+ lastLogFile := logFiles[len(logFiles)-1]
+ fileSizeMaxBytes := base.FileSizeMaxKb * 1024
+ fileInfo, err := lastLogFile.Info()
+ if err != nil || fileInfo.Size() >= fileSizeMaxBytes {
+ return false, "", err
+ }
+ // Return full path name
+ fullPathFileName := filepath.Join(base.TracingFolderPath,
lastLogFile.Name())
+ return true, fullPathFileName, nil
+}
+
+func removeOldFiles(base *rotatingFileWriterImpl) error {
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return nil
+ }
+ len := len(logFiles)
+ if len > int(base.FileCountMax) {
Review Comment:
`if len(logFiles) > int(base.FileCountMax) {` ?
We shouldn't shadow the `len` built-in function
##########
go/adbc/driver/snowflake/statement.go:
##########
@@ -499,41 +523,56 @@ func (st *statement) ExecuteQuery(ctx context.Context)
(array.RecordReader, int6
st.streamBind = nil
rdr := concatReader{}
- err := rdr.Init(&bind)
+ err = rdr.Init(&bind)
if err != nil {
return nil, -1, err
}
return &rdr, -1, nil
}
- loader, err := st.cnxn.cn.QueryArrowStream(ctx, st.query)
+ var loader gosnowflake.ArrowStreamLoader
+ loader, err = st.cnxn.cn.QueryArrowStream(ctx, st.query)
if err != nil {
- return nil, -1, errToAdbcErr(adbc.StatusInternal, err)
+ err = errToAdbcErr(adbc.StatusInternal, err)
+ return nil, -1, err
}
- rdr, err := newRecordReader(ctx, st.alloc, loader, st.queueSize,
st.prefetchConcurrency, st.useHighPrecision)
- nrec := loader.TotalRows()
- return rdr, nrec, err
+ reader, err = newRecordReader(ctx, st.alloc, loader, st.queueSize,
st.prefetchConcurrency, st.useHighPrecision)
+ nrows = loader.TotalRows()
+ return reader, nrows, err
}
// ExecuteUpdate executes a statement that does not generate a result
// set. It returns the number of rows affected if known, otherwise -1.
-func (st *statement) ExecuteUpdate(ctx context.Context) (int64, error) {
+func (st *statement) ExecuteUpdate(ctx context.Context) (nrows int64, err
error) {
ctx = st.setQueryContext(ctx)
+ err = nil
+ nrows = -1
+
+ _, span := st.StartSpan(ctx, "ExecuteUpdate")
Review Comment:
same comment as above. you can't ignore the `ctx` return from
`st.StartSpan`, you should swap the order here. It should look like:
```go
var span trace.Span
ctx, span = st.StartSpan(ctx, "ExecuteUpdate")
ctx, nrows = st.setQueryContext(ctx), -1
defer func() { .....
```
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
+
+ // Test if we can create/write a file in the traces folder
+ tempFile, err := os.CreateTemp(tracingFolderPath, logNamePrefix)
+ if err != nil {
+ return nil, err
+ }
+ defer func() {
+ _ = tempFile.Close()
+ _ = os.Remove(tempFile.Name())
+ }()
+ _, err = tempFile.WriteString("file started")
+ if err != nil {
+ return nil, err
+ }
+
+ return &rotatingFileWriterImpl{
+ TracingFolderPath: tracingFolderPath,
+ LogNamePrefix: logNamePrefix,
+ FileSizeMaxKb: fileSizeMaxKb,
+ FileCountMax: fileCountMax,
+ CurrentWriter: nil,
+ }, nil
+}
+
+func getDefaultTracingFolderPath() (string, error) {
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ fullPath := filepath.Join(userHome, ".adbc", "traces")
+ return fullPath, nil
+}
+
+// Closes the rotating file write
+func (base *rotatingFileWriterImpl) Close() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ base.CurrentWriter = nil
+ return err
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Clear() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ }
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return err
+ }
+ for _, file := range logFiles {
+ err := os.Remove(filepath.Join(base.TracingFolderPath,
file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Stat() (fs.FileInfo, error) {
+ if base.CurrentWriter != nil {
+ return base.CurrentWriter.Stat()
+ }
+ return nil, errors.New("no trace file is open")
+}
+
+func (base *rotatingFileWriterImpl) GetTracingFolderPath() string {
+ return base.TracingFolderPath
+}
+
+func (base *rotatingFileWriterImpl) GetLogNamePrefix() string {
+ return base.LogNamePrefix
+}
+
+func (base *rotatingFileWriterImpl) GetFileSizeMaxKb() int64 {
+ return base.FileSizeMaxKb
+}
+
+func (base *rotatingFileWriterImpl) GetFileCountMax() int {
+ return base.FileCountMax
+}
+
+func (base *rotatingFileWriterImpl) Write(p []byte) (int, error) {
+ err := (error)(nil)
+
+ // Check to see if file needs to rotate
+ if err = maybeCloseCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Ensure we have a current writer
+ if err = ensureCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Perform the write
+ return base.CurrentWriter.Write(p)
+}
+
+func maybeCloseCurrentWriter(base *rotatingFileWriterImpl) error {
+ if base.CurrentWriter != nil {
+ fileInfo, err := base.CurrentWriter.Stat()
+ if err != nil {
+ return err
+ }
+ if fileInfo.Size() >= int64(base.FileSizeMaxKb)*1024 {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = nil
+ return removeOldFiles(base)
+ }
+ }
+ return nil
+}
+
+func ensureCurrentWriter(base *rotatingFileWriterImpl) error {
+ const (
+ permissions = 0666 // Required to be full writable on Windows
we can open it again
+ createFlags = os.O_APPEND | os.O_CREATE | os.O_WRONLY
+ appendFlags = os.O_APPEND | os.O_WRONLY
+ )
+
+ if base.CurrentWriter == nil {
+ // check for a candidate file that is not full
+ ok, fullPathLastFile, err := getCandidateLogFileName(base)
+ if err != nil {
+ return err
+ }
+ // attempt to open exising candidate file
+ if ok {
+ currentWriter, err := os.OpenFile(fullPathLastFile,
appendFlags, permissions)
+ if err == nil {
+ base.CurrentWriter = currentWriter
+ return nil
+ }
+ // unable to open candidate file (locked?)
+ }
+
+ // open a new file
+ fullPath := buildNewFileName(base)
+ currentWriter, err := os.OpenFile(fullPath, createFlags,
permissions)
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = currentWriter
+ }
+ return nil
+}
+
+func buildNewFileName(base *rotatingFileWriterImpl) string {
+ now := time.Now().UTC()
+ safeTimeStamp :=
strings.ReplaceAll(strings.ReplaceAll(strings.Replace(strings.Replace(now.Format(time.RFC3339Nano),
"Z", "", 1), "T", "-", 1), ":", "-"), ".", "-")
+ fileName := base.LogNamePrefix + "-" + safeTimeStamp +
defaultTraceFileExt
+ fullPath := filepath.Join(base.TracingFolderPath, fileName)
+ return fullPath
+}
+
+func getCandidateLogFileName(base *rotatingFileWriterImpl) (bool, string,
error) {
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil || len(logFiles) < 1 {
+ return false, "", err
+ }
+ lastLogFile := logFiles[len(logFiles)-1]
+ fileSizeMaxBytes := base.FileSizeMaxKb * 1024
+ fileInfo, err := lastLogFile.Info()
+ if err != nil || fileInfo.Size() >= fileSizeMaxBytes {
+ return false, "", err
+ }
+ // Return full path name
+ fullPathFileName := filepath.Join(base.TracingFolderPath,
lastLogFile.Name())
+ return true, fullPathFileName, nil
+}
+
+func removeOldFiles(base *rotatingFileWriterImpl) error {
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return nil
+ }
+ len := len(logFiles)
+ if len > int(base.FileCountMax) {
+ numToRemove := len - int(base.FileCountMax)
+ for index, file := range logFiles {
+ if index < numToRemove {
+ err :=
os.Remove(filepath.Join(base.TracingFolderPath, file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ }
+ }
+ return nil
+}
+
+func getLogFiles(folderPath string, prefix string, ext string) ([]os.DirEntry,
error) {
+ files, err := os.ReadDir(folderPath)
+ if err != nil {
+ return nil, err
+ }
+
+ var logFiles []os.DirEntry
+ for _, file := range files {
+ baseName := filepath.Base(file.Name())
+ if strings.HasPrefix(baseName, prefix) &&
strings.HasSuffix(baseName, ext) {
+ logFiles = append(logFiles, file)
+ }
+ }
Review Comment:
could we use `fs.Glob`?
##########
go/adbc/driver/snowflake/statement.go:
##########
@@ -465,19 +475,33 @@ func (st *statement) executeIngest(ctx context.Context)
(int64, error) {
// of rows affected if known, otherwise it will be -1.
//
// This invalidates any prior result sets on this statement.
-func (st *statement) ExecuteQuery(ctx context.Context) (array.RecordReader,
int64, error) {
+func (st *statement) ExecuteQuery(ctx context.Context) (reader
array.RecordReader, nrows int64, err error) {
+ reader = nil
+ nrows = -1
+ err = nil
+
ctx = st.setQueryContext(ctx)
+ _, span := st.StartSpan(ctx, "ExecuteQuery")
Review Comment:
swap the order of these lines and don't ignore the return from `StartSpan`
otherwise the span won't propagate properly.
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
Review Comment:
can this be part of `newConfig` instead?
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
+ exporterType, _ := tryParseTraceExporterType(exporterName)
+ switch exporterType {
+ case TraceExporterConsole:
+ exporter, _ = newStdoutTraceExporter()
+ case TraceExporterOtlp:
+ exporter, _ = newOtlpTraceExporter(context.Background())
+ case TraceExporterAdbcFile:
+ exporter, _ = newAdbcFileExporter(driverName)
+ }
+
+ fullyQualifiedDriverName := driverNamespace + "." + driverName
+ if exporter != nil {
+ tracerProvider, err := newTracerProvider(exporter)
+ if err != nil {
+ panic(err)
+ }
+ base.Base().tracerShutdownFunc = tracerProvider.Shutdown
+ tracer = tracerProvider.Tracer(
+ fullyQualifiedDriverName,
+ trace.WithInstrumentationVersion(driverVersion),
+ trace.WithSchemaURL(semconv.SchemaURL),
+ )
+ } else {
+ tracer = otel.Tracer(fullyQualifiedDriverName)
+ }
+ base.Base().Tracer = tracer
+}
+
+func tryParseTraceExporterType(value string) (traceExporterType, bool) {
+ if te, ok := traceExporterNames[value]; ok {
+ return te, true
+ }
+ return TraceExporterNone, false
+}
+
+func getDriverVersion(driverInfo *DriverInfo) string {
+ var unknownDriverVersion = "unknown"
+ value, ok := driverInfo.GetInfoForInfoCode(adbc.InfoDriverVersion)
+ if !ok || value == nil {
+ return unknownDriverVersion
+ }
+ if driverVersion, ok := value.(string); ok {
+ return driverVersion
+ }
+ return unknownDriverVersion
+}
+
+func newOtlpTraceExporter(ctx context.Context) (*otlptrace.Exporter, error) {
+ return otlptracegrpc.New(
+ ctx,
+ otlptracegrpc.WithEndpoint("localhost:4317"),
+ otlptracegrpc.WithInsecure(),
+ otlptracegrpc.WithRetry(otlptracegrpc.RetryConfig{
+ Enabled: true,
+ InitialInterval: 5 * time.Second,
+ MaxInterval: 30 * time.Second,
+ // MaxAttempts: 5,
+ }),
+ )
Review Comment:
do we need to provide some way to configure this?
##########
go/adbc/driver/internal/driverbase/connection.go:
##########
@@ -242,6 +292,9 @@ func (base *ConnectionImplBase) SetOption(key string, val
string) error {
switch key {
Review Comment:
We need to be consistent. If `GetOption` is going to do
`strings.ToLower(strings.TrimSpace(key))` then `SetOption` should do the same,
or neither should
##########
go/adbc/driver/internal/driverbase/connection.go:
##########
@@ -553,8 +637,7 @@ func (cnxn *connection) Close() error {
if err == nil {
cnxn.Base().Closed = true
}
-
- return err
+ return nil
Review Comment:
why this change?
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
+ exporterType, _ := tryParseTraceExporterType(exporterName)
+ switch exporterType {
+ case TraceExporterConsole:
+ exporter, _ = newStdoutTraceExporter()
+ case TraceExporterOtlp:
+ exporter, _ = newOtlpTraceExporter(context.Background())
+ case TraceExporterAdbcFile:
+ exporter, _ = newAdbcFileExporter(driverName)
+ }
+
+ fullyQualifiedDriverName := driverNamespace + "." + driverName
+ if exporter != nil {
+ tracerProvider, err := newTracerProvider(exporter)
+ if err != nil {
+ panic(err)
Review Comment:
we should return the error, not panic
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
Review Comment:
it's not necessary to explicitly initialize it as `nil`, that should be the
zero value.
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
Review Comment:
using the more concise version above, this would become
```go
for _, opt := range options {
opt(&cfg)
}
```
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
+ exporterType, _ := tryParseTraceExporterType(exporterName)
+ switch exporterType {
+ case TraceExporterConsole:
+ exporter, _ = newStdoutTraceExporter()
+ case TraceExporterOtlp:
+ exporter, _ = newOtlpTraceExporter(context.Background())
+ case TraceExporterAdbcFile:
+ exporter, _ = newAdbcFileExporter(driverName)
+ }
+
+ fullyQualifiedDriverName := driverNamespace + "." + driverName
+ if exporter != nil {
+ tracerProvider, err := newTracerProvider(exporter)
+ if err != nil {
+ panic(err)
+ }
+ base.Base().tracerShutdownFunc = tracerProvider.Shutdown
+ tracer = tracerProvider.Tracer(
+ fullyQualifiedDriverName,
+ trace.WithInstrumentationVersion(driverVersion),
+ trace.WithSchemaURL(semconv.SchemaURL),
+ )
+ } else {
+ tracer = otel.Tracer(fullyQualifiedDriverName)
+ }
+ base.Base().Tracer = tracer
+}
+
+func tryParseTraceExporterType(value string) (traceExporterType, bool) {
+ if te, ok := traceExporterNames[value]; ok {
+ return te, true
+ }
+ return TraceExporterNone, false
+}
+
+func getDriverVersion(driverInfo *DriverInfo) string {
+ var unknownDriverVersion = "unknown"
+ value, ok := driverInfo.GetInfoForInfoCode(adbc.InfoDriverVersion)
+ if !ok || value == nil {
+ return unknownDriverVersion
+ }
+ if driverVersion, ok := value.(string); ok {
+ return driverVersion
+ }
+ return unknownDriverVersion
+}
+
+func newOtlpTraceExporter(ctx context.Context) (*otlptrace.Exporter, error) {
+ return otlptracegrpc.New(
+ ctx,
+ otlptracegrpc.WithEndpoint("localhost:4317"),
+ otlptracegrpc.WithInsecure(),
+ otlptracegrpc.WithRetry(otlptracegrpc.RetryConfig{
+ Enabled: true,
+ InitialInterval: 5 * time.Second,
+ MaxInterval: 30 * time.Second,
+ // MaxAttempts: 5,
+ }),
+ )
+}
+
+func newAdbcFileExporter(driverName string) (exporter *stdouttrace.Exporter,
err error) {
+ var fw io.Writer
+
+ fullyQualifiedDriverName := strings.ToLower(driverNamespace + "." +
driverName)
+ if fw, err =
NewRotatingFileWriter(WithLogNamePrefix(fullyQualifiedDriverName)); err != nil {
+ return nil, err
+ }
+ if exporter, err = stdouttrace.New(stdouttrace.WithWriter(fw)); err !=
nil {
+ return nil, err
+ }
+ return exporter, nil
Review Comment:
could we simplify it as the following?
```go
fw, err := NewRotatingFileWriter(WithLogNamePrefix(fullyQualifiedDriverName))
if err != nil {
return nil, err
}
return stdouttrace.New(stdouttrace.WithWriter(fw))
```
##########
go/adbc/driver/internal/driverbase/connection.go:
##########
@@ -222,7 +232,47 @@ func (base *ConnectionImplBase) ReadPartition(ctx
context.Context, serializedPar
return nil, base.ErrorHelper.Errorf(adbc.StatusNotImplemented,
"ReadPartition")
}
+func MaybeAddTraceParent(ctx context.Context, cnxn adbc.OTelTracing, st
adbc.OTelTracing) context.Context {
+ var hasTraceParent = false
+ var traceParentStr = ""
+ if st != nil && st.GetTraceParent() != "" {
+ traceParentStr = st.GetTraceParent()
+ hasTraceParent = true
+ } else if cnxn != nil && cnxn.GetTraceParent() != "" {
+ traceParentStr = cnxn.GetTraceParent()
+ hasTraceParent = true
+ }
+ if hasTraceParent {
+ spanContext, err := parseTraceparent(ctx, traceParentStr)
+ if err != nil {
+ return ctx
+ }
Review Comment:
should we log the error somehow?
##########
go/adbc/driver/internal/driverbase/connection.go:
##########
@@ -222,7 +232,47 @@ func (base *ConnectionImplBase) ReadPartition(ctx
context.Context, serializedPar
return nil, base.ErrorHelper.Errorf(adbc.StatusNotImplemented,
"ReadPartition")
}
+func MaybeAddTraceParent(ctx context.Context, cnxn adbc.OTelTracing, st
adbc.OTelTracing) context.Context {
+ var hasTraceParent = false
+ var traceParentStr = ""
+ if st != nil && st.GetTraceParent() != "" {
+ traceParentStr = st.GetTraceParent()
+ hasTraceParent = true
+ } else if cnxn != nil && cnxn.GetTraceParent() != "" {
+ traceParentStr = cnxn.GetTraceParent()
+ hasTraceParent = true
+ }
+ if hasTraceParent {
Review Comment:
we can probably eliminate the `hasTraceParent` variable entirely and just do
`if traceParentStr != "" {`
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
+ exporterType, _ := tryParseTraceExporterType(exporterName)
+ switch exporterType {
+ case TraceExporterConsole:
+ exporter, _ = newStdoutTraceExporter()
+ case TraceExporterOtlp:
+ exporter, _ = newOtlpTraceExporter(context.Background())
+ case TraceExporterAdbcFile:
+ exporter, _ = newAdbcFileExporter(driverName)
+ }
+
+ fullyQualifiedDriverName := driverNamespace + "." + driverName
+ if exporter != nil {
+ tracerProvider, err := newTracerProvider(exporter)
+ if err != nil {
+ panic(err)
+ }
+ base.Base().tracerShutdownFunc = tracerProvider.Shutdown
+ tracer = tracerProvider.Tracer(
+ fullyQualifiedDriverName,
+ trace.WithInstrumentationVersion(driverVersion),
+ trace.WithSchemaURL(semconv.SchemaURL),
+ )
+ } else {
+ tracer = otel.Tracer(fullyQualifiedDriverName)
+ }
+ base.Base().Tracer = tracer
+}
+
+func tryParseTraceExporterType(value string) (traceExporterType, bool) {
+ if te, ok := traceExporterNames[value]; ok {
+ return te, true
+ }
+ return TraceExporterNone, false
+}
+
+func getDriverVersion(driverInfo *DriverInfo) string {
+ var unknownDriverVersion = "unknown"
Review Comment:
`const`?
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -136,4 +200,113 @@ func (db *database) SetLogger(logger *slog.Logger) {
}
}
+func (base *database) InitTracing(driverName string, driverVersion string) {
+ base.Base().InitTracing(driverName, driverVersion)
+}
+
+func (base *DatabaseImplBase) InitTracing(driverName string, driverVersion
string) {
+ var exporter sdktrace.SpanExporter = nil
+ var tracer trace.Tracer
+
+ exporterName := os.Getenv("OTEL_TRACES_EXPORTER")
+ exporterType, _ := tryParseTraceExporterType(exporterName)
+ switch exporterType {
+ case TraceExporterConsole:
+ exporter, _ = newStdoutTraceExporter()
+ case TraceExporterOtlp:
+ exporter, _ = newOtlpTraceExporter(context.Background())
+ case TraceExporterAdbcFile:
+ exporter, _ = newAdbcFileExporter(driverName)
+ }
+
+ fullyQualifiedDriverName := driverNamespace + "." + driverName
+ if exporter != nil {
+ tracerProvider, err := newTracerProvider(exporter)
+ if err != nil {
+ panic(err)
+ }
+ base.Base().tracerShutdownFunc = tracerProvider.Shutdown
+ tracer = tracerProvider.Tracer(
+ fullyQualifiedDriverName,
+ trace.WithInstrumentationVersion(driverVersion),
+ trace.WithSchemaURL(semconv.SchemaURL),
+ )
+ } else {
+ tracer = otel.Tracer(fullyQualifiedDriverName)
+ }
+ base.Base().Tracer = tracer
+}
+
+func tryParseTraceExporterType(value string) (traceExporterType, bool) {
+ if te, ok := traceExporterNames[value]; ok {
+ return te, true
+ }
+ return TraceExporterNone, false
+}
+
+func getDriverVersion(driverInfo *DriverInfo) string {
+ var unknownDriverVersion = "unknown"
+ value, ok := driverInfo.GetInfoForInfoCode(adbc.InfoDriverVersion)
+ if !ok || value == nil {
+ return unknownDriverVersion
+ }
+ if driverVersion, ok := value.(string); ok {
+ return driverVersion
+ }
+ return unknownDriverVersion
+}
+
+func newOtlpTraceExporter(ctx context.Context) (*otlptrace.Exporter, error) {
+ return otlptracegrpc.New(
+ ctx,
+ otlptracegrpc.WithEndpoint("localhost:4317"),
+ otlptracegrpc.WithInsecure(),
+ otlptracegrpc.WithRetry(otlptracegrpc.RetryConfig{
+ Enabled: true,
+ InitialInterval: 5 * time.Second,
+ MaxInterval: 30 * time.Second,
+ // MaxAttempts: 5,
+ }),
+ )
+}
+
+func newAdbcFileExporter(driverName string) (exporter *stdouttrace.Exporter,
err error) {
+ var fw io.Writer
+
+ fullyQualifiedDriverName := strings.ToLower(driverNamespace + "." +
driverName)
+ if fw, err =
NewRotatingFileWriter(WithLogNamePrefix(fullyQualifiedDriverName)); err != nil {
+ return nil, err
+ }
+ if exporter, err = stdouttrace.New(stdouttrace.WithWriter(fw)); err !=
nil {
+ return nil, err
+ }
+ return exporter, nil
+}
+
+func newStdoutTraceExporter() (exporter *stdouttrace.Exporter, err error) {
+ if exporter, err = stdouttrace.New(); err != nil {
+ return nil, err
+ }
+ return exporter, nil
+}
Review Comment:
can't any callers of this just replace it by calling `stdouttrace.New()`?
This function seems unnecessary
##########
go/adbc/driver/internal/driverbase/database.go:
##########
@@ -19,12 +19,55 @@ package driverbase
import (
"context"
+ "io"
"log/slog"
+ "os"
+ "strings"
+ "time"
"github.com/apache/arrow-adbc/go/adbc"
"github.com/apache/arrow-go/v18/arrow/memory"
+ "go.opentelemetry.io/otel"
+ "go.opentelemetry.io/otel/exporters/otlp/otlptrace"
+ "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
+ "go.opentelemetry.io/otel/exporters/stdout/stdouttrace"
+ "go.opentelemetry.io/otel/sdk/resource"
+ sdktrace "go.opentelemetry.io/otel/sdk/trace"
+ semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
+ "go.opentelemetry.io/otel/trace"
)
+const driverNamespace = "apache.arrow.adbc"
+
+type traceExporterType int
+
+const (
+ TraceExporterNone traceExporterType = iota
+ TraceExporterOtlp
+ TraceExporterConsole
+ TraceExporterAdbcFile
+)
+
+var traceExporterNames = map[string]traceExporterType{
+ string(adbc.TelemetryExporterNone): TraceExporterNone,
+ string(adbc.TelemetryExporterOtlp): TraceExporterOtlp,
+ string(adbc.TelemetryExporterConsole): TraceExporterConsole,
+ string(adbc.TelemetryExporterAdbcFile): TraceExporterAdbcFile,
+}
+
+func (te traceExporterType) String() string {
+ return [...]string{
+ string(adbc.TelemetryExporterNone),
+ string(adbc.TelemetryExporterOtlp),
+ string(adbc.TelemetryExporterConsole),
+ string(adbc.TelemetryExporterAdbcFile),
+ }[te]
+}
+
+func (te traceExporterType) EnumIndex() int {
+ return int(te)
+}
Review Comment:
why is this needed as opposed to a user just doing `int(te)` themselves?
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
+
+ // Test if we can create/write a file in the traces folder
+ tempFile, err := os.CreateTemp(tracingFolderPath, logNamePrefix)
+ if err != nil {
+ return nil, err
+ }
+ defer func() {
+ _ = tempFile.Close()
+ _ = os.Remove(tempFile.Name())
+ }()
+ _, err = tempFile.WriteString("file started")
+ if err != nil {
+ return nil, err
+ }
+
+ return &rotatingFileWriterImpl{
+ TracingFolderPath: tracingFolderPath,
+ LogNamePrefix: logNamePrefix,
+ FileSizeMaxKb: fileSizeMaxKb,
+ FileCountMax: fileCountMax,
+ CurrentWriter: nil,
+ }, nil
+}
+
+func getDefaultTracingFolderPath() (string, error) {
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ fullPath := filepath.Join(userHome, ".adbc", "traces")
+ return fullPath, nil
+}
+
+// Closes the rotating file write
+func (base *rotatingFileWriterImpl) Close() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ base.CurrentWriter = nil
+ return err
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Clear() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ }
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return err
+ }
+ for _, file := range logFiles {
+ err := os.Remove(filepath.Join(base.TracingFolderPath,
file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Stat() (fs.FileInfo, error) {
+ if base.CurrentWriter != nil {
+ return base.CurrentWriter.Stat()
+ }
+ return nil, errors.New("no trace file is open")
+}
+
+func (base *rotatingFileWriterImpl) GetTracingFolderPath() string {
+ return base.TracingFolderPath
+}
+
+func (base *rotatingFileWriterImpl) GetLogNamePrefix() string {
+ return base.LogNamePrefix
+}
+
+func (base *rotatingFileWriterImpl) GetFileSizeMaxKb() int64 {
+ return base.FileSizeMaxKb
+}
+
+func (base *rotatingFileWriterImpl) GetFileCountMax() int {
+ return base.FileCountMax
+}
+
+func (base *rotatingFileWriterImpl) Write(p []byte) (int, error) {
+ err := (error)(nil)
+
+ // Check to see if file needs to rotate
+ if err = maybeCloseCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Ensure we have a current writer
+ if err = ensureCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Perform the write
+ return base.CurrentWriter.Write(p)
+}
+
+func maybeCloseCurrentWriter(base *rotatingFileWriterImpl) error {
+ if base.CurrentWriter != nil {
+ fileInfo, err := base.CurrentWriter.Stat()
+ if err != nil {
+ return err
+ }
+ if fileInfo.Size() >= int64(base.FileSizeMaxKb)*1024 {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = nil
+ return removeOldFiles(base)
+ }
+ }
+ return nil
+}
+
+func ensureCurrentWriter(base *rotatingFileWriterImpl) error {
+ const (
+ permissions = 0666 // Required to be full writable on Windows
we can open it again
+ createFlags = os.O_APPEND | os.O_CREATE | os.O_WRONLY
+ appendFlags = os.O_APPEND | os.O_WRONLY
+ )
+
+ if base.CurrentWriter == nil {
+ // check for a candidate file that is not full
+ ok, fullPathLastFile, err := getCandidateLogFileName(base)
+ if err != nil {
+ return err
+ }
+ // attempt to open exising candidate file
+ if ok {
+ currentWriter, err := os.OpenFile(fullPathLastFile,
appendFlags, permissions)
+ if err == nil {
+ base.CurrentWriter = currentWriter
+ return nil
+ }
+ // unable to open candidate file (locked?)
+ }
+
+ // open a new file
+ fullPath := buildNewFileName(base)
+ currentWriter, err := os.OpenFile(fullPath, createFlags,
permissions)
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = currentWriter
+ }
+ return nil
+}
+
+func buildNewFileName(base *rotatingFileWriterImpl) string {
+ now := time.Now().UTC()
+ safeTimeStamp :=
strings.ReplaceAll(strings.ReplaceAll(strings.Replace(strings.Replace(now.Format(time.RFC3339Nano),
"Z", "", 1), "T", "-", 1), ":", "-"), ".", "-")
+ fileName := base.LogNamePrefix + "-" + safeTimeStamp +
defaultTraceFileExt
+ fullPath := filepath.Join(base.TracingFolderPath, fileName)
+ return fullPath
+}
+
+func getCandidateLogFileName(base *rotatingFileWriterImpl) (bool, string,
error) {
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil || len(logFiles) < 1 {
+ return false, "", err
+ }
+ lastLogFile := logFiles[len(logFiles)-1]
+ fileSizeMaxBytes := base.FileSizeMaxKb * 1024
+ fileInfo, err := lastLogFile.Info()
+ if err != nil || fileInfo.Size() >= fileSizeMaxBytes {
+ return false, "", err
+ }
+ // Return full path name
+ fullPathFileName := filepath.Join(base.TracingFolderPath,
lastLogFile.Name())
+ return true, fullPathFileName, nil
Review Comment:
what's the point of the bool? Seems like just checking `err != nil` is
sufficient for a caller.
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
Review Comment:
same as above, move all these to `newConfig` after the options are applied.
##########
go/adbc/driver/snowflake/statement.go:
##########
@@ -465,19 +475,33 @@ func (st *statement) executeIngest(ctx context.Context)
(int64, error) {
// of rows affected if known, otherwise it will be -1.
//
// This invalidates any prior result sets on this statement.
-func (st *statement) ExecuteQuery(ctx context.Context) (array.RecordReader,
int64, error) {
+func (st *statement) ExecuteQuery(ctx context.Context) (reader
array.RecordReader, nrows int64, err error) {
+ reader = nil
+ nrows = -1
+ err = nil
Review Comment:
the only one of these that is necessary is `nrows = -1` the others are the
zero values and aren't necessary to have.
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
Review Comment:
have `WithFileSizeMaxKB` do `cfg.FileSizeMaxKb = max(defaultFileSizeMaxKb,
maxKb)`
same for the File Count Max.
##########
go/adbc/driver/snowflake/statement.go:
##########
@@ -59,6 +63,8 @@ type statement struct {
bound arrow.Record
streamBind array.RecordReader
+
+ traceParent string
Review Comment:
should `traceParent` be in `StatementImplBase`?
##########
go/adbc/driver/snowflake/statement.go:
##########
@@ -499,41 +523,56 @@ func (st *statement) ExecuteQuery(ctx context.Context)
(array.RecordReader, int6
st.streamBind = nil
rdr := concatReader{}
- err := rdr.Init(&bind)
+ err = rdr.Init(&bind)
Review Comment:
you named the record reader, lets be consistent and use `reader =
concatReader{}` instead of `rdr := concatReader{}`
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
+
+ // Test if we can create/write a file in the traces folder
+ tempFile, err := os.CreateTemp(tracingFolderPath, logNamePrefix)
+ if err != nil {
+ return nil, err
+ }
+ defer func() {
+ _ = tempFile.Close()
+ _ = os.Remove(tempFile.Name())
+ }()
+ _, err = tempFile.WriteString("file started")
+ if err != nil {
+ return nil, err
+ }
+
+ return &rotatingFileWriterImpl{
+ TracingFolderPath: tracingFolderPath,
+ LogNamePrefix: logNamePrefix,
+ FileSizeMaxKb: fileSizeMaxKb,
+ FileCountMax: fileCountMax,
+ CurrentWriter: nil,
+ }, nil
+}
+
+func getDefaultTracingFolderPath() (string, error) {
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ fullPath := filepath.Join(userHome, ".adbc", "traces")
+ return fullPath, nil
+}
+
+// Closes the rotating file write
+func (base *rotatingFileWriterImpl) Close() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ base.CurrentWriter = nil
+ return err
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Clear() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ }
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return err
+ }
+ for _, file := range logFiles {
+ err := os.Remove(filepath.Join(base.TracingFolderPath,
file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Stat() (fs.FileInfo, error) {
+ if base.CurrentWriter != nil {
+ return base.CurrentWriter.Stat()
+ }
+ return nil, errors.New("no trace file is open")
+}
+
+func (base *rotatingFileWriterImpl) GetTracingFolderPath() string {
+ return base.TracingFolderPath
+}
+
+func (base *rotatingFileWriterImpl) GetLogNamePrefix() string {
+ return base.LogNamePrefix
+}
+
+func (base *rotatingFileWriterImpl) GetFileSizeMaxKb() int64 {
+ return base.FileSizeMaxKb
+}
+
+func (base *rotatingFileWriterImpl) GetFileCountMax() int {
+ return base.FileCountMax
+}
+
+func (base *rotatingFileWriterImpl) Write(p []byte) (int, error) {
+ err := (error)(nil)
+
+ // Check to see if file needs to rotate
+ if err = maybeCloseCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Ensure we have a current writer
+ if err = ensureCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Perform the write
+ return base.CurrentWriter.Write(p)
+}
+
+func maybeCloseCurrentWriter(base *rotatingFileWriterImpl) error {
+ if base.CurrentWriter != nil {
+ fileInfo, err := base.CurrentWriter.Stat()
+ if err != nil {
+ return err
+ }
+ if fileInfo.Size() >= int64(base.FileSizeMaxKb)*1024 {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = nil
+ return removeOldFiles(base)
+ }
+ }
+ return nil
+}
+
+func ensureCurrentWriter(base *rotatingFileWriterImpl) error {
+ const (
+ permissions = 0666 // Required to be full writable on Windows
we can open it again
+ createFlags = os.O_APPEND | os.O_CREATE | os.O_WRONLY
+ appendFlags = os.O_APPEND | os.O_WRONLY
+ )
+
+ if base.CurrentWriter == nil {
+ // check for a candidate file that is not full
+ ok, fullPathLastFile, err := getCandidateLogFileName(base)
+ if err != nil {
+ return err
+ }
+ // attempt to open exising candidate file
+ if ok {
+ currentWriter, err := os.OpenFile(fullPathLastFile,
appendFlags, permissions)
+ if err == nil {
+ base.CurrentWriter = currentWriter
+ return nil
+ }
+ // unable to open candidate file (locked?)
Review Comment:
should we check the error and see if it's `ErrInvalid`, `ErrPermission`,
`ErrNotExists` etc?
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
+
+ // Test if we can create/write a file in the traces folder
+ tempFile, err := os.CreateTemp(tracingFolderPath, logNamePrefix)
+ if err != nil {
+ return nil, err
+ }
+ defer func() {
+ _ = tempFile.Close()
+ _ = os.Remove(tempFile.Name())
+ }()
+ _, err = tempFile.WriteString("file started")
+ if err != nil {
+ return nil, err
+ }
+
+ return &rotatingFileWriterImpl{
+ TracingFolderPath: tracingFolderPath,
+ LogNamePrefix: logNamePrefix,
+ FileSizeMaxKb: fileSizeMaxKb,
+ FileCountMax: fileCountMax,
+ CurrentWriter: nil,
+ }, nil
+}
+
+func getDefaultTracingFolderPath() (string, error) {
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ fullPath := filepath.Join(userHome, ".adbc", "traces")
+ return fullPath, nil
+}
+
+// Closes the rotating file write
+func (base *rotatingFileWriterImpl) Close() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ base.CurrentWriter = nil
+ return err
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Clear() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ }
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return err
+ }
+ for _, file := range logFiles {
+ err := os.Remove(filepath.Join(base.TracingFolderPath,
file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Stat() (fs.FileInfo, error) {
+ if base.CurrentWriter != nil {
+ return base.CurrentWriter.Stat()
+ }
+ return nil, errors.New("no trace file is open")
+}
+
+func (base *rotatingFileWriterImpl) GetTracingFolderPath() string {
+ return base.TracingFolderPath
+}
+
+func (base *rotatingFileWriterImpl) GetLogNamePrefix() string {
+ return base.LogNamePrefix
+}
+
+func (base *rotatingFileWriterImpl) GetFileSizeMaxKb() int64 {
+ return base.FileSizeMaxKb
+}
+
+func (base *rotatingFileWriterImpl) GetFileCountMax() int {
+ return base.FileCountMax
+}
+
+func (base *rotatingFileWriterImpl) Write(p []byte) (int, error) {
+ err := (error)(nil)
+
+ // Check to see if file needs to rotate
+ if err = maybeCloseCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Ensure we have a current writer
+ if err = ensureCurrentWriter(base); err != nil {
+ return 0, err
+ }
+
+ // Perform the write
+ return base.CurrentWriter.Write(p)
+}
+
+func maybeCloseCurrentWriter(base *rotatingFileWriterImpl) error {
+ if base.CurrentWriter != nil {
+ fileInfo, err := base.CurrentWriter.Stat()
+ if err != nil {
+ return err
+ }
+ if fileInfo.Size() >= int64(base.FileSizeMaxKb)*1024 {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = nil
+ return removeOldFiles(base)
+ }
+ }
+ return nil
+}
+
+func ensureCurrentWriter(base *rotatingFileWriterImpl) error {
+ const (
+ permissions = 0666 // Required to be full writable on Windows
we can open it again
+ createFlags = os.O_APPEND | os.O_CREATE | os.O_WRONLY
+ appendFlags = os.O_APPEND | os.O_WRONLY
+ )
+
+ if base.CurrentWriter == nil {
+ // check for a candidate file that is not full
+ ok, fullPathLastFile, err := getCandidateLogFileName(base)
+ if err != nil {
+ return err
+ }
+ // attempt to open exising candidate file
+ if ok {
+ currentWriter, err := os.OpenFile(fullPathLastFile,
appendFlags, permissions)
+ if err == nil {
+ base.CurrentWriter = currentWriter
+ return nil
+ }
+ // unable to open candidate file (locked?)
+ }
+
+ // open a new file
+ fullPath := buildNewFileName(base)
+ currentWriter, err := os.OpenFile(fullPath, createFlags,
permissions)
+ if err != nil {
+ return err
+ }
+ base.CurrentWriter = currentWriter
+ }
+ return nil
+}
+
+func buildNewFileName(base *rotatingFileWriterImpl) string {
+ now := time.Now().UTC()
+ safeTimeStamp :=
strings.ReplaceAll(strings.ReplaceAll(strings.Replace(strings.Replace(now.Format(time.RFC3339Nano),
"Z", "", 1), "T", "-", 1), ":", "-"), ".", "-")
+ fileName := base.LogNamePrefix + "-" + safeTimeStamp +
defaultTraceFileExt
+ fullPath := filepath.Join(base.TracingFolderPath, fileName)
+ return fullPath
+}
+
+func getCandidateLogFileName(base *rotatingFileWriterImpl) (bool, string,
error) {
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil || len(logFiles) < 1 {
+ return false, "", err
+ }
+ lastLogFile := logFiles[len(logFiles)-1]
+ fileSizeMaxBytes := base.FileSizeMaxKb * 1024
+ fileInfo, err := lastLogFile.Info()
+ if err != nil || fileInfo.Size() >= fileSizeMaxBytes {
+ return false, "", err
+ }
+ // Return full path name
+ fullPathFileName := filepath.Join(base.TracingFolderPath,
lastLogFile.Name())
+ return true, fullPathFileName, nil
+}
+
+func removeOldFiles(base *rotatingFileWriterImpl) error {
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return nil
+ }
+ len := len(logFiles)
+ if len > int(base.FileCountMax) {
+ numToRemove := len - int(base.FileCountMax)
+ for index, file := range logFiles {
+ if index < numToRemove {
+ err :=
os.Remove(filepath.Join(base.TracingFolderPath, file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ }
Review Comment:
we can get rid of the branch:
```go
for _, file := range logFiles[:numToRemove] {
```
##########
go/adbc/driver/internal/driverbase/rotating_file_writer.go:
##########
@@ -0,0 +1,404 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package driverbase
+
+import (
+ "errors"
+ "io"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "strings"
+ "time"
+)
+
+const (
+ defaultTracingFolderPath = ""
+ defaultLogNamePrefix = "apache.adbc.go"
+ defaultFileSizeMaxKb = int64(1024)
+ defaultFileCountMax = 100
+ defaultTraceFileExt = ".jsonl"
+)
+
+// Holds the configuration from the options
+type config struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+}
+
+// An option for the RotatingFileWriter
+type RotatingFileWriterOption interface {
+ apply(config) config
+}
+
+// Adds the TracingFolderPath option
+func WithTracingFolderPath(tracingFolderPath string) RotatingFileWriterOption {
+ return tracingFolderPathOption{tracingFolderPath}
+}
+
+type tracingFolderPathOption struct {
+ TracingFolderPath string
+}
+
+func (o tracingFolderPathOption) apply(cfg config) config {
+ cfg.TracingFolderPath = o.TracingFolderPath
+ return cfg
+}
+
+// Adds the LogNamePrefix option
+func WithLogNamePrefix(logNamePrefix string) RotatingFileWriterOption {
+ return logNamePrefixOption{logNamePrefix}
+}
+
+type logNamePrefixOption struct {
+ LogNamePrefix string
+}
+
+func (o logNamePrefixOption) apply(cfg config) config {
+ cfg.LogNamePrefix = o.LogNamePrefix
+ return cfg
+}
+
+// Adds the FileSizeMaxKb option
+func WithFileSizeMaxKb(fileSizeMaxKb int64) RotatingFileWriterOption {
+ return fileSizeMaxKbOption{fileSizeMaxKb}
+}
+
+type fileSizeMaxKbOption struct {
+ FileSizeMaxKb int64
+}
+
+func (o fileSizeMaxKbOption) apply(cfg config) config {
+ cfg.FileSizeMaxKb = o.FileSizeMaxKb
+ return cfg
+}
+
+// Adds the FileCountMax option
+func WithFileCountMax(fileCountMax int) RotatingFileWriterOption {
+ return fileCountMaxOption{fileCountMax}
+}
+
+type fileCountMaxOption struct {
+ FileCountMax int
+}
+
+func (o fileCountMaxOption) apply(cfg config) config {
+ cfg.FileCountMax = o.FileCountMax
+ return cfg
+}
+
+func newConfig(options ...RotatingFileWriterOption) config {
+ cfg := config{
+ TracingFolderPath: defaultTracingFolderPath,
+ LogNamePrefix: defaultLogNamePrefix,
+ FileSizeMaxKb: defaultFileSizeMaxKb,
+ FileCountMax: defaultFileCountMax,
+ }
+ for _, opt := range options {
+ // Applies the option value to the configuration
+ cfg = opt.apply(cfg)
+ }
+ return cfg
+}
+
+// A rotating file writer that writes bytes to new trace files into a given
TracingFolderPath until the trace file exceeds FileSizeMaxKb in size.
+// Then a new trace file is created. If the number archived trace files
exceeds FileCountMax, then the oldest file(s) are removed.
+// The files are named with the following pattern "<LogNamePrefix>-<current
date/time UTC>.jsonl"
+type RotatingFileWriter interface {
+ // Extends
+ io.Writer
+ io.Closer
+
+ // Gets the path to the tracing folder
+ GetTracingFolderPath() string
+ // Gets the prefix for the log file name
+ GetLogNamePrefix() string
+ // Gets the maximum file size for the trace file
+ GetFileSizeMaxKb() int64
+ // Gets the maximum number of archive trace files to keep in the
rotation
+ GetFileCountMax() int
+ // Gets the file stats for the current trace file
+ Stat() (fs.FileInfo, error)
+ // Clears the tracing folder of trace files with the given log file
prefix
+ Clear() error
+}
+
+type rotatingFileWriterImpl struct {
+ TracingFolderPath string
+ LogNamePrefix string
+ FileSizeMaxKb int64
+ FileCountMax int
+ CurrentWriter *os.File
+}
+
+// Creates a new RotatingFileWriter from the given options
+func NewRotatingFileWriter(options ...RotatingFileWriterOption)
(RotatingFileWriter, error) {
+
+ cfg := newConfig(options...)
+
+ var tracingFolderPath = cfg.TracingFolderPath
+ // Ensure default for tracingFolderPath
+ if strings.TrimSpace(tracingFolderPath) == "" {
+ fullPath, err := getDefaultTracingFolderPath()
+ if err != nil {
+ return nil, err
+ }
+ tracingFolderPath = fullPath
+ }
+ // Ensure tracingFolderPath exists
+ err := os.MkdirAll(tracingFolderPath, os.ModePerm)
+ if err != nil {
+ return nil, err
+ }
+
+ var logNamePrefix = cfg.LogNamePrefix
+ // Ensure default for logNamePrefix
+ if strings.TrimSpace(logNamePrefix) == "" {
+ namePrefix := defaultLogNamePrefix
+ logNamePrefix = namePrefix
+ }
+
+ var fileSizeMaxKb = cfg.FileSizeMaxKb
+ // Ensure default for fileSizeMaxKb
+ if fileSizeMaxKb <= 0 {
+ var maxKb = defaultFileSizeMaxKb
+ fileSizeMaxKb = maxKb
+ }
+
+ var fileCountMax = cfg.FileCountMax
+ // Ensure default for fileCountMax
+ if fileCountMax <= 0 {
+ var countMax = defaultFileCountMax
+ fileCountMax = countMax
+ }
+
+ // Test if we can create/write a file in the traces folder
+ tempFile, err := os.CreateTemp(tracingFolderPath, logNamePrefix)
+ if err != nil {
+ return nil, err
+ }
+ defer func() {
+ _ = tempFile.Close()
+ _ = os.Remove(tempFile.Name())
+ }()
+ _, err = tempFile.WriteString("file started")
+ if err != nil {
+ return nil, err
+ }
+
+ return &rotatingFileWriterImpl{
+ TracingFolderPath: tracingFolderPath,
+ LogNamePrefix: logNamePrefix,
+ FileSizeMaxKb: fileSizeMaxKb,
+ FileCountMax: fileCountMax,
+ CurrentWriter: nil,
+ }, nil
+}
+
+func getDefaultTracingFolderPath() (string, error) {
+ userHome, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ fullPath := filepath.Join(userHome, ".adbc", "traces")
+ return fullPath, nil
+}
+
+// Closes the rotating file write
+func (base *rotatingFileWriterImpl) Close() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ base.CurrentWriter = nil
+ return err
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Clear() error {
+ if base.CurrentWriter != nil {
+ err := base.CurrentWriter.Close()
+ if err != nil {
+ return err
+ }
+ }
+ logFiles, err := getLogFiles(base.TracingFolderPath,
base.LogNamePrefix, defaultTraceFileExt)
+ if err != nil {
+ return err
+ }
+ for _, file := range logFiles {
+ err := os.Remove(filepath.Join(base.TracingFolderPath,
file.Name()))
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (base *rotatingFileWriterImpl) Stat() (fs.FileInfo, error) {
+ if base.CurrentWriter != nil {
+ return base.CurrentWriter.Stat()
+ }
+ return nil, errors.New("no trace file is open")
+}
+
+func (base *rotatingFileWriterImpl) GetTracingFolderPath() string {
+ return base.TracingFolderPath
+}
+
+func (base *rotatingFileWriterImpl) GetLogNamePrefix() string {
+ return base.LogNamePrefix
+}
+
+func (base *rotatingFileWriterImpl) GetFileSizeMaxKb() int64 {
+ return base.FileSizeMaxKb
+}
+
+func (base *rotatingFileWriterImpl) GetFileCountMax() int {
+ return base.FileCountMax
+}
+
+func (base *rotatingFileWriterImpl) Write(p []byte) (int, error) {
+ err := (error)(nil)
Review Comment:
remove this line, it's unnecessary, just use `:=` below
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
