Skip to content

Commit

Permalink
Merge pull request #54 from Jiawei0227/processstart
Browse files Browse the repository at this point in the history 
Add process_start_time_seconds metric into csi metric lib
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Sep 30, 2020
2 parents fb645ec + 8632ac1 commit 1dba3d5
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 14 deletions.
4 changes: 2 additions & 2 deletions connection/connection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ func TestConnectMetrics(t *testing.T) {
`

if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), "csi_sidecar_operations_seconds"); err != nil {
// Ignore mismatches on csi_sidecar_operations_seconds_sum metric because execution time will vary from test to test.
err = verifyMetricsError(t, err, "csi_sidecar_operations_seconds_sum")
if err != nil {
Expand All @@ -393,7 +393,7 @@ func TestConnectMetrics(t *testing.T) {

expectedMetrics = strings.Replace(expectedMetrics, "csi_sidecar", metrics.SubsystemPlugin, -1)
if err := testutil.GatherAndCompare(
cmmServer.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmmServer.GetRegistry(), strings.NewReader(expectedMetrics), "csi_plugin_operations_seconds"); err != nil {
// Ignore mismatches on csi_sidecar_operations_seconds_sum metric because execution time will vary from test to test.
err = verifyMetricsError(t, err, metrics.SubsystemPlugin+"_operations_seconds_sum")
if err != nil {
Expand Down
11 changes: 10 additions & 1 deletion metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,11 @@ func NewCSIMetricsManagerWithOptions(driverName string, options ...MetricsManage
subsystem: SubsystemSidecar,
stabilityLevel: metrics.ALPHA,
}

// https://github.com/open-telemetry/opentelemetry-collector/issues/969
// Add process_start_time_seconds into the metric to let the start time be parsed correctly
metrics.RegisterProcessStartTime(cmm.registry.Register)

for _, option := range options {
option(&cmm)
}
Expand Down Expand Up @@ -357,7 +362,11 @@ func VerifyMetricsMatch(expectedMetrics, actualMetrics string, metricToIgnore st
wantScanner.Scan()
wantLine := strings.TrimSpace(wantScanner.Text())
gotLine := strings.TrimSpace(gotScanner.Text())
if wantLine != gotLine && (metricToIgnore == "" || !strings.HasPrefix(gotLine, metricToIgnore)) {
if wantLine != gotLine &&
(metricToIgnore == "" || !strings.HasPrefix(gotLine, metricToIgnore)) &&
// We should ignore the comments from metricToIgnore, otherwise the verification will
// fail because of the comments.
!strings.HasPrefix(gotLine, "#") {
return fmt.Errorf("\r\nMetric Want: %q\r\nMetric Got: %q\r\n", wantLine, gotLine)
}
}
Expand Down
57 changes: 46 additions & 11 deletions metrics/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ import (
"k8s.io/component-base/metrics/testutil"
)

const (
SidecarOperationMetric = "csi_sidecar_operations_seconds"
ProcessStartTimeMetric = "process_start_time_seconds"
)

func TestRecordMetrics(t *testing.T) {
testcases := map[string]struct {
subsystem string
Expand Down Expand Up @@ -102,15 +107,17 @@ func testRecordMetrics(t *testing.T, subsystem string, stabilityLevel metrics.St
csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="OK",method_name="/csi.v1.Controller/ControllerGetCapabilities"} 20
csi_sidecar_operations_seconds_count{driver_name="fake.csi.driver.io",grpc_status_code="OK",method_name="/csi.v1.Controller/ControllerGetCapabilities"} 1
`
metricName := SidecarOperationMetric
if subsystem != "" {
expectedMetrics = strings.Replace(expectedMetrics, "csi_sidecar", subsystem, -1)
metricName = strings.Replace(metricName, "csi_sidecar", subsystem, -1)
}
if stabilityLevel != "" {
expectedMetrics = strings.Replace(expectedMetrics, "ALPHA", string(stabilityLevel), -1)
}

if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), metricName); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -151,7 +158,7 @@ func TestFixedLabels(t *testing.T) {
`

if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), SidecarOperationMetric); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -200,7 +207,7 @@ func TestVaryingLabels(t *testing.T) {
`

if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), SidecarOperationMetric); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -273,7 +280,7 @@ func TestTwoVaryingLabels(t *testing.T) {
`

if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), SidecarOperationMetric); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -318,7 +325,7 @@ func TestVaryingLabelsBackfill(t *testing.T) {
`

if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), SidecarOperationMetric); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -392,7 +399,7 @@ func TestCombinedLabels(t *testing.T) {
`

if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), SidecarOperationMetric); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -431,7 +438,7 @@ func TestRecordMetrics_NoDriverName(t *testing.T) {
`

if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), SidecarOperationMetric); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -469,7 +476,7 @@ func TestRecordMetrics_Negative(t *testing.T) {
csi_sidecar_operations_seconds_count{driver_name="fake.csi.driver.io",grpc_status_code="InvalidArgument",method_name="myOperation"} 1
`
if err := testutil.GatherAndCompare(
cmm.GetRegistry(), strings.NewReader(expectedMetrics)); err != nil {
cmm.GetRegistry(), strings.NewReader(expectedMetrics), SidecarOperationMetric); err != nil {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -505,6 +512,7 @@ func TestStartMetricsEndPoint_Noop(t *testing.T) {
if err != nil {
t.Fatalf("Failed to parse metrics response. Response was: %+v Error: %v", resp, err)
}
actualMetrics := string(contentBytes)

expectedMetrics := `# HELP csi_sidecar_operations_seconds [ALPHA] Container Storage Interface operation duration with gRPC error code status total
# TYPE csi_sidecar_operations_seconds histogram
Expand All @@ -524,10 +532,37 @@ func TestStartMetricsEndPoint_Noop(t *testing.T) {
csi_sidecar_operations_seconds_bucket{driver_name="fake.csi.driver.io",grpc_status_code="OK",method_name="/csi.v1.Controller/ControllerGetCapabilities",le="+Inf"} 1
csi_sidecar_operations_seconds_sum{driver_name="fake.csi.driver.io",grpc_status_code="OK",method_name="/csi.v1.Controller/ControllerGetCapabilities"} 20
csi_sidecar_operations_seconds_count{driver_name="fake.csi.driver.io",grpc_status_code="OK",method_name="/csi.v1.Controller/ControllerGetCapabilities"} 1
`
`

actualMetrics := string(contentBytes)
if err := VerifyMetricsMatch(expectedMetrics, actualMetrics, ""); err != nil {
if err := VerifyMetricsMatch(expectedMetrics, actualMetrics, ProcessStartTimeMetric); err != nil {
t.Fatalf("Metrics returned by end point do not match expectation: %v", err)
}
}

func TestProcessStartTimeMetricExist(t *testing.T) {
// Arrange
cmm := NewCSIMetricsManagerForSidecar(
"fake.csi.driver.io" /* driverName */)
operationDuration, _ := time.ParseDuration("20s")

// Act
cmm.RecordMetrics(
"/csi.v1.Controller/ControllerGetCapabilities", /* operationName */
nil, /* operationErr */
operationDuration /* operationDuration */)

// Assert
metricsFamilies, err := cmm.GetRegistry().Gather()
if err != nil {
t.Fatalf("Error fetching metrics: %v", err)
}

// check process_start_time_seconds exist
for _, metricsFamily := range metricsFamilies {
if metricsFamily.GetName() == ProcessStartTimeMetric {
return
}
}

t.Fatalf("Metrics does not contain %v. Scraped content: %v", ProcessStartTimeMetric, metricsFamilies)
}

0 comments on commit 1dba3d5

Please sign in to comment.