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.

Sign up for GitHub

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

[POC] Make scope attributes as identifying for Tracer, Meter, Logger #5806

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

[POC] Make scope attributes as identifying for Tracer, Meter, Logger #5806

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cff632e
instrumentation: Add Attributes to Scope and make scope attributes as…
pellared Sep 11, 2024
9238189
Fix typo
pellared Sep 11, 2024
b063b1c
Add missing dot
pellared Sep 11, 2024
dba9c61
OTLP exporters set InstrumentationScope.Attributes
pellared Sep 11, 2024
437cce1
Update stdoutlog tests
pellared Sep 11, 2024
54f8092
Update stdoutmetric test
pellared Sep 11, 2024
182ea85
Update stdouttrace test
pellared Sep 11, 2024
7b2bb8c
Fix sdk/trace tests
pellared Sep 11, 2024
07bfb33
Update SameLoggers test
pellared Sep 11, 2024
7710a1f
Update same loggers
pellared Sep 11, 2024
ea10e40
createScopeInfoMetric to handle scope attributes
pellared Oct 28, 2024
3af499a
Merge branch 'main' into scope-attr
pellared Oct 28, 2024
ea5fbd9
Fix new test
pellared Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions exporters/otlp/otlplog/otlploggrpc/internal/transform/log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ func scopeLogsMap(dst *map[instrumentation.Scope]*lpb.ScopeLogs, records []log.R
var emptyScope instrumentation.Scope
if scope != emptyScope {
sl.Scope = &cpb.InstrumentationScope{
Name: scope.Name,
Version: scope.Version,
Name: scope.Name,
Version: scope.Version,
Attributes: AttrIter(scope.Attributes.Iter()),
}
sl.SchemaUrl = scope.SchemaURL
}
Expand Down
5 changes: 3 additions & 2 deletions exporters/otlp/otlplog/otlploghttp/internal/transform/log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ func scopeLogsMap(dst *map[instrumentation.Scope]*lpb.ScopeLogs, records []log.R
var emptyScope instrumentation.Scope
if scope != emptyScope {
sl.Scope = &cpb.InstrumentationScope{
Name: scope.Name,
Version: scope.Version,
Name: scope.Name,
Version: scope.Version,
Attributes: AttrIter(scope.Attributes.Iter()),
}
sl.SchemaUrl = scope.SchemaURL
}
Expand Down
5 changes: 3 additions & 2 deletions exporters/otlp/otlpmetric/otlpmetricgrpc/internal/transform/metricdata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ func ScopeMetrics(sms []metricdata.ScopeMetrics) ([]*mpb.ScopeMetrics, error) {

out = append(out, &mpb.ScopeMetrics{
Scope: &cpb.InstrumentationScope{
Name: sm.Scope.Name,
Version: sm.Scope.Version,
Name: sm.Scope.Name,
Version: sm.Scope.Version,
Attributes: AttrIter(sm.Scope.Attributes.Iter()),
},
Metrics: ms,
SchemaUrl: sm.Scope.SchemaURL,
Expand Down
5 changes: 3 additions & 2 deletions exporters/otlp/otlpmetric/otlpmetrichttp/internal/transform/metricdata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ func ScopeMetrics(sms []metricdata.ScopeMetrics) ([]*mpb.ScopeMetrics, error) {

out = append(out, &mpb.ScopeMetrics{
Scope: &cpb.InstrumentationScope{
Name: sm.Scope.Name,
Version: sm.Scope.Version,
Name: sm.Scope.Name,
Version: sm.Scope.Version,
Attributes: AttrIter(sm.Scope.Attributes.Iter()),
},
Metrics: ms,
SchemaUrl: sm.Scope.SchemaURL,
Expand Down
5 changes: 3 additions & 2 deletions exporters/otlp/otlptrace/internal/tracetransform/instrumentation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ func InstrumentationScope(il instrumentation.Scope) *commonpb.InstrumentationSco
return nil
}
return &commonpb.InstrumentationScope{
Name: il.Name,
Version: il.Version,
Name: il.Name,
Version: il.Version,
Attributes: Iterator(il.Attributes.Iter()),
}
}
5 changes: 3 additions & 2 deletions exporters/stdout/stdoutlog/exporter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func getJSON(now *time.Time) string {
timestamps = "\"Timestamp\":" + string(serializedNow) + ",\"ObservedTimestamp\":" + string(serializedNow) + ","
}

return "{" + timestamps + "\"Severity\":9,\"SeverityText\":\"INFO\",\"Body\":{\"Type\":\"String\",\"Value\":\"test\"},\"Attributes\":[{\"Key\":\"key\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"key2\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"key3\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"key4\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"key5\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"bool\",\"Value\":{\"Type\":\"Bool\",\"Value\":true}}],\"TraceID\":\"0102030405060708090a0b0c0d0e0f10\",\"SpanID\":\"0102030405060708\",\"TraceFlags\":\"01\",\"Resource\":[{\"Key\":\"foo\",\"Value\":{\"Type\":\"STRING\",\"Value\":\"bar\"}}],\"Scope\":{\"Name\":\"name\",\"Version\":\"version\",\"SchemaURL\":\"https://example.com/custom-schema\"},\"DroppedAttributes\":10}\n"
return "{" + timestamps + "\"Severity\":9,\"SeverityText\":\"INFO\",\"Body\":{\"Type\":\"String\",\"Value\":\"test\"},\"Attributes\":[{\"Key\":\"key\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"key2\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"key3\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"key4\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"key5\",\"Value\":{\"Type\":\"String\",\"Value\":\"value\"}},{\"Key\":\"bool\",\"Value\":{\"Type\":\"Bool\",\"Value\":true}}],\"TraceID\":\"0102030405060708090a0b0c0d0e0f10\",\"SpanID\":\"0102030405060708\",\"TraceFlags\":\"01\",\"Resource\":[{\"Key\":\"foo\",\"Value\":{\"Type\":\"STRING\",\"Value\":\"bar\"}}],\"Scope\":{\"Name\":\"name\",\"Version\":\"version\",\"SchemaURL\":\"https://example.com/custom-schema\",\"Attributes\":{}},\"DroppedAttributes\":10}\n"
Copy link
Member

Choose a reason for hiding this comment

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

Should we populate values to attributes to make sure the output actually has a value?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree. Leaving this open to not forget about it when I will be working on actual implementation.

}

func getJSONs(now *time.Time) string {
Expand Down Expand Up @@ -263,7 +263,8 @@ func getPrettyJSON(now *time.Time) string {
"Scope": {
"Name": "name",
"Version": "version",
"SchemaURL": "https://example.com/custom-schema"
"SchemaURL": "https://example.com/custom-schema",
"Attributes": {}
},
"DroppedAttributes": 10
}
Expand Down
3 changes: 2 additions & 1 deletion exporters/stdout/stdoutmetric/example_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ func Example() {
// "Scope": {
// "Name": "example",
// "Version": "0.0.1",
// "SchemaURL": ""
// "SchemaURL": "",
// "Attributes": null
// },
// "Metrics": [
// {
Expand Down
6 changes: 4 additions & 2 deletions exporters/stdout/stdouttrace/trace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,14 @@ func expectedJSON(now time.Time) string {
"InstrumentationScope": {
"Name": "",
"Version": "",
"SchemaURL": ""
"SchemaURL": "",
"Attributes": null
},
"InstrumentationLibrary": {
"Name": "",
"Version": "",
"SchemaURL": ""
"SchemaURL": "",
"Attributes": null
}
}
`
Expand Down
1 change: 1 addition & 0 deletions internal/global/meter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ func (p *meterProvider) Meter(name string, opts ...metric.MeterOption) metric.Me
name: name,
version: c.InstrumentationVersion(),
schema: c.SchemaURL(),
attrs: c.InstrumentationAttributes(),
}

if p.meters == nil {
Expand Down
8 changes: 7 additions & 1 deletion internal/global/trace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
name: name,
version: c.InstrumentationVersion(),
schema: c.SchemaURL(),
attrs: c.InstrumentationAttributes(),
}

if p.tracers == nil {
Expand All @@ -102,7 +103,12 @@ func (p *tracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
return t
}

type il struct{ name, version, schema string }
type il struct {
name string
version string
schema string
attrs attribute.Set
}

// tracer is a placeholder for a trace.Tracer.
//
Expand Down
3 changes: 2 additions & 1 deletion internal/global/trace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/embedded"
"go.opentelemetry.io/otel/trace/noop"
Expand Down Expand Up @@ -209,7 +210,7 @@ func TestTraceProviderDelegatesSameInstance(t *testing.T) {
gtp := TracerProvider()
tracer := gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz"))
assert.Same(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")))
assert.Same(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")))
assert.NotSame(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz"), trace.WithInstrumentationAttributes(attribute.String("k", "v"))))

SetTracerProvider(fnTracerProvider{
tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
Expand Down
5 changes: 3 additions & 2 deletions internal/shared/otlp/otlplog/transform/log.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ func scopeLogsMap(dst *map[instrumentation.Scope]*lpb.ScopeLogs, records []log.R
var emptyScope instrumentation.Scope
if scope != emptyScope {
sl.Scope = &cpb.InstrumentationScope{
Name: scope.Name,
Version: scope.Version,
Name: scope.Name,
Version: scope.Version,
Attributes: AttrIter(scope.Attributes.Iter()),
}
sl.SchemaUrl = scope.SchemaURL
}
Expand Down
5 changes: 3 additions & 2 deletions internal/shared/otlp/otlpmetric/transform/metricdata.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ func ScopeMetrics(sms []metricdata.ScopeMetrics) ([]*mpb.ScopeMetrics, error) {

out = append(out, &mpb.ScopeMetrics{
Scope: &cpb.InstrumentationScope{
Name: sm.Scope.Name,
Version: sm.Scope.Version,
Name: sm.Scope.Name,
Version: sm.Scope.Version,
Attributes: AttrIter(sm.Scope.Attributes.Iter()),
},
Metrics: ms,
SchemaUrl: sm.Scope.SchemaURL,
Expand Down
9 changes: 8 additions & 1 deletion log/internal/global/log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,20 @@ import (
"sync"
"sync/atomic"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/log"
"go.opentelemetry.io/otel/log/embedded"
)

// instLib defines the instrumentation library a logger is created for.
//
// Do not use sdk/instrumentation (API cannot depend on the SDK).
type instLib struct{ name, version, schemaURL string }
type instLib struct {
name string
version string
schemaURL string
attrs attribute.Set
}

type loggerProvider struct {
embedded.LoggerProvider
Expand All @@ -41,6 +47,7 @@ func (p *loggerProvider) Logger(name string, options ...log.LoggerOption) log.Lo
name: name,
version: cfg.InstrumentationVersion(),
schemaURL: cfg.SchemaURL(),
attrs: cfg.InstrumentationAttributes(),
}

if p.loggers == nil {
Expand Down
4 changes: 4 additions & 0 deletions log/internal/global/log_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/log"
"go.opentelemetry.io/otel/log/embedded"
"go.opentelemetry.io/otel/log/noop"
Expand Down Expand Up @@ -126,6 +127,9 @@ func TestDelegation(t *testing.T) {
alt := provider.Logger("alt")
assert.NotSame(t, pre0, alt)

alt2 := provider.Logger(preName, log.WithInstrumentationAttributes(attribute.String("k", "v")))
assert.NotSame(t, pre0, alt2)

delegate := &testLoggerProvider{}
provider.setDelegate(delegate)

Expand Down
10 changes: 7 additions & 3 deletions log/logtest/recorder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"sync"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/log"
"go.opentelemetry.io/otel/log/embedded"
)
Expand Down Expand Up @@ -66,6 +67,8 @@ type ScopeRecords struct {
Version string
// SchemaURL of the telemetry emitted by the scope.
SchemaURL string
// Attributes of the telemetry emitted by the scope.
Attributes attribute.Set

// Records are the log records, and their associated context this
// instrumentation scope recorded.
Expand Down Expand Up @@ -104,9 +107,10 @@ func (r *Recorder) Logger(name string, opts ...log.LoggerOption) log.Logger {

nl := &logger{
scopeRecord: &ScopeRecords{
Name: name,
Version: cfg.InstrumentationVersion(),
SchemaURL: cfg.SchemaURL(),
Name: name,
Version: cfg.InstrumentationVersion(),
SchemaURL: cfg.SchemaURL(),
Attributes: cfg.InstrumentationAttributes(),
},
enabledFn: r.enabledFn,
}
Expand Down
4 changes: 4 additions & 0 deletions sdk/instrumentation/scope.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

package instrumentation // import "go.opentelemetry.io/otel/sdk/instrumentation"

import "go.opentelemetry.io/otel/attribute"

// Scope represents the instrumentation scope.
type Scope struct {
// Name is the name of the instrumentation scope. This should be the
Expand All @@ -12,4 +14,6 @@ type Scope struct {
Version string
// SchemaURL of the telemetry emitted by the scope.
SchemaURL string
// Attributes of the telemetry emitted by the scope.
Attributes attribute.Set
}
7 changes: 4 additions & 3 deletions sdk/log/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,10 @@ func (p *LoggerProvider) Logger(name string, opts ...log.LoggerOption) log.Logge

cfg := log.NewLoggerConfig(opts...)
scope := instrumentation.Scope{
Name: name,
Version: cfg.InstrumentationVersion(),
SchemaURL: cfg.SchemaURL(),
Name: name,
Version: cfg.InstrumentationVersion(),
SchemaURL: cfg.SchemaURL(),
Attributes: cfg.InstrumentationAttributes(),
}

p.loggersMu.Lock()
Expand Down
14 changes: 9 additions & 5 deletions sdk/log/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,15 @@ func TestLoggerProviderLogger(t *testing.T) {
t.Run("SameLoggers", func(t *testing.T) {
p := NewLoggerProvider()

l0, l1 := p.Logger("l0"), p.Logger("l1")
l2, l3 := p.Logger("l0"), p.Logger("l1")

assert.Same(t, l0, l2)
assert.Same(t, l1, l3)
l0, l1, l2 := p.Logger("l0"), p.Logger("l1"), p.Logger("l0", log.WithInstrumentationAttributes(attribute.String("foo", "bar")))
assert.NotSame(t, l0, l1)
assert.NotSame(t, l0, l2)
assert.NotSame(t, l1, l2)

l3, l4, l5 := p.Logger("l0"), p.Logger("l1"), p.Logger("l0", log.WithInstrumentationAttributes(attribute.String("foo", "bar")))
assert.Same(t, l0, l3)
assert.Same(t, l1, l4)
assert.Same(t, l2, l5)
})
}

Expand Down
8 changes: 5 additions & 3 deletions sdk/metric/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,17 @@ func (mp *MeterProvider) Meter(name string, options ...metric.MeterOption) metri

c := metric.NewMeterConfig(options...)
s := instrumentation.Scope{
Name: name,
Version: c.InstrumentationVersion(),
SchemaURL: c.SchemaURL(),
Name: name,
Version: c.InstrumentationVersion(),
SchemaURL: c.SchemaURL(),
Attributes: c.InstrumentationAttributes(),
}

global.Info("Meter created",
"Name", s.Name,
"Version", s.Version,
"SchemaURL", s.SchemaURL,
"Attributes", s.Attributes,
)

return mp.meters.Lookup(s, func() *meter {
Expand Down
2 changes: 2 additions & 0 deletions sdk/metric/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
api "go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/noop"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
Expand Down Expand Up @@ -95,6 +96,7 @@ func TestMeterProviderReturnsSameMeter(t *testing.T) {

assert.Same(t, mtr, mp.Meter(""))
assert.NotSame(t, mtr, mp.Meter("diff"))
assert.NotSame(t, mtr, mp.Meter("", api.WithInstrumentationAttributes(attribute.String("k", "v"))))
}

func TestEmptyMeterName(t *testing.T) {
Expand Down
9 changes: 5 additions & 4 deletions sdk/trace/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,10 @@ func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
name = defaultTracerName
}
is := instrumentation.Scope{
Name: name,
Version: c.InstrumentationVersion(),
SchemaURL: c.SchemaURL(),
Name: name,
Version: c.InstrumentationVersion(),
SchemaURL: c.SchemaURL(),
Attributes: c.InstrumentationAttributes(),
}

t, ok := func() (trace.Tracer, bool) {
Expand All @@ -168,7 +169,7 @@ func (p *TracerProvider) Tracer(name string, opts ...trace.TracerOption) trace.T
// slowing down all tracing consumers.
// - Logging code may be instrumented with tracing and deadlock because it could try
// acquiring the same non-reentrant mutex.
global.Info("Tracer created", "name", name, "version", is.Version, "schemaURL", is.SchemaURL)
global.Info("Tracer created", "name", name, "version", is.Version, "schemaURL", is.SchemaURL, "attributes", is.Attributes)
}
return t
}
Expand Down
6 changes: 5 additions & 1 deletion sdk/trace/trace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,11 @@ func cmpDiff(x, y interface{}) string {
cmp.AllowUnexported(snapshot{}),
cmp.AllowUnexported(attribute.Value{}),
cmp.AllowUnexported(Event{}),
cmp.AllowUnexported(trace.TraceState{}))
cmp.AllowUnexported(trace.TraceState{}),
cmp.Comparer(func(x, y attribute.Set) bool {
return x.Equals(&y)
}),
)
}

// checkChild is test utility function that tests that c has fields set appropriately,
Expand Down
Loading