search

opentracing / basictracer-go

Basic implementation of the OpenTracing API for Go. 🛑 This library is DEPRECATED!
Apache License 2.0
81 stars 27 forks source link

event: update NetTraceIntegrator to support LogFields, Tag #40

Closed RaduBerinde closed 8 years ago

RaduBerinde commented 8 years ago

NetTraceIntegrator now handles EventLogFields and EventTag and produces messages for them.

I used my best judgment for how to convert KV fields to a single message but I am kind of pulling it out of a hat. Ideally it would be consistent with how Lighstep will do it; @bensigelman are there any thoughts/plans on that front?

@tschottdorf @petermattis

tbg commented 8 years ago

uber/zap just introduced a text encoder for their KV logging, maybe there's something to crib there.

RaduBerinde commented 8 years ago

Looked at zap's text_encoder, it encodes KVs as key1=val1 key2=val2 ...

I want to avoid having an event= in every message so I treated that key differently.

jmacd commented 8 years ago

LGTM

bhs commented 8 years ago

Thanks @RaduBerinde!

jmacd commented 8 years ago

Converting to string means copying and allocating a second copy of the data, though. ᐧ

On Wed, Sep 28, 2016 at 4:17 PM, RaduBerinde notifications@github.com wrote:

@RaduBerinde commented on this pull request.

In events/event_nettrace.go https://github.com/opentracing/basictracer-go/pull/40:

@@ -16,6 +20,18 @@ var NetTraceIntegrator = func() func(basictracer.SpanEvent) { tr.SetMaxEvents(1000) case basictracer.EventFinish: tr.Finish()

  • case basictracer.EventTag:
  • tr.LazyPrintf("%s:%v", t.Key, t.Value)
  • case basictracer.EventLogFields:
  • var buf bytes.Buffer
  • for i, f := range t.Fields {
  • if i > 0 {
  • buf.WriteByte(' ')
  • }
  • fmt.Fprintf(&buf, "%s:%v", f.Key(), f.Value())
  • } +
  • tr.LazyPrintf("%s", buf.String())

It's a judgment call - bytes.Buffer is at least 100 bytes or so and would need to get moved to heap and kept around, while the average message is much shorter.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/opentracing/basictracer-go/pull/40, or mute the thread https://github.com/notifications/unsubscribe-auth/ADdiiUe9o94vZie_pj-fA77oVQSQSBHcks5quvWVgaJpZM4KIBLD .