[tracing] LightStep OpenTracing support #2100
Conversation
- Add LightStep as a supported OT backend. - Update docs.
Codecov Report
@@ Coverage Diff @@
## master #2100 /- ##
========================================
Coverage 70.2% 72.4% 2.2%
========================================
Files 1004 1008 4
Lines 86378 86653 275
========================================
Hits 60650 62756 2106
Misses 21580 19710 -1870
- Partials 4148 4187 39
Continue to review full report at Codecov.
|
|
|
||
| lightstep "github.com/lightstep/lightstep-tracer-go" | ||
| "github.com/m3db/m3/src/x/instrument" |
There was a problem hiding this comment.
nit: github.com/m3db/m3... imports in the middle separated from the third party exports at the bottom? (linting seems to be turned off right now)
| // Supported tracing backends. Currently only jaeger is supported. | ||
| var ( | ||
| TracingBackendJaeger = "jaeger" | ||
| TracingBackendJaeger = "jaeger" | ||
| TracingBackendLightstep = "lightstep" |
There was a problem hiding this comment.
nit: Comments on these exported symbols?
src/x/opentracing/tracing.go
Outdated
| tags[spanTagBuildVersion] = instrument.Version | ||
| tags[spanTagBuildDate] = instrument.BuildDate | ||
| tags[spanTagBuildTimeUnix] = instrument.BuildTimeUnix | ||
| tags[spanTagBuildGoVersion] = runtime.Version() |
There was a problem hiding this comment.
👍 these seem quite valuable - should we add them to cfg.Jaeger.NewTracer(...) with cfg.Jaeger.NewTracer(..., jaeger.TracerOptions.Tag(key, value), jaeger.TracerOptions.Tag(key, value), ...) as options?
src/x/opentracing/tracing.go
Outdated
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | ||
| l.tracer.Close(ctx) | ||
| cancel() | ||
| return nil |
There was a problem hiding this comment.
Worth returning ctx.Error() perhaps? (in case it was timed out)
What this PR does / why we need it:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: