Skip to content

Commit

Permalink
Deprecate Library and move all uses to Scope (#2977)
Browse files Browse the repository at this point in the history
* Deprecate Library and move all uses to Scope

* Add PR number to changelog

* Don't change signatures in stable modules

* Revert some changes

* Rename internal struct names

* A bit more renaming

* Update sdk/trace/span.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update based on feedback

* Revert change

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
  • Loading branch information
3 people committed Jul 6, 2022
1 parent 5795c70 commit 575e1bb
Show file tree
Hide file tree
Showing 18 changed files with 155 additions and 121 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Expand Up @@ -17,6 +17,13 @@ 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.
Use the equivalent `Scope` struct instead. (#2977)
- The `ReadOnlySpan.InstrumentationLibrary` method from the `go.opentelemetry.io/otel/sdk/trace` package is deprecated.
Use the equivalent `ReadOnlySpan.InstrumentationScope` method 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 {
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.
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

0 comments on commit 575e1bb

Please sign in to comment.