kitex-contrib / obs-opentelemetry

An extension library of OpenTelemetry for Kitex
Apache License 2.0
24 stars 21 forks source link

fix: print span info with ctx && add CtxKVLog method #16

Closed weedge closed 1 year ago

weedge commented 1 year ago

go get go.opentelemetry.io/otel@v1.11.1

CHANGE:

  1. add zap logger for zap fields, log kv pairs

    func (l *Logger) CtxKVLog(ctx context.Context, level klog.Level, format string, kvs ...interface{})
  2. fix: print span info with ctx

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

weedge commented 1 year ago

need to sign cla it's ok , signed the CLA

CoderPoet commented 1 year ago

Please fix the problem with code formatting

weedge commented 1 year ago

Please fix the problem with code formatting

golangci-lint run --out-format=github-actions --path-prefix=./logging/zap -E gofumpt, it's ok

rogerogers commented 1 year ago

Seems not necessary to introduce logger, zap.Any is even slower than sugar.

weedge commented 1 year ago

Seems not necessary to introduce logger, zap.Any is even slower than sugar.

I see https://github.com/uber-go/zap#performance performance is good to me; Please show some benchmark ? i see zap.Any code switch case , if defined type , no reflection, like %v or %+v;

rogerogers commented 1 year ago

Seems not necessary to introduce logger, zap.Any is even slower than sugar.

I see https://github.com/uber-go/zap#performance performance is good to me; Please show some benchmark ? i see zap.Any code switch case , if defined type , no reflection, like %v or %+v;

func BenchmarkZapAny(b *testing.B) {
    l, _ := zap.NewProduction()
    for i := 0; i < b.N; i++ {
        l.Info("bench", zap.String("key", "value"), zap.Any("key1", "value1"))
    }
}

func BenchmarkZapSugar(b *testing.B) {
    l, _ := zap.NewProduction()
    s := l.Sugar()
    for i := 0; i < b.N; i++ {
        s.Infow("bench", "key", "value", zap.Any("key1", "value1"))
    }
}

https://pkg.go.dev/go.uber.org/zap#hdr-Choosing_a_Logger

sugar has similar methods.

weedge commented 1 year ago

Seems not necessary to introduce logger, zap.Any is even slower than sugar.

I see https://github.com/uber-go/zap#performance performance is good to me; Please show some benchmark ? i see zap.Any code switch case , if defined type , no reflection, like %v or %+v;

func BenchmarkZapAny(b *testing.B) {
  l, _ := zap.NewProduction()
  for i := 0; i < b.N; i++ {
      l.Info("bench", zap.String("key", "value"), zap.Any("key1", "value1"))
  }
}

func BenchmarkZapSugar(b *testing.B) {
  l, _ := zap.NewProduction()
  s := l.Sugar()
  for i := 0; i < b.N; i++ {
      s.Infow("bench", "key", "value", zap.Any("key1", "value1"))
  }
}

https://pkg.go.dev/go.uber.org/zap#hdr-Choosing_a_Logger

sugar has similar methods.

3q~, sugar **w method print kv log is ok~

ppzqh commented 1 year ago

@CoderPoet Seems that all issues are fixed. Please help to take a look at this pr, thanks!

bytedance-oss-robot[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CoderPoet, weedge

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kitex-contrib/obs-opentelemetry/blob/main/OWNERS)~~ [CoderPoet] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment