-
Notifications
You must be signed in to change notification settings - Fork 824
docstore: Opentelemetry migration of docstore #3550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
1946e78
0b04c34
47330e9
672b9c1
11a5731
77be8c3
7dbfca9
992ccac
477c7cc
d7e1eea
65163b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,12 +24,17 @@ import ( | |
"sort" | ||
"strings" | ||
"sync" | ||
"time" | ||
"unicode/utf8" | ||
|
||
"go.opentelemetry.io/otel" | ||
"go.opentelemetry.io/otel/attribute" | ||
"go.opentelemetry.io/otel/metric" | ||
"go.opentelemetry.io/otel/trace" | ||
"gocloud.dev/docstore/driver" | ||
"gocloud.dev/gcerrors" | ||
"gocloud.dev/internal/gcerr" | ||
"gocloud.dev/internal/oc" | ||
gcdkotel "gocloud.dev/internal/otel" | ||
) | ||
|
||
// A Document is a set of field-value pairs. One or more fields, called the key | ||
|
@@ -46,34 +51,61 @@ type Document = interface{} | |
// To create a Collection, use constructors found in driver subpackages. | ||
type Collection struct { | ||
driver driver.Collection | ||
tracer *oc.Tracer | ||
tracer *gcdkotel.Tracer | ||
mu sync.Mutex | ||
closed bool | ||
} | ||
|
||
const pkgName = "gocloud.dev/docstore" | ||
|
||
var ( | ||
latencyMeasure = oc.LatencyMeasure(pkgName) | ||
// Initialize OpenTelemetry meter and tracer | ||
meter = otel.GetMeterProvider().Meter(pkgName) | ||
|
||
// OpenCensusViews are predefined views for OpenCensus metrics. | ||
// The views include counts and latency distributions for API method calls. | ||
// See the example at https://godoc.org/go.opencensus.io/stats/view for usage. | ||
OpenCensusViews = oc.Views(pkgName, latencyMeasure) | ||
// Define counter instrument for completed calls (replaces the previous OpenCensus metrics) | ||
completedCallsCounter metric.Int64Counter | ||
|
||
// Define histogram instrument for latency measurement | ||
latencyHistogram metric.Float64Histogram | ||
|
||
// Tracer for creating spans | ||
tracer = otel.GetTracerProvider().Tracer(pkgName) | ||
) | ||
|
||
// Initialize the metrics | ||
func init() { | ||
var err error | ||
|
||
// Create the completed calls counter | ||
completedCallsCounter, err = meter.Int64Counter( | ||
pkgName+"/completed_calls", | ||
metric.WithDescription("Total number of completed method calls"), | ||
metric.WithUnit("{call}"), | ||
) | ||
if err != nil { | ||
log.Printf("Failed to create completed_calls counter: %v", err) | ||
} | ||
|
||
// Create the latency histogram | ||
latencyHistogram, err = meter.Float64Histogram( | ||
pkgName+"/latency", | ||
metric.WithDescription("Latency of method calls"), | ||
metric.WithUnit("ms"), | ||
) | ||
if err != nil { | ||
log.Printf("Failed to create latency histogram: %v", err) | ||
} | ||
} | ||
|
||
// NewCollection is intended for use by drivers only. Do not use in application code. | ||
var NewCollection = newCollection | ||
|
||
// newCollection makes a Collection. | ||
func newCollection(d driver.Collection) *Collection { | ||
providerName := gcdkotel.ProviderName(d) | ||
c := &Collection{ | ||
driver: d, | ||
tracer: &oc.Tracer{ | ||
Package: pkgName, | ||
Provider: oc.ProviderName(d), | ||
LatencyMeasure: latencyMeasure, | ||
}, | ||
tracer: gcdkotel.NewTracer(pkgName, providerName), | ||
} | ||
_, file, lineno, ok := runtime.Caller(1) | ||
runtime.SetFinalizer(c, func(c *Collection) { | ||
|
@@ -338,15 +370,42 @@ func (l *ActionList) Do(ctx context.Context) error { | |
return l.do(ctx, true) | ||
} | ||
|
||
// do implements Do with optional OpenCensus tracing, so it can be used internally. | ||
func (l *ActionList) do(ctx context.Context, oc bool) (err error) { | ||
// do implements Do with optional OpenTelemetry tracing, so it can be used internally. | ||
func (l *ActionList) do(ctx context.Context, withTracing bool) (err error) { | ||
if err := l.coll.checkClosed(); err != nil { | ||
return ActionListError{{-1, errClosed}} | ||
} | ||
|
||
if oc { | ||
ctx = l.coll.tracer.Start(ctx, "ActionList.Do") | ||
defer func() { l.coll.tracer.End(ctx, err) }() | ||
var startTime time.Time | ||
var span trace.Span | ||
|
||
if withTracing { | ||
startTime = time.Now() | ||
ctx, span = l.coll.tracer.Start(ctx, "ActionList.Do") | ||
defer func() { | ||
l.coll.tracer.End(span, err) | ||
|
||
// Record metrics | ||
if completedCallsCounter != nil { | ||
attr := []attribute.KeyValue{ | ||
gcdkotel.MethodKey.String("ActionList.Do"), | ||
gcdkotel.ProviderKey.String(l.coll.tracer.Provider), | ||
} | ||
completedCallsCounter.Add(context.Background(), 1, metric.WithAttributes(attr...)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jba This has been resolved appropriately |
||
} | ||
|
||
if latencyHistogram != nil { | ||
attr := []attribute.KeyValue{ | ||
gcdkotel.MethodKey.String("ActionList.Do"), | ||
gcdkotel.ProviderKey.String(l.coll.tracer.Provider), | ||
} | ||
latencyHistogram.Record( | ||
context.Background(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
float64(time.Since(startTime).Milliseconds()), | ||
metric.WithAttributes(attr...), | ||
) | ||
} | ||
}() | ||
} | ||
|
||
das, err := l.toDriverActions() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could these two be defined in the scope of
if withTracing {}
below. Seems we dont need them outside the scope.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pramineni01 this has been moved to the withTracing scope