Skip to content

Commit

Permalink
admin: fix nil panic when retrieving metric metadata
Browse files Browse the repository at this point in the history
Previously, the code that retrieved metric metadata assumed that the underlying
node would have at least a single store configured in the `MetricRecorder`
instance. This is not true on a SQL tenant since the storage node abstracts
away all store-related information.

The code now checks to make sure that at least one store is found before moving
ahead with retrieving store metric metadata.

Epic: CRDB-12100
Release note: None
  • Loading branch information
dhartunian committed Jan 30, 2023
1 parent 10ef5d9 commit 9c114e2
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
13 changes: 13 additions & 0 deletions pkg/ccl/serverccl/adminccl/tenant_admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ func TestTenantAdminAPI(t *testing.T) {
t.Run("tenant_unimplemented", func(t *testing.T) {
testUnimplementedRPCs(ctx, t, testHelper)
})

t.Run("tenant_metricmetadata", func(t *testing.T) {
testMetricMetadataRPC(ctx, t, testHelper)
})
}

func testMetricMetadataRPC(ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper) {
http := helper.TestCluster().TenantAdminHTTPClient(t, 1)
defer http.Close()

metricMetadataResp := serverpb.MetricMetadataResponse{}
http.GetJSON("/_admin/v1/metricmetadata", &metricMetadataResp)
require.NotEmpty(t, metricMetadataResp.Metadata)
}

func testUnimplementedRPCs(ctx context.Context, t *testing.T, helper serverccl.TenantTestHelper) {
Expand Down
6 changes: 5 additions & 1 deletion pkg/server/status/recorder.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,17 @@ func (mr *MetricsRecorder) GetMetricsMetadata() map[string]metric.Metadata {
// Get a random storeID.
var sID roachpb.StoreID

storeFound := false
for storeID := range mr.mu.storeRegistries {
sID = storeID
storeFound = true
break
}

// Get metric metadata from that store because all stores have the same metadata.
mr.mu.storeRegistries[sID].WriteMetricsMetadata(metrics)
if storeFound {
mr.mu.storeRegistries[sID].WriteMetricsMetadata(metrics)
}

return metrics
}
Expand Down

0 comments on commit 9c114e2

Please sign in to comment.