Skip to content

Commit 224c947

Browse files
ianthehatianthehat
committed
internal/telemetry: replace TagSet with TagMap and TagPointer
This separates the concerns of tag collections that have to be iterated and tag collections that need lookup by key. Also make it so that events just carry a plain slice of tags. We pass a TagMap down through the exporters and allow it to be extended on the way. We no longer need the event.Query method (or the event type) We now exclusivley use Key as the identity, and no longer have a common core implementation but just implement it directly in each type. This removes some confusion that was causing the same key through different paths to end up with a different identity. Change-Id: I61e47adcb397f4ca83dd90342b021dd8e9571ed3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/224278 Run-TryBot: Ian Cottrell <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Emmanuel Odeke <[email protected]>
1 parent 62abcc1 commit 224c947

File tree

24 files changed

+994
-508
lines changed

24 files changed

+994
-508
lines changed

‎internal/lsp/debug/metrics.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ var (
4949
}
5050
)
5151

52-
func registerMetrics(m *metric.Exporter) {
52+
func registerMetrics(m *metric.Config) {
5353
receivedBytes.Record(m, tag.ReceivedBytes)
5454
sentBytes.Record(m, tag.SentBytes)
5555
latency.Record(m, tag.Latency)

‎internal/lsp/debug/rpc.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,21 @@ type rpcCodeBucket struct {
9191
Count int64
9292
}
9393

94-
func (r *rpcs) ProcessEvent(ctx context.Context, ev event.Event) (context.Context, event.Event) {
94+
func (r *rpcs) ProcessEvent(ctx context.Context, ev event.Event, tagMap event.TagMap) context.Context {
9595
if !ev.IsRecord() {
96-
return ctx, ev
96+
return ctx
9797
}
9898
r.mu.Lock()
9999
defer r.mu.Unlock()
100-
metrics := metric.Entries.Get(ev.Tags).([]metric.Data)
100+
metrics := metric.Entries.Get(tagMap).([]metric.Data)
101101
for _, data := range metrics {
102102
for i, group := range data.Groups() {
103103
set := &r.Inbound
104-
if tag.RPCDirection.Get(group) == tag.Outbound {
104+
groupTags := event.NewTagMap(group...)
105+
if tag.RPCDirection.Get(groupTags) == tag.Outbound {
105106
set = &r.Outbound
106107
}
107-
method := tag.Method.Get(group)
108+
method := tag.Method.Get(groupTags)
108109
index := sort.Search(len(*set), func(i int) bool {
109110
return (*set)[i].Method >= method
110111
})
@@ -120,7 +121,7 @@ func (r *rpcs) ProcessEvent(ctx context.Context, ev event.Event) (context.Contex
120121
case started.Name:
121122
stats.Started = data.(*metric.Int64Data).Rows[i]
122123
case completed.Name:
123-
status := tag.StatusCode.Get(group)
124+
status := tag.StatusCode.Get(groupTags)
124125
var b *rpcCodeBucket
125126
for c, entry := range stats.Codes {
126127
if entry.Key == status {
@@ -180,7 +181,7 @@ func (r *rpcs) ProcessEvent(ctx context.Context, ev event.Event) (context.Contex
180181
stats.InProgress = stats.Started - stats.Completed
181182
}
182183
}
183-
return ctx, ev
184+
return ctx
184185
}
185186

186187
func (r *rpcs) getData(req *http.Request) interface{} {

‎internal/lsp/debug/serve.go

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ type Instance struct {
5252

5353
LogWriter io.Writer
5454

55-
metrics metric.Exporter
55+
exporter event.Exporter
56+
5657
ocagent *ocagent.Exporter
5758
prometheus *prometheus.Exporter
5859
rpcs *rpcs
@@ -383,7 +384,7 @@ func getMemory(r *http.Request) interface{} {
383384
}
384385

385386
func init() {
386-
event.SetExporter(makeExporter(os.Stderr))
387+
event.SetExporter(makeGlobalExporter(os.Stderr))
387388
}
388389

389390
func GetInstance(ctx context.Context) *Instance {
@@ -406,7 +407,6 @@ func WithInstance(ctx context.Context, workdir, agent string) context.Context {
406407
OCAgentConfig: agent,
407408
}
408409
i.LogWriter = os.Stderr
409-
registerMetrics(&i.metrics)
410410
ocConfig := ocagent.Discover()
411411
//TODO: we should not need to adjust the discovered configuration
412412
ocConfig.Address = i.OCAgentConfig
@@ -415,6 +415,7 @@ func WithInstance(ctx context.Context, workdir, agent string) context.Context {
415415
i.rpcs = &rpcs{}
416416
i.traces = &traces{}
417417
i.State = &State{}
418+
i.exporter = makeInstanceExporter(i)
418419
return context.WithValue(ctx, instanceKey, i)
419420
}
420421

@@ -539,33 +540,42 @@ func (i *Instance) writeMemoryDebug(threshold uint64) error {
539540
return nil
540541
}
541542

542-
func makeExporter(stderr io.Writer) event.Exporter {
543-
return func(ctx context.Context, ev event.Event) (context.Context, event.Event) {
544-
ctx, ev = export.ContextSpan(ctx, ev)
543+
func makeGlobalExporter(stderr io.Writer) event.Exporter {
544+
return func(ctx context.Context, ev event.Event, tags event.TagMap) context.Context {
545545
i := GetInstance(ctx)
546546
if ev.IsLog() && (ev.Error != nil || i == nil) {
547547
fmt.Fprintf(stderr, "%v\n", ev)
548548
}
549-
ctx, ev = protocol.LogEvent(ctx, ev)
549+
ctx = protocol.LogEvent(ctx, ev, tags)
550550
if i == nil {
551-
return ctx, ev
551+
return ctx
552552
}
553-
ctx, ev = export.Tag(ctx, ev)
554-
ctx, ev = i.metrics.ProcessEvent(ctx, ev)
553+
return i.exporter(ctx, ev, tags)
554+
}
555+
}
556+
557+
func makeInstanceExporter(i *Instance) event.Exporter {
558+
exporter := func(ctx context.Context, ev event.Event, tags event.TagMap) context.Context {
555559
if i.ocagent != nil {
556-
ctx, ev = i.ocagent.ProcessEvent(ctx, ev)
560+
ctx = i.ocagent.ProcessEvent(ctx, ev, tags)
557561
}
558562
if i.prometheus != nil {
559-
ctx, ev = i.prometheus.ProcessEvent(ctx, ev)
563+
ctx = i.prometheus.ProcessEvent(ctx, ev, tags)
560564
}
561565
if i.rpcs != nil {
562-
ctx, ev = i.rpcs.ProcessEvent(ctx, ev)
566+
ctx = i.rpcs.ProcessEvent(ctx, ev, tags)
563567
}
564568
if i.traces != nil {
565-
ctx, ev = i.traces.ProcessEvent(ctx, ev)
569+
ctx = i.traces.ProcessEvent(ctx, ev, tags)
566570
}
567-
return ctx, ev
568-
}
571+
return ctx
572+
}
573+
metrics := metric.Config{}
574+
registerMetrics(&metrics)
575+
exporter = metrics.Exporter(exporter)
576+
exporter = export.Spans(exporter)
577+
exporter = export.Labels(exporter)
578+
return exporter
569579
}
570580

571581
type dataFunc func(*http.Request) interface{}

‎internal/lsp/debug/trace.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,12 @@ type traceEvent struct {
7373
Tags string
7474
}
7575

76-
func (t *traces) ProcessEvent(ctx context.Context, ev event.Event) (context.Context, event.Event) {
76+
func (t *traces) ProcessEvent(ctx context.Context, ev event.Event, tags event.TagMap) context.Context {
7777
t.mu.Lock()
7878
defer t.mu.Unlock()
7979
span := export.GetSpan(ctx)
8080
if span == nil {
81-
return ctx, ev
81+
return ctx
8282
}
8383

8484
switch {
@@ -93,37 +93,37 @@ func (t *traces) ProcessEvent(ctx context.Context, ev event.Event) (context.Cont
9393
SpanID: span.ID.SpanID,
9494
ParentID: span.ParentID,
9595
Name: span.Name,
96-
Start: span.Start,
97-
Tags: renderTags(span.Tags),
96+
Start: span.Start.At,
97+
Tags: renderTags(span.Start.Tags()),
9898
}
9999
t.unfinished[span.ID] = td
100100
// and wire up parents if we have them
101101
if !span.ParentID.IsValid() {
102-
return ctx, ev
102+
return ctx
103103
}
104104
parentID := export.SpanContext{TraceID: span.ID.TraceID, SpanID: span.ParentID}
105105
parent, found := t.unfinished[parentID]
106106
if !found {
107107
// trace had an invalid parent, so it cannot itself be valid
108-
return ctx, ev
108+
return ctx
109109
}
110110
parent.Children = append(parent.Children, td)
111111

112112
case ev.IsEndSpan():
113113
// finishing, must be already in the map
114114
td, found := t.unfinished[span.ID]
115115
if !found {
116-
return ctx, ev // if this happens we are in a bad place
116+
return ctx // if this happens we are in a bad place
117117
}
118118
delete(t.unfinished, span.ID)
119119

120-
td.Finish = span.Finish
121-
td.Duration = span.Finish.Sub(span.Start)
120+
td.Finish = span.Finish.At
121+
td.Duration = span.Finish.At.Sub(span.Start.At)
122122
td.Events = make([]traceEvent, len(span.Events))
123123
for i, event := range span.Events {
124124
td.Events[i] = traceEvent{
125125
Time: event.At,
126-
Tags: renderTags(event.Tags),
126+
Tags: renderTags(event.Tags()),
127127
}
128128
}
129129

@@ -140,7 +140,7 @@ func (t *traces) ProcessEvent(ctx context.Context, ev event.Event) (context.Cont
140140
fillOffsets(td, td.Start)
141141
}
142142
}
143-
return ctx, ev
143+
return ctx
144144
}
145145

146146
func (t *traces) getData(req *http.Request) interface{} {
@@ -169,11 +169,11 @@ func fillOffsets(td *traceData, start time.Time) {
169169
}
170170
}
171171

172-
func renderTags(tags event.TagSet) string {
172+
func renderTags(tags event.TagIterator) string {
173173
buf := &bytes.Buffer{}
174-
for i := tags.Iterator(); i.Next(); {
175-
tag := i.Value()
176-
fmt.Fprintf(buf, "%s=%q ", tag.Key().Name(), tag.Value())
174+
for ; tags.Valid(); tags.Advance() {
175+
tag := tags.Tag()
176+
fmt.Fprintf(buf, "%s=%q ", tag.Key.Name(), tag.Value)
177177
}
178178
return buf.String()
179179
}

‎internal/lsp/protocol/context.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,18 @@ func WithClient(ctx context.Context, client Client) context.Context {
1818
return context.WithValue(ctx, clientKey, client)
1919
}
2020

21-
func LogEvent(ctx context.Context, ev event.Event) (context.Context, event.Event) {
21+
func LogEvent(ctx context.Context, ev event.Event, tags event.TagMap) context.Context {
2222
if !ev.IsLog() {
23-
return ctx, ev
23+
return ctx
2424
}
2525
client, ok := ctx.Value(clientKey).(Client)
2626
if !ok {
27-
return ctx, ev
27+
return ctx
2828
}
2929
msg := &LogMessageParams{Type: Info, Message: fmt.Sprint(ev)}
3030
if ev.Error != nil {
3131
msg.Type = Error
3232
}
3333
go client.LogMessage(xcontext.Detach(ctx), msg)
34-
return ctx, ev
34+
return ctx
3535
}

‎internal/telemetry/bench_test.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,4 @@ func (nw *noopWriter) Write(b []byte) (int, error) {
142142
return len(b), nil
143143
}
144144

145-
var noopLogger = export.LogWriter(new(noopWriter), false)
146-
147-
func noopExporter(ctx context.Context, ev event.Event) (context.Context, event.Event) {
148-
ctx, ev = export.ContextSpan(ctx, ev)
149-
return noopLogger(ctx, ev)
150-
}
145+
var noopExporter = export.Spans(export.LogWriter(new(noopWriter), false))

‎internal/telemetry/event/event.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ const (
1717
StartSpanType
1818
EndSpanType
1919
LabelType
20-
QueryType
2120
DetachType
2221
RecordType
2322
)
@@ -27,14 +26,14 @@ type Event struct {
2726
At time.Time
2827
Message string
2928
Error error
30-
Tags TagSet
29+
30+
tags []Tag
3131
}
3232

3333
func (e Event) IsLog() bool { return e.Type == LogType }
3434
func (e Event) IsEndSpan() bool { return e.Type == EndSpanType }
3535
func (e Event) IsStartSpan() bool { return e.Type == StartSpanType }
3636
func (e Event) IsLabel() bool { return e.Type == LabelType }
37-
func (e Event) IsQuery() bool { return e.Type == QueryType }
3837
func (e Event) IsDetach() bool { return e.Type == DetachType }
3938
func (e Event) IsRecord() bool { return e.Type == RecordType }
4039

@@ -50,8 +49,19 @@ func (e Event) Format(f fmt.State, r rune) {
5049
fmt.Fprintf(f, ": %v", e.Error)
5150
}
5251
}
53-
for i := e.Tags.Iterator(); i.Next(); {
54-
tag := i.Value()
55-
fmt.Fprintf(f, "\n\t%s = %v", tag.key.name, tag.value)
52+
for it := e.Tags(); it.Valid(); it.Advance() {
53+
tag := it.Tag()
54+
fmt.Fprintf(f, "\n\t%s = %v", tag.Key.Name(), tag.Value)
55+
}
56+
}
57+
58+
func (ev Event) Tags() TagIterator {
59+
if len(ev.tags) == 0 {
60+
return TagIterator{}
5661
}
62+
return NewTagIterator(ev.tags...)
63+
}
64+
65+
func (ev Event) Map() TagMap {
66+
return NewTagMap(ev.tags...)
5767
}

‎internal/telemetry/event/export.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212

1313
// Exporter is a function that handles events.
1414
// It may return a modified context and event.
15-
type Exporter func(context.Context, Event) (context.Context, Event)
15+
type Exporter func(context.Context, Event, TagMap) context.Context
1616

1717
var (
1818
exporter unsafe.Pointer
@@ -34,11 +34,11 @@ func SetExporter(e Exporter) {
3434
}
3535

3636
// ProcessEvent is called to deliver an event to the global exporter.
37-
func ProcessEvent(ctx context.Context, ev Event) (context.Context, Event) {
37+
func ProcessEvent(ctx context.Context, ev Event) context.Context {
3838
exporterPtr := (*Exporter)(atomic.LoadPointer(&exporter))
3939
if exporterPtr == nil {
40-
return ctx, ev
40+
return ctx
4141
}
4242
// and now also hand the event of to the current exporter
43-
return (*exporterPtr)(ctx, ev)
43+
return (*exporterPtr)(ctx, ev, ev.Map())
4444
}

0 commit comments

Comments
 (0)