diff --git a/bigtable/bigtable.go b/bigtable/bigtable.go index bee6dc6dbd83..841dc8751fe0 100644 --- a/bigtable/bigtable.go +++ b/bigtable/bigtable.go @@ -562,7 +562,7 @@ func (c *Client) prepareStatementWithMetadata(ctx context.Context, query string, preparedStatement, err = c.prepareStatement(ctx, mt, query, paramTypes, opts...) statusCode, statusErr := convertToGrpcStatusErr(err) - mt.setCurrOpStatus(statusCode.String()) + mt.setCurrOpStatus(statusCode) return preparedStatement, statusErr } @@ -741,7 +741,7 @@ func (bs *BoundStatement) Execute(ctx context.Context, f func(ResultRow) bool, o err = bs.execute(ctx, f, mt) statusCode, statusErr := convertToGrpcStatusErr(err) - mt.setCurrOpStatus(statusCode.String()) + mt.setCurrOpStatus(statusCode) return statusErr } @@ -1033,7 +1033,7 @@ func (t *Table) ReadRows(ctx context.Context, arg RowSet, f func(Row) bool, opts err = t.readRows(ctx, arg, f, mt, opts...) statusCode, statusErr := convertToGrpcStatusErr(err) - mt.setCurrOpStatus(statusCode.String()) + mt.setCurrOpStatus(statusCode) return statusErr } @@ -1713,7 +1713,7 @@ func (t *Table) Apply(ctx context.Context, row string, m *Mutation, opts ...Appl err = t.apply(ctx, mt, row, m, opts...) statusCode, statusErr := convertToGrpcStatusErr(err) - mt.setCurrOpStatus(statusCode.String()) + mt.setCurrOpStatus(statusCode) return statusErr } @@ -2008,7 +2008,7 @@ func (t *Table) applyGroup(ctx context.Context, group []*entryErr, opts ...Apply }, t.c.retryOption) statusCode, statusErr := convertToGrpcStatusErr(err) - mt.setCurrOpStatus(statusCode.String()) + mt.setCurrOpStatus(statusCode) return statusErr } @@ -2153,7 +2153,7 @@ func (t *Table) ApplyReadModifyWrite(ctx context.Context, row string, m *ReadMod updatedRow, err := t.applyReadModifyWrite(ctx, mt, row, m) statusCode, statusErr := convertToGrpcStatusErr(err) - mt.setCurrOpStatus(statusCode.String()) + mt.setCurrOpStatus(statusCode) return updatedRow, statusErr } @@ -2234,7 +2234,7 @@ func (t *Table) SampleRowKeys(ctx context.Context) ([]string, error) { rowKeys, err := t.sampleRowKeys(ctx, mt) statusCode, statusErr := convertToGrpcStatusErr(err) - mt.setCurrOpStatus(statusCode.String()) + mt.setCurrOpStatus(statusCode) return rowKeys, statusErr } diff --git a/bigtable/metrics.go b/bigtable/metrics.go index 738dc6372d20..8e67f855244b 100644 --- a/bigtable/metrics.go +++ b/bigtable/metrics.go @@ -25,12 +25,16 @@ import ( "sync/atomic" "time" + "regexp" + "strings" + "cloud.google.com/go/bigtable/internal" "github.com/google/uuid" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" sdkmetric "go.opentelemetry.io/otel/sdk/metric" "google.golang.org/api/option" + "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" "google.golang.org/grpc/stats" ) @@ -176,6 +180,8 @@ var ( } sharedLatencyStatsHandler = &latencyStatsHandler{} + + camel = regexp.MustCompile("([a-z0-9])([A-Z])") ) type metricInfo struct { @@ -727,12 +733,16 @@ func (mt *builtinMetricsTracer) recordOperationCompletion() { mt.instrumentAppBlockingLatencies.Record(mt.ctx, mt.currOp.appBlockingLatency, metric.WithAttributeSet(appBlockingLatAttrs)) } -func (mt *builtinMetricsTracer) setCurrOpStatus(status string) { +func (mt *builtinMetricsTracer) setCurrOpStatus(code codes.Code) { if !mt.builtInEnabled { return } - mt.currOp.setStatus(status) + mt.currOp.setStatus(canonicalString(code)) +} + +func canonicalString(c codes.Code) string { + return strings.ToUpper(camel.ReplaceAllString(c.String(), "${1}_${2}")) } func (mt *builtinMetricsTracer) incrementAppBlockingLatency(latency float64) { diff --git a/bigtable/metrics_test.go b/bigtable/metrics_test.go index 368405986010..cc86a9166555 100644 --- a/bigtable/metrics_test.go +++ b/bigtable/metrics_test.go @@ -731,7 +731,7 @@ func TestToOtelMetricAttrs(t *testing.T) { method: "ReadRows", isStreaming: true, currOp: opTracer{ - status: codes.OK.String(), + status: canonicalString(codes.OK), currAttempt: attemptTracer{ startTime: time.Now(), clusterID: "my-cluster", @@ -755,7 +755,7 @@ func TestToOtelMetricAttrs(t *testing.T) { attribute.String(monitoredResLabelKeyTable, "my-table"), attribute.String(metricLabelKeyMethod, "ReadRows"), attribute.Bool(metricLabelKeyStreamingOperation, true), - attribute.String(metricLabelKeyStatus, codes.OK.String()), + attribute.String(metricLabelKeyStatus, canonicalString(codes.OK)), attribute.String(monitoredResLabelKeyCluster, clusterID1), attribute.String(monitoredResLabelKeyZone, zoneID1), }, @@ -936,6 +936,11 @@ func TestFirstResponseLatencyWithDelayedStream(t *testing.T) { // Metric does not match target client UID. Skipping continue } + + wantStatus := canonicalString(codes.OK) + if strings.Contains(metricType, metricNameFirstRespLatencies) && ts.GetMetric().GetLabels()[metricLabelKeyStatus] != wantStatus { + t.Errorf("Incorrect status: got: %v, want: %v", ts.GetMetric().GetLabels()[metricLabelKeyStatus], wantStatus) + } // If we reach here, the metric belongs to our test client instance foundMetricsForClientUID = append(foundMetricsForClientUID, metricType) @@ -1142,7 +1147,6 @@ func TestApplicationLatencies(t *testing.T) { if _, exists := metricLabels[metricLabelKeyStreamingOperation]; exists { t.Errorf("Label %s should not be present for %s", metricLabelKeyStreamingOperation, expectedMetricType) } - resLabels := ts.GetResource().GetLabels() if tblName, ok := resLabels[monitoredResLabelKeyTable]; (ok && tblName != tableID && tblName != "") || !ok { 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) { t.Errorf("Failed to find metric %s for client UID %s", expectedMetricType, clientUID) } } + +func TestCanonicalString(t *testing.T) { + tests := []struct { + code codes.Code + want string + }{ + {codes.OK, "OK"}, + {codes.Canceled, "CANCELED"}, + {codes.DeadlineExceeded, "DEADLINE_EXCEEDED"}, + {codes.Code(100), "CODE(100)"}, + } + + for _, test := range tests { + got := canonicalString(test.code) + if got != test.want { + t.Errorf("canonicalString(%v) = %q, want %q", test.code, got, test.want) + } + } +}