Skip to content

Commit 1c1bc08

Browse files
authored
Add better logging options (jaegertracing#5675)
## 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]>
1 parent ca8cdae commit 1c1bc08

File tree

3 files changed

+18
-13
lines changed

3 files changed

+18
-13
lines changed

cmd/internal/flags/flags.go

+11-3
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
const (
3030
spanStorageType = "span-storage.type" // deprecated
3131
logLevel = "log-level"
32+
logEncoding = "log-encoding" // json or console
3233
configFile = "config-file"
3334
)
3435

@@ -98,23 +99,26 @@ type SharedFlags struct {
9899
}
99100

100101
type logging struct {
101-
Level string
102+
Level string
103+
Encoding string
102104
}
103105

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

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

115118
// InitFromViper initializes SharedFlags with properties from viper
116119
func (flags *SharedFlags) InitFromViper(v *viper.Viper) *SharedFlags {
117120
flags.Logging.Level = v.GetString(logLevel)
121+
flags.Logging.Encoding = v.GetString(logEncoding)
118122
return flags
119123
}
120124

@@ -126,5 +130,9 @@ func (flags *SharedFlags) NewLogger(conf zap.Config, options ...zap.Option) (*za
126130
return nil, err
127131
}
128132
conf.Level = zap.NewAtomicLevelAt(level)
133+
conf.Encoding = flags.Logging.Encoding
134+
if flags.Logging.Encoding == "console" {
135+
conf.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
136+
}
129137
return conf.Build(options...)
130138
}

cmd/internal/flags/service.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func NewService(adminPort int) *Service {
6868
func (s *Service) AddFlags(flagSet *flag.FlagSet) {
6969
AddConfigFileFlag(flagSet)
7070
if s.NoStorage {
71-
AddLoggingFlag(flagSet)
71+
AddLoggingFlags(flagSet)
7272
} else {
7373
AddFlags(flagSet)
7474
}

pkg/es/config/config.go

+6-9
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ func (c *Configuration) getConfigOptions(logger *zap.Logger) ([]elastic.ClientOp
378378
options = append(options, elastic.SetSendGetBodyAs(c.SendGetBodyAs))
379379
}
380380

381-
options, err := addLoggerOptions(options, c.LogLevel)
381+
options, err := addLoggerOptions(options, c.LogLevel, logger)
382382
if err != nil {
383383
return options, err
384384
}
@@ -391,13 +391,11 @@ func (c *Configuration) getConfigOptions(logger *zap.Logger) ([]elastic.ClientOp
391391
return options, nil
392392
}
393393

394-
func addLoggerOptions(options []elastic.ClientOptionFunc, logLevel string) ([]elastic.ClientOptionFunc, error) {
394+
func addLoggerOptions(options []elastic.ClientOptionFunc, logLevel string, logger *zap.Logger) ([]elastic.ClientOptionFunc, error) {
395395
// Decouple ES logger from the log-level assigned to the parent application's log-level; otherwise, the least
396396
// permissive log-level will dominate.
397397
// e.g. --log-level=info and --es.log-level=debug would mute ES's debug logging and would require --log-level=debug
398398
// to show ES debug logs.
399-
prodConfig := zap.NewProductionConfig()
400-
401399
var lvl zapcore.Level
402400
var setLogger func(logger elastic.Logger) elastic.ClientOptionFunc
403401

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

418-
prodConfig.Level.SetLevel(lvl)
419-
esLogger, err := prodConfig.Build()
420-
if err != nil {
421-
return options, err
422-
}
416+
esLogger := logger.WithOptions(
417+
zap.IncreaseLevel(lvl),
418+
zap.AddCallerSkip(2), // to ensure the right caller:lineno are logged
419+
)
423420

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

0 commit comments

Comments
 (0)