Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add process_start_time_seconds metric into csi metric lib #54

Merged
merged 1 commit into from
Sep 30, 2020
Merged

Add process_start_time_seconds metric into csi metric lib #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions connection/connection_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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 {
msau42 marked this conversation as resolved.
Show resolved Hide resolved
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)
}