Skip to content

Commit

Permalink
all: Harmonize logger usage
Browse files Browse the repository at this point in the history
  • Loading branch information
KrishnaIyer committed Nov 6, 2023
1 parent 46f8fb7 commit bec38d9
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 82 deletions.
16 changes: 0 additions & 16 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"go.thethings.network/lorawan-stack-migrate/pkg/export"
"go.thethings.network/lorawan-stack-migrate/pkg/source"
"go.thethings.network/lorawan-stack/v3/pkg/log"
"go.thethings.network/lorawan-stack/v3/pkg/rpcmiddleware/rpclog"
)

var (
Expand All @@ -40,25 +39,10 @@ var (

SilenceUsage: true,
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
logLevel := log.InfoLevel
if verbose, _ := cmd.Flags().GetBool("verbose"); verbose {
logLevel = log.DebugLevel
}
logHandler, err := log.NewZap("console")
if err != nil {
return err
}
logger = log.NewLogger(
logHandler,
log.WithLevel(logLevel),
)
rpclog.ReplaceGrpcLogger(logger)
ctx = log.NewContext(ctx, logger)

exportCfg.DevIDPrefix, _ = cmd.Flags().GetString("dev-id-prefix")
exportCfg.EUIForID, _ = cmd.Flags().GetBool("set-eui-as-id")
ctx = export.NewContext(ctx, exportCfg)

cmd.SetContext(ctx)
return nil
},
Expand Down
1 change: 0 additions & 1 deletion pkg/commands/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ func ExecuteParentPersistentPreRun(cmd *cobra.Command, args []string) error {
return nil
}
p := cmd.Parent()

if f := p.PersistentPreRunE; f != nil {
if err := f(p, args); err != nil {
return err
Expand Down
43 changes: 43 additions & 0 deletions pkg/log/log.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright © 2023 The Things Network Foundation, The Things Industries B.V.
//
// Licensed 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 log

import (
"context"

"go.uber.org/zap"
)

type loggerKeyType string

var loggerKey loggerKeyType = "logger"

// NewContext returns a new context with a *zap.SugaredLogger and panics if the logger is nil.
func NewContext(parentCtx context.Context, logger *zap.SugaredLogger) context.Context {
if logger == nil {
panic("Nil logger")
}
return context.WithValue(parentCtx, loggerKey, logger)
}

// FromContext retrieves a *zap.SugaredLogger from a context and panics if there isn't one.
func FromContext(ctx context.Context) *zap.SugaredLogger {
val := ctx.Value(loggerKey)
logger, ok := val.(*zap.SugaredLogger)
if !ok {
panic("No logger in context")
}
return logger
}
20 changes: 11 additions & 9 deletions pkg/source/firefly/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package client

import (
"bytes"
"context"
"crypto/tls"
"crypto/x509"
"encoding/json"
Expand All @@ -26,8 +27,8 @@ import (
"os"
"time"

"go.thethings.network/lorawan-stack-migrate/pkg/log"
"go.thethings.network/lorawan-stack/v3/pkg/errors"
"go.uber.org/zap"
)

const defaultTimeout = 10 * time.Second
Expand All @@ -44,11 +45,11 @@ type Config struct {
type Client struct {
*Config
*http.Client
logger *zap.SugaredLogger
ctx context.Context
}

// New creates a new Firefly client.
func (cfg *Config) NewClient(logger *zap.SugaredLogger) (*Client, error) {
func (cfg *Config) NewClient(ctx context.Context) (*Client, error) {
httpTransport := &http.Transport{}
if cfg.CACertPath != "" && !cfg.UseHTTP {
pemBytes, err := os.ReadFile(cfg.CACertPath)
Expand All @@ -72,7 +73,7 @@ func (cfg *Config) NewClient(logger *zap.SugaredLogger) (*Client, error) {
Transport: httpTransport,
Timeout: defaultTimeout,
},
logger: logger,
ctx: ctx,
}, nil
}

Expand Down Expand Up @@ -100,7 +101,8 @@ func (c *Client) do(resource, method string, body []byte, params string) ([]byte
Path: fmt.Sprintf("/api/v1/%s", resource),
RawQuery: authAndParams,
}
logger := c.logger.With("url", url.String())

logger := log.FromContext(c.ctx).With("url", url.String())
logger.Debug("Request resource")
req, err := http.NewRequest(method, url.String(), bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
Expand All @@ -112,12 +114,12 @@ func (c *Client) do(resource, method string, body []byte, params string) ([]byte
return nil, err
}
defer res.Body.Close()
body, err = io.ReadAll(res.Body)
if err != nil {
return nil, err
}
switch {
case res.StatusCode == http.StatusOK:
body, err := io.ReadAll(res.Body)
if err != nil {
return nil, err
}
return body, nil
case res.StatusCode == http.StatusNotFound:
return nil, errResourceNotFound.WithAttributes("resource", resource)
Expand Down
13 changes: 0 additions & 13 deletions pkg/source/firefly/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"os"

"github.com/spf13/pflag"
"go.uber.org/zap"

"go.thethings.network/lorawan-stack-migrate/pkg/source"
"go.thethings.network/lorawan-stack-migrate/pkg/source/firefly/client"
Expand All @@ -42,8 +41,6 @@ type Config struct {
flags *pflag.FlagSet
}

var logger *zap.SugaredLogger

// NewConfig returns a new Firefly configuration.
func NewConfig() *Config {
config := &Config{
Expand Down Expand Up @@ -101,16 +98,6 @@ where the devices are exported but they are still valid on the firefly server
// Initialize the configuration.
func (c *Config) Initialize(src source.Config) error {
c.src = src
cfg := zap.NewProductionConfig()
if c.src.Verbose {
cfg.Level = zap.NewAtomicLevelAt(zap.DebugLevel)
}
zapLogger, err := cfg.Build()
if err != nil {
return err
}
logger = zapLogger.Sugar()

if c.appID == "" {
return errNoAppID.New()
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/source/firefly/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"google.golang.org/protobuf/types/known/wrapperspb"

"go.thethings.network/lorawan-stack-migrate/pkg/iterator"
"go.thethings.network/lorawan-stack-migrate/pkg/log"
"go.thethings.network/lorawan-stack-migrate/pkg/source"
"go.thethings.network/lorawan-stack-migrate/pkg/source/firefly/client"
"go.thethings.network/lorawan-stack-migrate/pkg/util"
Expand All @@ -41,7 +42,7 @@ func createNewSource(cfg *Config) source.CreateSource {
if err := cfg.Initialize(src); err != nil {
return nil, err
}
client, err := cfg.NewClient(logger)
client, err := cfg.NewClient(log.NewContext(ctx, src.Logger))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -103,7 +104,7 @@ func (s Source) ExportDevice(devEUI string) (*ttnpb.EndDevice, error) {
Source: ttnpb.LocationSource_SOURCE_REGISTRY,
},
}
logger.Debugw("Set location", "location", v3dev.Locations)
s.src.Logger.Debugw("Set location", "location", v3dev.Locations)
}
v3dev.Ids.DevEui, err = util.UnmarshalTextToBytes(&types.EUI64{}, ffdev.EUI)
if err != nil {
Expand Down Expand Up @@ -165,7 +166,7 @@ func (s Source) ExportDevice(devEUI string) (*ttnpb.EndDevice, error) {
}

if s.invalidateKeys {
logger.Debugw("Increment the last byte of the device keys", "device_id", ffdev.Name, "device_eui", ffdev.EUI)
s.src.Logger.Debugw("Increment the last byte of the device keys", "device_id", ffdev.Name, "device_eui", ffdev.EUI)
// Increment the last byte of the device keys.
// This makes it easier to rollback a migration if needed.
updated := ffdev.WithIncrementedKeys()
Expand All @@ -184,7 +185,7 @@ func (s Source) RangeDevices(_ string, f func(source.Source, string) error) erro
devs []client.Device
err error
)
logger.Debugw("Firefly LNS does not group devices by an application. Get all devices accessible by the API key")
s.src.Logger.Debugw("Firefly LNS does not group devices by an application. Get all devices accessible by the API key")
devs, err = s.GetAllDevices()
if err != nil {
return err
Expand Down
12 changes: 12 additions & 0 deletions pkg/source/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@ import (
"github.com/spf13/pflag"
"go.thethings.network/lorawan-stack-migrate/pkg/iterator"
"go.thethings.network/lorawan-stack/v3/pkg/ttnpb"
"go.uber.org/zap"
)

type Config struct {
DryRun, Verbose bool
FrequencyPlansURL string

Logger *zap.SugaredLogger

source string
}

Expand Down Expand Up @@ -85,6 +88,15 @@ func NewSource(ctx context.Context) (Source, error) {
if RootConfig.Source() == "" {
return nil, ErrNoSource.New()
}
cfg := zap.NewProductionConfig()
if RootConfig.Verbose {
cfg.Level = zap.NewAtomicLevelAt(zap.DebugLevel)
}
zapLogger, err := cfg.Build()
if err != nil {
return nil, err
}
RootConfig.Logger = zapLogger.Sugar()
if registration, ok := registeredSources[RootConfig.Source()]; ok {
return registration.Create(ctx, RootConfig)
}
Expand Down
25 changes: 2 additions & 23 deletions pkg/source/tts/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,8 @@ import (
"github.com/spf13/pflag"
"go.thethings.network/lorawan-stack-migrate/pkg/source"
"go.thethings.network/lorawan-stack-migrate/pkg/source/tts/api"
"go.uber.org/zap"
)

var logger *zap.SugaredLogger

type serverConfig struct {
defaultGRPCAddress,
ApplicationServerGRPCAddress,
Expand Down Expand Up @@ -150,12 +147,6 @@ type Config struct {
func (c *Config) Initialize(rootConfig source.Config) error {
c.Config = rootConfig

var err error
logger, err = NewLogger(c.Verbose)
if err != nil {
return err
}

if c.appAPIKey == "" {
return errNoAppAPIKey.New()
}
Expand All @@ -164,7 +155,7 @@ func (c *Config) Initialize(rootConfig source.Config) error {
switch {
case c.insecure:
api.SetInsecure(true)
logger.Warn("Using insecure connection to API")
c.Logger.Warn("Using insecure connection to API")

default:
if c.caPath != "" {
Expand All @@ -179,25 +170,13 @@ func (c *Config) Initialize(rootConfig source.Config) error {

// DeleteSourceDevice is not allowed during a dry run
if c.DryRun && c.DeleteSourceDevice {
logger.Warn("Cannot delete source devices during a dry run.")
c.Logger.Warn("Cannot delete source devices during a dry run.")
c.DeleteSourceDevice = false
}

return nil
}

func NewLogger(verbose bool) (*zap.SugaredLogger, error) {
cfg := zap.NewProductionConfig()
if verbose {
cfg.Level = zap.NewAtomicLevelAt(zap.DebugLevel)
}
logger, err := cfg.Build()
if err != nil {
return nil, err
}
return logger.Sugar(), nil
}

func setCustomCA(path string) error {
pemBytes, err := os.ReadFile(path)
if err != nil {
Expand Down
Loading

0 comments on commit bec38d9

Please sign in to comment.