Skip to content
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

Deprecate Library and move all uses to Scope #2977

Merged
merged 11 commits into from Jul 6, 2022
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -17,6 +17,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Support for go1.16. Support is now only for go1.17 and go1.18 (#2917)

### Deprecated

- The `Library` struct in the `go.opentelemetry.io/otel/sdk/instrumentation` package is deprecated.
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
Use the equivalent `Scope` struct instead. (#2977)

## [1.7.0/0.30.0] - 2022-04-28

### Added
Expand Down
8 changes: 4 additions & 4 deletions exporters/jaeger/jaeger.go
Expand Up @@ -142,10 +142,10 @@ func spanToThrift(ss sdktrace.ReadOnlySpan) *gen.Span {
}
}

if il := ss.InstrumentationLibrary(); il.Name != "" {
tags = append(tags, getStringTag(keyInstrumentationLibraryName, il.Name))
if il.Version != "" {
tags = append(tags, getStringTag(keyInstrumentationLibraryVersion, il.Version))
if is := ss.InstrumentationScope(); is.Name != "" {
tags = append(tags, getStringTag(keyInstrumentationLibraryName, is.Name))
if is.Version != "" {
tags = append(tags, getStringTag(keyInstrumentationLibraryVersion, is.Version))
}
}

Expand Down
Expand Up @@ -19,8 +19,8 @@ import (
commonpb "go.opentelemetry.io/proto/otlp/common/v1"
)

func InstrumentationScope(il instrumentation.Library) *commonpb.InstrumentationScope {
if il == (instrumentation.Library{}) {
func InstrumentationScope(il instrumentation.Scope) *commonpb.InstrumentationScope {
if il == (instrumentation.Scope{}) {
return nil
}
return &commonpb.InstrumentationScope{
Expand Down
10 changes: 5 additions & 5 deletions exporters/otlp/otlptrace/internal/tracetransform/span.go
Expand Up @@ -34,7 +34,7 @@ func Spans(sdl []tracesdk.ReadOnlySpan) []*tracepb.ResourceSpans {

type key struct {
r attribute.Distinct
il instrumentation.Library
is instrumentation.Scope
}
ssm := make(map[key]*tracepb.ScopeSpans)

Expand All @@ -47,15 +47,15 @@ func Spans(sdl []tracesdk.ReadOnlySpan) []*tracepb.ResourceSpans {
rKey := sd.Resource().Equivalent()
k := key{
r: rKey,
il: sd.InstrumentationLibrary(),
is: sd.InstrumentationScope(),
}
scopeSpan, iOk := ssm[k]
if !iOk {
// Either the resource or instrumentation library were unknown.
// Either the resource or instrumentation scope were unknown.
scopeSpan = &tracepb.ScopeSpans{
Scope: InstrumentationScope(sd.InstrumentationLibrary()),
Scope: InstrumentationScope(sd.InstrumentationScope()),
Spans: []*tracepb.Span{},
SchemaUrl: sd.InstrumentationLibrary().SchemaURL,
SchemaUrl: sd.InstrumentationScope().SchemaURL,
}
}
scopeSpan.Spans = append(scopeSpan.Spans, span(sd))
Expand Down
Expand Up @@ -264,7 +264,7 @@ func TestSpanData(t *testing.T) {
attribute.Int64("rk2", 5),
attribute.StringSlice("rk3", []string{"sv1", "sv2"}),
),
InstrumentationLibrary: instrumentation.Library{
InstrumentationLibrary: instrumentation.Scope{
Name: "go.opentelemetry.io/test/otel",
Version: "v0.0.1",
SchemaURL: semconv.SchemaURL,
Expand Down
8 changes: 4 additions & 4 deletions exporters/zipkin/model.go
Expand Up @@ -218,10 +218,10 @@ func toZipkinTags(data tracesdk.ReadOnlySpan) map[string]string {
delete(m, "error")
}

if il := data.InstrumentationLibrary(); il.Name != "" {
m[keyInstrumentationLibraryName] = il.Name
if il.Version != "" {
m[keyInstrumentationLibraryVersion] = il.Version
if is := data.InstrumentationScope(); is.Name != "" {
m[keyInstrumentationLibraryName] = is.Name
if is.Version != "" {
m[keyInstrumentationLibraryVersion] = is.Version
}
}

Expand Down
1 change: 1 addition & 0 deletions sdk/instrumentation/library.go
Expand Up @@ -22,4 +22,5 @@ For more information see
package instrumentation // import "go.opentelemetry.io/otel/sdk/instrumentation"

// Library represents the instrumentation library.
// Deprecated: please use Scope instead.
type Library = Scope
20 changes: 10 additions & 10 deletions sdk/metric/controller/basic/controller.go
Expand Up @@ -59,7 +59,7 @@ var ErrControllerStarted = fmt.Errorf("controller already started")
type Controller struct {
// lock synchronizes Start() and Stop().
lock sync.Mutex
libraries sync.Map
scopes sync.Map
checkpointerFactory export.CheckpointerFactory

resource *resource.Resource
Expand All @@ -85,21 +85,21 @@ var _ metric.MeterProvider = &Controller{}
// with opts.
func (c *Controller) Meter(instrumentationName string, opts ...metric.MeterOption) metric.Meter {
cfg := metric.NewMeterConfig(opts...)
library := instrumentation.Library{
scope := instrumentation.Scope{
Name: instrumentationName,
Version: cfg.InstrumentationVersion(),
SchemaURL: cfg.SchemaURL(),
}

m, ok := c.libraries.Load(library)
m, ok := c.scopes.Load(scope)
if !ok {
checkpointer := c.checkpointerFactory.NewCheckpointer()
m, _ = c.libraries.LoadOrStore(
library,
m, _ = c.scopes.LoadOrStore(
scope,
registry.NewUniqueInstrumentMeterImpl(&accumulatorCheckpointer{
Accumulator: sdk.NewAccumulator(checkpointer),
checkpointer: checkpointer,
library: library,
scope: scope,
}))
}
return sdkapi.WrapMeterImpl(m.(*registry.UniqueInstrumentMeterImpl))
Expand All @@ -108,7 +108,7 @@ func (c *Controller) Meter(instrumentationName string, opts ...metric.MeterOptio
type accumulatorCheckpointer struct {
*sdk.Accumulator
checkpointer export.Checkpointer
library instrumentation.Library
scope instrumentation.Scope
}

var _ sdkapi.MeterImpl = &accumulatorCheckpointer{}
Expand Down Expand Up @@ -248,7 +248,7 @@ func (c *Controller) collect(ctx context.Context) error {
// registered to this controller. This briefly locks the controller.
func (c *Controller) accumulatorList() []*accumulatorCheckpointer {
var r []*accumulatorCheckpointer
c.libraries.Range(func(key, value interface{}) bool {
c.scopes.Range(func(key, value interface{}) bool {
acc, ok := value.(*registry.UniqueInstrumentMeterImpl).MeterImpl().(*accumulatorCheckpointer)
if ok {
r = append(r, acc)
Expand All @@ -272,7 +272,7 @@ func (c *Controller) checkpoint(ctx context.Context) error {
}

// checkpointSingleAccumulator checkpoints a single instrumentation
// library's accumulator, which involves calling
// scope's accumulator, which involves calling
// checkpointer.StartCollection, accumulator.Collect, and
// checkpointer.FinishCollection in sequence.
func (c *Controller) checkpointSingleAccumulator(ctx context.Context, ac *accumulatorCheckpointer) error {
Expand Down Expand Up @@ -330,7 +330,7 @@ func (c *Controller) ForEach(readerFunc func(l instrumentation.Library, r export
if err := func() error {
reader.RLock()
defer reader.RUnlock()
return readerFunc(acPair.library, reader)
return readerFunc(acPair.scope, reader)
}(); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/controller/basic/controller_test.go
Expand Up @@ -43,7 +43,7 @@ func getMap(t *testing.T, cont *controller.Controller) map[string]float64 {
out := processortest.NewOutput(attribute.DefaultEncoder())

require.NoError(t, cont.ForEach(
func(_ instrumentation.Library, reader export.Reader) error {
func(_ instrumentation.Scope, reader export.Reader) error {
return reader.ForEach(
aggregation.CumulativeTemporalitySelector(),
func(record export.Record) error {
Expand Down
7 changes: 5 additions & 2 deletions sdk/metric/metrictest/exporter.go
Expand Up @@ -64,9 +64,12 @@ func NewTestMeterProvider(opts ...Option) (metric.MeterProvider, *Exporter) {
return c, exp
}

// Library is the same as "sdk/instrumentation".Library but there is
// Deprecated: please use Scope instead.
type Library = Scope

// Scope is the same as "sdk/instrumentation".Scope but there is
// a package cycle to use it so it is redeclared here.
type Library struct {
type Scope struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't exactly sure what I should do here but chose to rename this to Scope and create a type alias to Libary as well. WDYT?

InstrumentationName string
InstrumentationVersion string
SchemaURL string
Expand Down
14 changes: 7 additions & 7 deletions sdk/trace/provider.go
Expand Up @@ -74,7 +74,7 @@ func (cfg tracerProviderConfig) MarshalLog() interface{} {
// instrumentation so it can trace operational flow through a system.
type TracerProvider struct {
mu sync.Mutex
namedTracer map[instrumentation.Library]*tracer
namedTracer map[instrumentation.Scope]*tracer
spanProcessors atomic.Value

// These fields are not protected by the lock mu. They are assumed to be
Expand Down Expand Up @@ -110,7 +110,7 @@ func NewTracerProvider(opts ...TracerProviderOption) *TracerProvider {
o = ensureValidTracerProviderConfig(o)

tp := &TracerProvider{
namedTracer: make(map[instrumentation.Library]*tracer),
namedTracer: make(map[instrumentation.Scope]*tracer),
sampler: o.sampler,
idGenerator: o.idGenerator,
spanLimits: o.spanLimits,
Expand Down Expand Up @@ -141,18 +141,18 @@ func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
if name == "" {
name = defaultTracerName
}
il := instrumentation.Library{
is := instrumentation.Scope{
Name: name,
Version: c.InstrumentationVersion(),
SchemaURL: c.SchemaURL(),
}
t, ok := p.namedTracer[il]
t, ok := p.namedTracer[is]
if !ok {
t = &tracer{
provider: p,
instrumentationLibrary: il,
provider: p,
instrumentationScope: is,
}
p.namedTracer[il] = t
p.namedTracer[is] = t
global.Info("Tracer created", "name", name, "version", c.InstrumentationVersion(), "schemaURL", c.SchemaURL())
}
return t
Expand Down
2 changes: 1 addition & 1 deletion sdk/trace/provider_test.go
Expand Up @@ -96,7 +96,7 @@ func TestSchemaURL(t *testing.T) {

// Verify that the SchemaURL of the constructed Tracer is correctly populated.
tracerStruct := tracerIface.(*tracer)
assert.EqualValues(t, schemaURL, tracerStruct.instrumentationLibrary.SchemaURL)
assert.EqualValues(t, schemaURL, tracerStruct.instrumentationScope.SchemaURL)
}

func TestTracerProviderSamplerConfigFromEnv(t *testing.T) {
Expand Down
40 changes: 23 additions & 17 deletions sdk/trace/snapshot.go
Expand Up @@ -26,22 +26,22 @@ import (
// snapshot is an record of a spans state at a particular checkpointed time.
// It is used as a read-only representation of that state.
type snapshot struct {
name string
spanContext trace.SpanContext
parent trace.SpanContext
spanKind trace.SpanKind
startTime time.Time
endTime time.Time
attributes []attribute.KeyValue
events []Event
links []Link
status Status
childSpanCount int
droppedAttributeCount int
droppedEventCount int
droppedLinkCount int
resource *resource.Resource
instrumentationLibrary instrumentation.Library
name string
spanContext trace.SpanContext
parent trace.SpanContext
spanKind trace.SpanKind
startTime time.Time
endTime time.Time
attributes []attribute.KeyValue
events []Event
links []Link
status Status
childSpanCount int
droppedAttributeCount int
droppedEventCount int
droppedLinkCount int
resource *resource.Resource
instrumentationScope instrumentation.Scope
}

var _ ReadOnlySpan = snapshot{}
Expand Down Expand Up @@ -102,10 +102,16 @@ func (s snapshot) Status() Status {
return s.status
}

// InstrumentationScope returns information about the instrumentation
// scope that created the span.
func (s snapshot) InstrumentationScope() instrumentation.Scope {
return s.instrumentationScope
}

// InstrumentationLibrary returns information about the instrumentation
// library that created the span.
func (s snapshot) InstrumentationLibrary() instrumentation.Library {
return s.instrumentationLibrary
return s.instrumentationScope
}

// Resource returns information about the entity that produced the span.
Expand Down
16 changes: 14 additions & 2 deletions sdk/trace/span.go
Expand Up @@ -63,8 +63,12 @@ type ReadOnlySpan interface {
Events() []Event
// Status returns the spans status.
Status() Status
// InstrumentationScope returns information about the instrumentation
// scope that created the span.
InstrumentationScope() instrumentation.Scope
// InstrumentationLibrary returns information about the instrumentation
// library that created the span.
// Deprecated: please use InstrumentationScope instead
craigpastro marked this conversation as resolved.
Show resolved Hide resolved
InstrumentationLibrary() instrumentation.Library
// Resource returns information about the entity that produced the span.
Resource() *resource.Resource
Expand Down Expand Up @@ -584,12 +588,20 @@ func (s *recordingSpan) Status() Status {
return s.status
}

// InstrumentationScope returns the instrumentation.Scope associated with
// the Tracer that created this span.
func (s *recordingSpan) InstrumentationScope() instrumentation.Scope {
s.mu.Lock()
defer s.mu.Unlock()
return s.tracer.instrumentationScope
}

// InstrumentationLibrary returns the instrumentation.Library associated with
// the Tracer that created this span.
func (s *recordingSpan) InstrumentationLibrary() instrumentation.Library {
s.mu.Lock()
defer s.mu.Unlock()
return s.tracer.instrumentationLibrary
return s.tracer.instrumentationScope
}

// Resource returns the Resource associated with the Tracer that created this
Expand Down Expand Up @@ -668,7 +680,7 @@ func (s *recordingSpan) snapshot() ReadOnlySpan {
defer s.mu.Unlock()

sd.endTime = s.endTime
sd.instrumentationLibrary = s.tracer.instrumentationLibrary
sd.instrumentationScope = s.tracer.instrumentationScope
sd.name = s.name
sd.parent = s.parent
sd.resource = s.tracer.provider.resource
Expand Down
2 changes: 1 addition & 1 deletion sdk/trace/span_processor_filter_example_test.go
Expand Up @@ -67,7 +67,7 @@ func (f InstrumentationBlacklist) ForceFlush(ctx context.Context) error {
return f.Next.ForceFlush(ctx)
}
func (f InstrumentationBlacklist) OnEnd(s ReadOnlySpan) {
if f.Blacklist != nil && f.Blacklist[s.InstrumentationLibrary().Name] {
if f.Blacklist != nil && f.Blacklist[s.InstrumentationScope().Name] {
// Drop spans from this instrumentation
return
}
Expand Down