Skip to content

Commit b35ee8f

Browse files
bhshkhbhshkh
authored
fix(bigtable): screaming uppercase metric status (#13484)
Fixes: #13466
1 parent c7f8d55 commit b35ee8f

File tree

3 files changed

+45
-12
lines changed

3 files changed

+45
-12
lines changed

‎bigtable/bigtable.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ func (c *Client) prepareStatementWithMetadata(ctx context.Context, query string,
562562

563563
preparedStatement, err = c.prepareStatement(ctx, mt, query, paramTypes, opts...)
564564
statusCode, statusErr := convertToGrpcStatusErr(err)
565-
mt.setCurrOpStatus(statusCode.String())
565+
mt.setCurrOpStatus(statusCode)
566566
return preparedStatement, statusErr
567567
}
568568

@@ -741,7 +741,7 @@ func (bs *BoundStatement) Execute(ctx context.Context, f func(ResultRow) bool, o
741741

742742
err = bs.execute(ctx, f, mt)
743743
statusCode, statusErr := convertToGrpcStatusErr(err)
744-
mt.setCurrOpStatus(statusCode.String())
744+
mt.setCurrOpStatus(statusCode)
745745
return statusErr
746746
}
747747

@@ -1033,7 +1033,7 @@ func (t *Table) ReadRows(ctx context.Context, arg RowSet, f func(Row) bool, opts
10331033

10341034
err = t.readRows(ctx, arg, f, mt, opts...)
10351035
statusCode, statusErr := convertToGrpcStatusErr(err)
1036-
mt.setCurrOpStatus(statusCode.String())
1036+
mt.setCurrOpStatus(statusCode)
10371037
return statusErr
10381038
}
10391039

@@ -1713,7 +1713,7 @@ func (t *Table) Apply(ctx context.Context, row string, m *Mutation, opts ...Appl
17131713

17141714
err = t.apply(ctx, mt, row, m, opts...)
17151715
statusCode, statusErr := convertToGrpcStatusErr(err)
1716-
mt.setCurrOpStatus(statusCode.String())
1716+
mt.setCurrOpStatus(statusCode)
17171717
return statusErr
17181718
}
17191719

@@ -2008,7 +2008,7 @@ func (t *Table) applyGroup(ctx context.Context, group []*entryErr, opts ...Apply
20082008
}, t.c.retryOption)
20092009

20102010
statusCode, statusErr := convertToGrpcStatusErr(err)
2011-
mt.setCurrOpStatus(statusCode.String())
2011+
mt.setCurrOpStatus(statusCode)
20122012
return statusErr
20132013
}
20142014

@@ -2153,7 +2153,7 @@ func (t *Table) ApplyReadModifyWrite(ctx context.Context, row string, m *ReadMod
21532153

21542154
updatedRow, err := t.applyReadModifyWrite(ctx, mt, row, m)
21552155
statusCode, statusErr := convertToGrpcStatusErr(err)
2156-
mt.setCurrOpStatus(statusCode.String())
2156+
mt.setCurrOpStatus(statusCode)
21572157
return updatedRow, statusErr
21582158
}
21592159

@@ -2234,7 +2234,7 @@ func (t *Table) SampleRowKeys(ctx context.Context) ([]string, error) {
22342234

22352235
rowKeys, err := t.sampleRowKeys(ctx, mt)
22362236
statusCode, statusErr := convertToGrpcStatusErr(err)
2237-
mt.setCurrOpStatus(statusCode.String())
2237+
mt.setCurrOpStatus(statusCode)
22382238
return rowKeys, statusErr
22392239
}
22402240

‎bigtable/metrics.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,16 @@ import (
2525
"sync/atomic"
2626
"time"
2727

28+
"regexp"
29+
"strings"
30+
2831
"cloud.google.com/go/bigtable/internal"
2932
"github.com/google/uuid"
3033
"go.opentelemetry.io/otel/attribute"
3134
"go.opentelemetry.io/otel/metric"
3235
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
3336
"google.golang.org/api/option"
37+
"google.golang.org/grpc/codes"
3438
"google.golang.org/grpc/metadata"
3539
"google.golang.org/grpc/stats"
3640
)
@@ -176,6 +180,8 @@ var (
176180
}
177181

178182
sharedLatencyStatsHandler = &latencyStatsHandler{}
183+
184+
camel = regexp.MustCompile("([a-z0-9])([A-Z])")
179185
)
180186

181187
type metricInfo struct {
@@ -727,12 +733,16 @@ func (mt *builtinMetricsTracer) recordOperationCompletion() {
727733
mt.instrumentAppBlockingLatencies.Record(mt.ctx, mt.currOp.appBlockingLatency, metric.WithAttributeSet(appBlockingLatAttrs))
728734
}
729735

730-
func (mt *builtinMetricsTracer) setCurrOpStatus(status string) {
736+
func (mt *builtinMetricsTracer) setCurrOpStatus(code codes.Code) {
731737
if !mt.builtInEnabled {
732738
return
733739
}
734740

735-
mt.currOp.setStatus(status)
741+
mt.currOp.setStatus(canonicalString(code))
742+
}
743+
744+
func canonicalString(c codes.Code) string {
745+
return strings.ToUpper(camel.ReplaceAllString(c.String(), "${1}_${2}"))
736746
}
737747

738748
func (mt *builtinMetricsTracer) incrementAppBlockingLatency(latency float64) {

‎bigtable/metrics_test.go‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ func TestToOtelMetricAttrs(t *testing.T) {
731731
method: "ReadRows",
732732
isStreaming: true,
733733
currOp: opTracer{
734-
status: codes.OK.String(),
734+
status: canonicalString(codes.OK),
735735
currAttempt: attemptTracer{
736736
startTime: time.Now(),
737737
clusterID: "my-cluster",
@@ -755,7 +755,7 @@ func TestToOtelMetricAttrs(t *testing.T) {
755755
attribute.String(monitoredResLabelKeyTable, "my-table"),
756756
attribute.String(metricLabelKeyMethod, "ReadRows"),
757757
attribute.Bool(metricLabelKeyStreamingOperation, true),
758-
attribute.String(metricLabelKeyStatus, codes.OK.String()),
758+
attribute.String(metricLabelKeyStatus, canonicalString(codes.OK)),
759759
attribute.String(monitoredResLabelKeyCluster, clusterID1),
760760
attribute.String(monitoredResLabelKeyZone, zoneID1),
761761
},
@@ -936,6 +936,11 @@ func TestFirstResponseLatencyWithDelayedStream(t *testing.T) {
936936
// Metric does not match target client UID. Skipping
937937
continue
938938
}
939+
940+
wantStatus := canonicalString(codes.OK)
941+
if strings.Contains(metricType, metricNameFirstRespLatencies) && ts.GetMetric().GetLabels()[metricLabelKeyStatus] != wantStatus {
942+
t.Errorf("Incorrect status: got: %v, want: %v", ts.GetMetric().GetLabels()[metricLabelKeyStatus], wantStatus)
943+
}
939944
// If we reach here, the metric belongs to our test client instance
940945
foundMetricsForClientUID = append(foundMetricsForClientUID, metricType)
941946

@@ -1142,7 +1147,6 @@ func TestApplicationLatencies(t *testing.T) {
11421147
if _, exists := metricLabels[metricLabelKeyStreamingOperation]; exists {
11431148
t.Errorf("Label %s should not be present for %s", metricLabelKeyStreamingOperation, expectedMetricType)
11441149
}
1145-
11461150
resLabels := ts.GetResource().GetLabels()
11471151
if tblName, ok := resLabels[monitoredResLabelKeyTable]; (ok && tblName != tableID && tblName != "") || !ok {
11481152
t.Errorf("Label %s: got %q, want %q for resource %s", monitoredResLabelKeyTable, tblName, tableID, ts.GetResource())
@@ -1155,3 +1159,22 @@ func TestApplicationLatencies(t *testing.T) {
11551159
t.Errorf("Failed to find metric %s for client UID %s", expectedMetricType, clientUID)
11561160
}
11571161
}
1162+
1163+
func TestCanonicalString(t *testing.T) {
1164+
tests := []struct {
1165+
code codes.Code
1166+
want string
1167+
}{
1168+
{codes.OK, "OK"},
1169+
{codes.Canceled, "CANCELED"},
1170+
{codes.DeadlineExceeded, "DEADLINE_EXCEEDED"},
1171+
{codes.Code(100), "CODE(100)"},
1172+
}
1173+
1174+
for _, test := range tests {
1175+
got := canonicalString(test.code)
1176+
if got != test.want {
1177+
t.Errorf("canonicalString(%v) = %q, want %q", test.code, got, test.want)
1178+
}
1179+
}
1180+
}

0 commit comments

Comments
 (0)