Skip to content
This repository was archived by the owner on Apr 2, 2024. It is now read-only.

Commit 886c4af

Browse files
harkishenharkishen
committed
implement suggestions - 1.
Signed-off-by: Harkishen-Singh <[email protected]>
1 parent 5ee0244 commit 886c4af

File tree

4 files changed

+56
-46
lines changed

4 files changed

+56
-46
lines changed

‎pkg/jaeger/query/query.go

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/jaegertracing/jaeger/model"
1515
"github.com/jaegertracing/jaeger/storage/dependencystore"
1616
"github.com/jaegertracing/jaeger/storage/spanstore"
17+
1718
"github.com/timescale/promscale/pkg/log"
1819
"github.com/timescale/promscale/pkg/pgmodel/metrics"
1920
"github.com/timescale/promscale/pkg/pgxconn"
@@ -47,12 +48,12 @@ func (p *Query) GetTrace(ctx context.Context, traceID model.TraceID) (*model.Tra
4748
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_trace", "code": ""}).Inc()
4849
start := time.Now()
4950
res, err := getTrace(ctx, p.conn, traceID)
50-
if err == nil {
51-
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_trace", "code": "200"}).Inc()
52-
traceRequestsExec.Add(1)
53-
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "get_trace"}).Observe(time.Since(start).Seconds())
54-
} else {
51+
if err != nil {
5552
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_trace", "code": "500"}).Inc()
53+
} else {
54+
traceRequestsExec.Add(1)
55+
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_trace", "code": "200"}).Inc()
56+
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "get_trace", "code": "200"}).Observe(time.Since(start).Seconds())
5657
}
5758
return res, logError(err)
5859
}
@@ -61,11 +62,11 @@ func (p *Query) GetServices(ctx context.Context) ([]string, error) {
6162
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_services", "code": ""}).Inc()
6263
start := time.Now()
6364
res, err := getServices(ctx, p.conn)
64-
if err == nil {
65-
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_services", "code": "200"}).Inc()
66-
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "get_services"}).Observe(time.Since(start).Seconds())
67-
} else {
65+
if err != nil {
6866
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_services", "code": "500"}).Inc()
67+
} else {
68+
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_services", "code": "200"}).Inc()
69+
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "get_services", "code": "200"}).Observe(time.Since(start).Seconds())
6970
}
7071
return res, logError(err)
7172
}
@@ -74,11 +75,11 @@ func (p *Query) GetOperations(ctx context.Context, query spanstore.OperationQuer
7475
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_operations", "code": ""}).Inc()
7576
start := time.Now()
7677
res, err := getOperations(ctx, p.conn, query)
77-
if err == nil {
78-
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_operations", "code": "200"}).Inc()
79-
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "get_operations"}).Observe(time.Since(start).Seconds())
80-
} else {
78+
if err != nil {
8179
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_operations", "code": "500"}).Inc()
80+
} else {
81+
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_operations", "code": "200"}).Inc()
82+
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "get_operations", "code": "200"}).Observe(time.Since(start).Seconds())
8283
}
8384
return res, logError(err)
8485
}
@@ -87,12 +88,12 @@ func (p *Query) FindTraces(ctx context.Context, query *spanstore.TraceQueryParam
8788
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "find_traces", "code": ""}).Inc()
8889
start := time.Now()
8990
res, err := findTraces(ctx, p.conn, query)
90-
if err == nil {
91-
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "find_traces", "code": "200"}).Inc()
92-
traceRequestsExec.Add(1)
93-
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "find_traces"}).Observe(time.Since(start).Seconds())
94-
} else {
91+
if err != nil {
9592
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "find_traces", "code": "500"}).Inc()
93+
} else {
94+
traceRequestsExec.Add(1)
95+
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "find_traces", "code": "200"}).Inc()
96+
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "find_traces", "code": "200"}).Observe(time.Since(start).Seconds())
9697
}
9798
return res, logError(err)
9899
}
@@ -101,12 +102,12 @@ func (p *Query) FindTraceIDs(ctx context.Context, query *spanstore.TraceQueryPar
101102
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "find_trace_ids", "code": ""}).Inc()
102103
start := time.Now()
103104
res, err := findTraceIDs(ctx, p.conn, query)
104-
if err == nil {
105-
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "find_trace_ids", "code": "200"}).Inc()
106-
traceRequestsExec.Add(1)
107-
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "find_trace_ids"}).Observe(time.Since(start).Seconds())
108-
} else {
105+
if err != nil {
109106
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "find_trace_ids", "code": "500"}).Inc()
107+
} else {
108+
traceRequestsExec.Add(1)
109+
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "find_trace_ids", "code": "200"}).Inc()
110+
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "find_trace_ids", "code": "200"}).Observe(time.Since(start).Seconds())
110111
}
111112
return res, logError(err)
112113
}
@@ -115,12 +116,12 @@ func (p *Query) GetDependencies(ctx context.Context, endTs time.Time, lookback t
115116
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_dependencies", "code": ""}).Inc()
116117
start := time.Now()
117118
res, err := getDependencies(ctx, p.conn, endTs, lookback)
118-
if err == nil {
119-
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_dependencies", "code": "200"}).Inc()
120-
dependencyRequestsExec.Add(1)
121-
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "get_dependencies"}).Observe(time.Since(start).Seconds())
122-
} else {
119+
if err != nil {
123120
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_dependencies", "code": "500"}).Inc()
121+
} else {
122+
dependencyRequestsExec.Add(1)
123+
metrics.RequestsTotal.With(prometheus.Labels{"type": "trace", "handler": "get_dependencies", "code": "200"}).Inc()
124+
metrics.RequestsDuration.With(prometheus.Labels{"type": "trace", "handler": "get_dependencies", "code": "200"}).Observe(time.Since(start).Seconds())
124125
}
125126
return res, logError(err)
126127
}

‎pkg/pgmodel/ingestor/trace/cache.go

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"github.com/prometheus/client_golang/prometheus"
55
"github.com/timescale/promscale/pkg/clockcache"
66
"github.com/timescale/promscale/pkg/util"
7+
"sync"
78
)
89

910
const (
@@ -13,20 +14,40 @@ const (
1314
tagCacheSize = 10000
1415
)
1516

17+
// Only for tests: Metrics get registered twice in E2E tests, leading to panic.
18+
// Hence, we must register only once.
19+
var regSchema, regOp, regInst, regTag sync.Once
20+
1621
func newSchemaCache() *clockcache.Cache {
17-
return clockcache.WithMax(urlCacheSize)
22+
c := clockcache.WithMax(urlCacheSize)
23+
regSchema.Do(func() {
24+
registerMetrics("schema", c)
25+
})
26+
return c
1827
}
1928

2029
func newOperationCache() *clockcache.Cache {
21-
return clockcache.WithMax(operationCacheSize)
30+
c := clockcache.WithMax(operationCacheSize)
31+
regOp.Do(func() {
32+
registerMetrics("operation", c)
33+
})
34+
return c
2235
}
2336

2437
func newInstrumentationLibraryCache() *clockcache.Cache {
25-
return clockcache.WithMax(instLibCacheSize)
38+
c := clockcache.WithMax(instLibCacheSize)
39+
regInst.Do(func() {
40+
registerMetrics("instrumentation_lib", c)
41+
})
42+
return c
2643
}
2744

2845
func newTagCache() *clockcache.Cache {
29-
return clockcache.WithMax(tagCacheSize)
46+
c := clockcache.WithMax(tagCacheSize)
47+
regTag.Do(func() {
48+
registerMetrics("tag", c)
49+
})
50+
return c
3051
}
3152

3253
func registerMetrics(cacheName string, c *clockcache.Cache) {

‎pkg/pgmodel/ingestor/trace/writer.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"context"
99
"encoding/binary"
1010
"fmt"
11-
"sync"
1211
"time"
1312

1413
"github.com/prometheus/client_golang/prometheus"
@@ -60,25 +59,14 @@ type traceWriterImpl struct {
6059
tagCache *clockcache.Cache
6160
}
6261

63-
var register sync.Once
64-
6562
func NewWriter(conn pgxconn.PgxConn) *traceWriterImpl {
66-
writer := &traceWriterImpl{
63+
return &traceWriterImpl{
6764
conn: conn,
6865
schemaCache: newSchemaCache(),
6966
instLibCache: newInstrumentationLibraryCache(),
7067
opCache: newOperationCache(),
7168
tagCache: newTagCache(),
7269
}
73-
register.Do(func() {
74-
// Only for tests: These metrics get registered twice in E2E tests, leading to panic.
75-
// Hence, we must register only once.
76-
registerMetrics("schema", writer.schemaCache)
77-
registerMetrics("instrumentation_library", writer.instLibCache)
78-
registerMetrics("operation", writer.opCache)
79-
registerMetrics("tag", writer.tagCache)
80-
})
81-
return writer
8270
}
8371

8472
func (t *traceWriterImpl) queueSpanLinks(linkBatch pgxconn.PgxBatch, tagsBatch tagBatch, links pdata.SpanLinkSlice, traceID pgtype.UUID, spanID pgtype.Int8, spanStartTime time.Time) error {

‎pkg/pgmodel/metrics/metrics.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ var (
120120
Help: "Time taken by function to respond to query.",
121121
Buckets: []float64{0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, 30, 50, 100, 250, 500, 1000, 2500},
122122
},
123-
[]string{"type", "handler"},
123+
[]string{"type", "handler", "code"},
124124
)
125125
RequestsTotal = prometheus.NewCounterVec(
126126
prometheus.CounterOpts{

0 commit comments

Comments
 (0)