Skip to content

Commit

Permalink
Add better logging options (jaegertracing#5675)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Part of jaegertracing#5290
- It's difficult to troubleshoot ES queries because logs are always
written in JSON format, so the ES req/resp JSON is being escaped.

## Description of the changes
- Add `--log-encoding` option to allow changing logs to plain text
(console).
- Fix logger initialization for ES to reuse the existing logger instead
of creating a new one (new one was switching back to JSON)

## How was this change tested?
```
$ SPAN_STORAGE_TYPE=elasticsearch go run -tags=ui ./cmd/all-in-one --es.log-level=debug --log-encoding=console
```
A query is logged like this:
```
2024-06-22T22:56:34.272-0400	info	[email protected]+incompatible/client.go:835	GET /_msearch?rest_total_hits_as_int=true HTTP/1.1
Host: 127.0.0.1:9200
User-Agent: elastic/6.2.37 (darwin-arm64)
Content-Length: 310
Accept: application/json
Content-Type: application/json
Accept-Encoding: gzip

{"ignore_unavailable":true,"indices":["jaeger-span-2024-06-23","jaeger-span-2024-06-22","jaeger-span-2024-06-21","jaeger-span-2024-06-20"]}
{"query":{"bool":{"must":{"term":{"traceID":"1a2cee16e928c8a2f00d9ae784d8cccc"}}}},"search_after":[1718848594271777],"size":10000,"sort":[{"startTime":{"order":"asc"}}]}
```


## Checklist
- [ ] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [ ] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro authored Jun 23, 2024
1 parent ca8cdae commit 1c1bc08
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 13 deletions.
14 changes: 11 additions & 3 deletions cmd/internal/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
const (
spanStorageType = "span-storage.type" // deprecated
logLevel = "log-level"
logEncoding = "log-encoding" // json or console
configFile = "config-file"
)

Expand Down Expand Up @@ -98,23 +99,26 @@ type SharedFlags struct {
}

type logging struct {
Level string
Level string
Encoding string
}

// AddFlags adds flags for SharedFlags
func AddFlags(flagSet *flag.FlagSet) {
flagSet.String(spanStorageType, "", "(deprecated) please use SPAN_STORAGE_TYPE environment variable. Run this binary with the 'env' command for help.")
AddLoggingFlag(flagSet)
AddLoggingFlags(flagSet)
}

// AddLoggingFlag adds logging flag for SharedFlags
func AddLoggingFlag(flagSet *flag.FlagSet) {
func AddLoggingFlags(flagSet *flag.FlagSet) {
flagSet.String(logLevel, "info", "Minimal allowed log Level. For more levels see https://github.com/uber-go/zap")
flagSet.String(logEncoding, "json", "Log encoding. Supported values are 'json' and 'console'.")
}

// InitFromViper initializes SharedFlags with properties from viper
func (flags *SharedFlags) InitFromViper(v *viper.Viper) *SharedFlags {
flags.Logging.Level = v.GetString(logLevel)
flags.Logging.Encoding = v.GetString(logEncoding)
return flags
}

Expand All @@ -126,5 +130,9 @@ func (flags *SharedFlags) NewLogger(conf zap.Config, options ...zap.Option) (*za
return nil, err
}
conf.Level = zap.NewAtomicLevelAt(level)
conf.Encoding = flags.Logging.Encoding
if flags.Logging.Encoding == "console" {
conf.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
}
return conf.Build(options...)
}
2 changes: 1 addition & 1 deletion cmd/internal/flags/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func NewService(adminPort int) *Service {
func (s *Service) AddFlags(flagSet *flag.FlagSet) {
AddConfigFileFlag(flagSet)
if s.NoStorage {
AddLoggingFlag(flagSet)
AddLoggingFlags(flagSet)
} else {
AddFlags(flagSet)
}
Expand Down
15 changes: 6 additions & 9 deletions pkg/es/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func (c *Configuration) getConfigOptions(logger *zap.Logger) ([]elastic.ClientOp
options = append(options, elastic.SetSendGetBodyAs(c.SendGetBodyAs))
}

options, err := addLoggerOptions(options, c.LogLevel)
options, err := addLoggerOptions(options, c.LogLevel, logger)
if err != nil {
return options, err
}
Expand All @@ -391,13 +391,11 @@ func (c *Configuration) getConfigOptions(logger *zap.Logger) ([]elastic.ClientOp
return options, nil
}

func addLoggerOptions(options []elastic.ClientOptionFunc, logLevel string) ([]elastic.ClientOptionFunc, error) {
func addLoggerOptions(options []elastic.ClientOptionFunc, logLevel string, logger *zap.Logger) ([]elastic.ClientOptionFunc, error) {
// Decouple ES logger from the log-level assigned to the parent application's log-level; otherwise, the least
// permissive log-level will dominate.
// e.g. --log-level=info and --es.log-level=debug would mute ES's debug logging and would require --log-level=debug
// to show ES debug logs.
prodConfig := zap.NewProductionConfig()

var lvl zapcore.Level
var setLogger func(logger elastic.Logger) elastic.ClientOptionFunc

Expand All @@ -415,11 +413,10 @@ func addLoggerOptions(options []elastic.ClientOptionFunc, logLevel string) ([]el
return options, fmt.Errorf("unrecognized log-level: \"%s\"", logLevel)
}

prodConfig.Level.SetLevel(lvl)
esLogger, err := prodConfig.Build()
if err != nil {
return options, err
}
esLogger := logger.WithOptions(
zap.IncreaseLevel(lvl),
zap.AddCallerSkip(2), // to ensure the right caller:lineno are logged
)

// Elastic client requires a "Printf"-able logger.
l := zapgrpc.NewLogger(esLogger)
Expand Down

0 comments on commit 1c1bc08

Please sign in to comment.