Skip to content

Commit 5083a1e

Browse files
Jiawei0227Jiawei0227
committed
Add Migrated metrics to the connection record metrics clientside
1 parent ffd4067 commit 5083a1e

File tree

4 files changed

+142
-97
lines changed

4 files changed

+142
-97
lines changed

‎connection/connection.go‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,13 @@ type ExtendedCSIMetricsManager struct {
191191
metrics.CSIMetricsManager
192192
}
193193

194+
type AdditionalInfo struct {
195+
Migrated string
196+
}
197+
type AdditionalInfoKeyType struct{}
198+
199+
var AdditionalInfoKey AdditionalInfoKeyType
200+
194201
// RecordMetricsClientInterceptor is a gPRC unary interceptor for recording metrics for CSI operations
195202
// in a gRPC client.
196203
func (cmm ExtendedCSIMetricsManager) RecordMetricsClientInterceptor(
@@ -203,11 +210,33 @@ func (cmm ExtendedCSIMetricsManager) RecordMetricsClientInterceptor(
203210
start := time.Now()
204211
err := invoker(ctx, method, req, reply, cc, opts...)
205212
duration := time.Since(start)
206-
cmm.RecordMetrics(
207-
method, /* operationName */
208-
err, /* operationErr */
209-
duration, /* operationDuration */
210-
)
213+
214+
if cmm.HaveAdditionalLabel(metrics.LabelMigrated) {
215+
// record migration status
216+
additionalInfo := ctx.Value(AdditionalInfoKey)
217+
migrated := "false"
218+
if additionalInfo != nil {
219+
migrated = additionalInfo.(AdditionalInfo).Migrated
220+
}
221+
cmmv, metricsErr := cmm.WithLabelValues(map[string]string{metrics.LabelMigrated: migrated})
222+
if metricsErr != nil {
223+
klog.Errorf("Failed to record migrated status, error: %v", metricsErr)
224+
} else {
225+
cmmv.RecordMetrics(
226+
method, /* operationName */
227+
err, /* operationErr */
228+
duration, /* operationDuration */
229+
)
230+
}
231+
} else {
232+
// Record the default metric
233+
cmm.RecordMetrics(
234+
method, /* operationName */
235+
err, /* operationErr */
236+
duration, /* operationDuration */
237+
)
238+
}
239+
211240
return err
212241
}
213242

‎connection/connection_test.go‎

Lines changed: 99 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -340,64 +340,109 @@ func TestExplicitReconnect(t *testing.T) {
340340
}
341341

342342
func TestConnectMetrics(t *testing.T) {
343-
tmp := tmpDir(t)
344-
defer os.RemoveAll(tmp)
345-
cmmServer := metrics.NewCSIMetricsManagerForPlugin("fake.csi.driver.io")
346-
// We have to have a real implementation of the gRPC call, otherwise the metrics
347-
// interceptor is not called. The CSI identity service is used because it's simple.
348-
addr, stopServer := startServer(t, tmp, &identityServer{}, nil, cmmServer)
349-
defer stopServer()
350-
351-
cmm := metrics.NewCSIMetricsManager("fake.csi.driver.io")
352-
conn, err := Connect(addr, cmm)
353-
if assert.NoError(t, err, "connect via absolute path") &&
354-
assert.NotNil(t, conn, "got a connection") {
355-
defer conn.Close()
356-
assert.Equal(t, connectivity.Ready, conn.GetState(), "connection ready")
357-
358-
identityClient := csi.NewIdentityClient(conn)
359-
if _, err := identityClient.GetPluginInfo(context.Background(), &csi.GetPluginInfoRequest{}); assert.Error(t, err) {
360-
errStatus, _ := status.FromError(err)
361-
assert.Equal(t, codes.Unimplemented, errStatus.Code(), "not implemented")
362-
}
343+
testCases := []struct {
344+
name string
345+
expectedMetrics string
346+
ctx context.Context
347+
checkServer bool
348+
cmm metrics.CSIMetricsManager
349+
}{
350+
{
351+
name: "Regular connection test",
352+
expectedMetrics: `# HELP csi_sidecar_operations_seconds [ALPHA] Container Storage Interface operation duration with gRPC error code status total
353+
# TYPE csi_sidecar_operations_seconds histogram
354+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="0.1"} 1
355+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="0.25"} 1
356+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="0.5"} 1
357+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="1"} 1
358+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="2.5"} 1
359+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="5"} 1
360+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="10"} 1
361+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="15"} 1
362+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="25"} 1
363+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="50"} 1
364+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="120"} 1
365+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="300"} 1
366+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="600"} 1
367+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="+Inf"} 1
368+
csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 0
369+
csi_sidecar_operations_seconds_count{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 1
370+
`,
371+
ctx: context.Background(),
372+
cmm: metrics.NewCSIMetricsManager("fake.csi.driver.io"),
373+
checkServer: true,
374+
},
375+
{
376+
name: "connection test with migrated metrics",
377+
expectedMetrics: `# HELP csi_sidecar_operations_seconds [ALPHA] Container Storage Interface operation duration with gRPC error code status total
378+
# TYPE csi_sidecar_operations_seconds histogram
379+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="0.1"} 1
380+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="0.25"} 1
381+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="0.5"} 1
382+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="1"} 1
383+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="2.5"} 1
384+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="5"} 1
385+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="10"} 1
386+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="15"} 1
387+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="25"} 1
388+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="50"} 1
389+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="120"} 1
390+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="300"} 1
391+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="600"} 1
392+
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true",le="+Inf"} 1
393+
csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true"} 0
394+
csi_sidecar_operations_seconds_count{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",migrated="true"} 1
395+
`,
396+
ctx: context.WithValue(context.Background(), AdditionalInfoKey, AdditionalInfo{
397+
Migrated: "true",
398+
}),
399+
cmm: metrics.NewCSIMetricsManagerWithOptions("fake.csi.driver.io", metrics.WithLabelNames(metrics.LabelMigrated)),
400+
checkServer: false,
401+
},
363402
}
403+
for _, test := range testCases {
404+
t.Logf("Running testcase %v", test.name)
405+
tmp := tmpDir(t)
406+
defer os.RemoveAll(tmp)
407+
cmmServer := metrics.NewCSIMetricsManagerForPlugin("fake.csi.driver.io")
408+
// We have to have a real implementation of the gRPC call, otherwise the metrics
409+
// interceptor is not called. The CSI identity service is used because it's simple.
410+
addr, stopServer := startServer(t, tmp, &identityServer{}, nil, cmmServer)
411+
defer stopServer()
412+
413+
cmm := test.cmm
414+
conn, err := Connect(addr, cmm)
415+
if assert.NoError(t, err, "connect via absolute path") &&
416+
assert.NotNil(t, conn, "got a connection") {
417+
defer conn.Close()
418+
assert.Equal(t, connectivity.Ready, conn.GetState(), "connection ready")
419+
420+
identityClient := csi.NewIdentityClient(conn)
421+
if _, err := identityClient.GetPluginInfo(test.ctx, &csi.GetPluginInfoRequest{}); assert.Error(t, err) {
422+
errStatus, _ := status.FromError(err)
423+
assert.Equal(t, codes.Unimplemented, errStatus.Code(), "not implemented")
424+
}
425+
}
364426

365-
expectedMetrics := `# HELP csi_sidecar_operations_seconds [ALPHA] Container Storage Interface operation duration with gRPC error code status total
366-
# TYPE csi_sidecar_operations_seconds histogram
367-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="0.1"} 1
368-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="0.25"} 1
369-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="0.5"} 1
370-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="1"} 1
371-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="2.5"} 1
372-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="5"} 1
373-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="10"} 1
374-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="15"} 1
375-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="25"} 1
376-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="50"} 1
377-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="120"} 1
378-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="300"} 1
379-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="600"} 1
380-
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo",le="+Inf"} 1
381-
csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 0
382-
csi_sidecar_operations_seconds_count{driver_name="fake.csi.driver.io",grpc_status_code="Unimplemented",method_name="/csi.v1.Identity/GetPluginInfo"} 1
383-
`
384-
385-
if err := testutil.GatherAndCompare(
386-
cmm.GetRegistry(), strings.NewReader(expectedMetrics), "csi_sidecar_operations_seconds"); err != nil {
387-
// Ignore mismatches on csi_sidecar_operations_seconds_sum metric because execution time will vary from test to test.
388-
err = verifyMetricsError(t, err, "csi_sidecar_operations_seconds_sum")
389-
if err != nil {
390-
t.Errorf("Expected client metrics not found -- %v", err)
427+
if err := testutil.GatherAndCompare(
428+
cmm.GetRegistry(), strings.NewReader(test.expectedMetrics), "csi_sidecar_operations_seconds"); err != nil {
429+
// Ignore mismatches on csi_sidecar_operations_seconds_sum metric because execution time will vary from test to test.
430+
err = verifyMetricsError(t, err, "csi_sidecar_operations_seconds_sum")
431+
if err != nil {
432+
t.Errorf("Expected client metrics not found -- %v", err)
433+
}
391434
}
392-
}
393435

394-
expectedMetrics = strings.Replace(expectedMetrics, "csi_sidecar", metrics.SubsystemPlugin, -1)
395-
if err := testutil.GatherAndCompare(
396-
cmmServer.GetRegistry(), strings.NewReader(expectedMetrics), "csi_plugin_operations_seconds"); err != nil {
397-
// Ignore mismatches on csi_sidecar_operations_seconds_sum metric because execution time will vary from test to test.
398-
err = verifyMetricsError(t, err, metrics.SubsystemPlugin+"_operations_seconds_sum")
399-
if err != nil {
400-
t.Errorf("Expected server metrics not found -- %v", err)
436+
if test.checkServer {
437+
expectedMetrics := strings.Replace(test.expectedMetrics, "csi_sidecar", metrics.SubsystemPlugin, -1)
438+
if err := testutil.GatherAndCompare(
439+
cmmServer.GetRegistry(), strings.NewReader(expectedMetrics), "csi_plugin_operations_seconds"); err != nil {
440+
// Ignore mismatches on csi_sidecar_operations_seconds_sum metric because execution time will vary from test to test.
441+
err = verifyMetricsError(t, err, metrics.SubsystemPlugin+"_operations_seconds_sum")
442+
if err != nil {
443+
t.Errorf("Expected server metrics not found -- %v", err)
444+
}
445+
}
401446
}
402447
}
403448
}

0 commit comments

Comments
 (0)