Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions cmd/otto/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package main

import (
"context"
"errors"
"log/slog"
"net/http"
"os"
"os/signal"
"strings"
"syscall"
"time"

"github.com/rustyeddy/otto/logging"
"github.com/spf13/cobra"
)

const serverAddr = ":8011"
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The serverAddr constant is hardcoded to ":8011". This should be configurable via a command-line flag to allow users to run the server on different ports. Consider adding a --port or --addr flag to the serve command.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


var (
logLevel string
logFormat string
logOutput string
logFile string
)

var rootCmd = &cobra.Command{
Use: "otto",
Short: "OttO IoT server",
SilenceUsage: true,
SilenceErrors: true,
}

var serveCmd = &cobra.Command{
Use: "serve",
Short: "Start the HTTP server",
RunE: runServe,
}

func init() {
serveCmd.Flags().StringVar(&logLevel, "log-level", logging.DefaultLevel, "Log level (debug, info, warn, error)")
serveCmd.Flags().StringVar(&logFormat, "log-format", logging.DefaultFormat, "Log format (text, json)")
serveCmd.Flags().StringVar(&logOutput, "log-output", logging.DefaultOutput, "Log output (stdout, stderr, file, string)")
serveCmd.Flags().StringVar(&logFile, "log-file", "", "Log file path (required when log-output=file)")
rootCmd.AddCommand(serveCmd)
}

func main() {
if err := rootCmd.Execute(); err != nil {
slog.Error("command failed", "error", err)
os.Exit(1)
}
}

func runServe(cmd *cobra.Command, args []string) error {
if strings.EqualFold(logOutput, "file") && strings.TrimSpace(logFile) == "" {
return errors.New("log-output=file requires --log-file")
}

cfg := logging.Config{
Level: logLevel,
Format: logFormat,
Output: logOutput,
FilePath: logFile,
}

logService, err := logging.NewService(cfg)
if err != nil {
return err
}

mux := http.NewServeMux()
mux.Handle("/api/log", logService)

server := &http.Server{
Addr: serverAddr,
Handler: mux,
}

ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
defer stop()

errCh := make(chan error, 1)
go func() {
errCh <- server.ListenAndServe()
}()

select {
case <-ctx.Done():
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if err := server.Shutdown(shutdownCtx); err != nil {
return err
}
return nil
case err := <-errCh:
if err != nil && !errors.Is(err, http.ErrServerClosed) {
return err
}
return nil
}
}
Comment on lines +1 to +102
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cmd/otto package lacks test coverage. According to the project's comprehensive testing approach (evident from tests throughout the codebase), the serve command should have tests verifying flag parsing, error handling for missing log file, service initialization, and graceful shutdown behavior.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

93 changes: 93 additions & 0 deletions logging/build.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package logging

import (
"bytes"
"fmt"
"io"
"log/slog"
"os"
"strings"
)

// ParseLevel converts a string into a slog.Level.
func ParseLevel(s string) (slog.Level, error) {
value := strings.ToLower(strings.TrimSpace(s))
if value == "warning" {
value = "warn"
}

switch value {
case "debug":
return slog.LevelDebug, nil
case "info":
return slog.LevelInfo, nil
case "warn":
return slog.LevelWarn, nil
case "error":
return slog.LevelError, nil
default:
return slog.LevelInfo, fmt.Errorf("unsupported level %q", s)
}
}

// Build builds a slog.Logger using the provided configuration.
func Build(cfg Config) (*slog.Logger, io.Closer, *bytes.Buffer, error) {
cfg, err := normalizeConfig(cfg)
if err != nil {
return nil, nil, nil, err
}

level, err := ParseLevel(cfg.Level)
if err != nil {
return nil, nil, nil, err
}

var (
writer io.Writer
closer io.Closer
buf *bytes.Buffer
)

switch cfg.Output {
case "stdout":
writer = os.Stdout
case "stderr":
writer = os.Stderr
case "file":
file, err := os.OpenFile(cfg.FilePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
if err != nil {
return nil, nil, nil, fmt.Errorf("open log file: %w", err)
}
writer = file
closer = file
case "string":
if cfg.Buffer != nil {
buf = cfg.Buffer
} else {
buf = &bytes.Buffer{}
}
writer = buf
default:
return nil, nil, nil, fmt.Errorf("unsupported output %q", cfg.Output)
}

opts := &slog.HandlerOptions{Level: level}
var handler slog.Handler
if cfg.Format == "json" {
handler = slog.NewJSONHandler(writer, opts)
} else {
handler = slog.NewTextHandler(writer, opts)
}

logger := slog.New(handler)
return logger, closer, buf, nil
}
Comment on lines +33 to +84
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Build function lacks test coverage for stderr output and JSON format with various output types. While file and string outputs are tested, the stderr output and JSON format paths should have explicit test coverage.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


// ApplyGlobal applies the logger and level to slog defaults.
func ApplyGlobal(logger *slog.Logger, level slog.Level) {
if logger == nil {
return
}
slog.SetDefault(logger)
slog.SetLogLoggerLevel(level)
}
Comment on lines +86 to +93
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ApplyGlobal function lacks test coverage. This exported function modifies global state and should have tests verifying that it correctly sets the default logger and log level, and that it handles the nil logger case appropriately.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

91 changes: 91 additions & 0 deletions logging/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package logging
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logging package is missing package-level documentation. According to the project's coding guidelines (guideline 1000000, item 5), public APIs should be documented. Add a package comment at the top of one of the files (typically the main file like config.go) describing the package's purpose, main types, and usage.

Copilot generated this review using guidance from repository custom instructions.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


import (
"bytes"
"fmt"
"strings"
)

const (
DefaultLevel = "info"
DefaultFormat = "text"
DefaultOutput = "stdout"
)

// Config defines configuration for logging outputs and formatting.
type Config struct {
Level string `json:"level"`
Format string `json:"format"`
Output string `json:"output"`
FilePath string `json:"filePath,omitempty"`
Buffer *bytes.Buffer `json:"-"`
}

// DefaultConfig returns the default logging configuration.
func DefaultConfig() Config {
return Config{
Level: DefaultLevel,
Format: DefaultFormat,
Output: DefaultOutput,
}
}

// WithDefaults fills in empty fields with defaults.
func (c Config) WithDefaults() Config {
if strings.TrimSpace(c.Level) == "" {
c.Level = DefaultLevel
}
if strings.TrimSpace(c.Format) == "" {
c.Format = DefaultFormat
}
if strings.TrimSpace(c.Output) == "" {
c.Output = DefaultOutput
}
return c
}
Comment on lines +33 to +45
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public method WithDefaults lacks test coverage. While other functions in config.go are tested indirectly through Service tests, this exported method should have explicit tests to verify its behavior, especially edge cases with empty strings and whitespace.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


// Normalize lowercases string fields and clears file/buffer fields when not used.
func (c Config) Normalize() Config {
c.Level = strings.ToLower(strings.TrimSpace(c.Level))
c.Format = strings.ToLower(strings.TrimSpace(c.Format))
c.Output = strings.ToLower(strings.TrimSpace(c.Output))
if c.Output != "file" {
c.FilePath = ""
}
if c.Output != "string" {
c.Buffer = nil
}
return c
}
Comment on lines +47 to +59
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public method Normalize lacks test coverage. This exported method should have explicit tests to verify its behavior, including testing that it properly lowercases inputs, clears FilePath when output is not "file", and clears Buffer when output is not "string".

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


// Validate checks the configuration for supported values.
func (c Config) Validate() error {
if _, err := ParseLevel(c.Level); err != nil {
return err
}

switch c.Format {
case "text", "json":
default:
return fmt.Errorf("unsupported format %q", c.Format)
}

switch c.Output {
case "stdout", "stderr", "file", "string":
default:
return fmt.Errorf("unsupported output %q", c.Output)
}

if c.Output == "file" && strings.TrimSpace(c.FilePath) == "" {
return fmt.Errorf("file output requires filePath")
}
return nil
}
Comment on lines +61 to +83
Copy link

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public method Validate lacks direct test coverage. While validation is tested indirectly through other tests, this exported method should have explicit tests covering all validation paths: invalid level, invalid format, invalid output, and the file output without filePath case.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


func normalizeConfig(cfg Config) (Config, error) {
cfg = cfg.WithDefaults().Normalize()
if err := cfg.Validate(); err != nil {
return Config{}, err
}
return cfg, nil
}
Loading
Loading