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.

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

[Cherry-pick]Add process_start_time_seconds metric into csi metric lib #56

Merged
merged 1 commit into from
Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion connection/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,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 Down
10 changes: 9 additions & 1 deletion metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ func NewCSIMetricsManager(driverName string) CSIMetricsManager {
),
}

// 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.RawRegister)

cmm.SetDriverName(driverName)
cmm.registerMetrics()
return &cmm
Expand Down Expand Up @@ -171,7 +175,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
41 changes: 37 additions & 4 deletions metrics/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ import (
"k8s.io/component-base/metrics/testutil"
)

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

func TestRecordMetrics(t *testing.T) {
// Arrange
cmm := NewCSIMetricsManager(
Expand Down Expand Up @@ -62,7 +67,7 @@ func TestRecordMetrics(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 @@ -101,7 +106,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 @@ -139,7 +144,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 @@ -197,7 +202,35 @@ func TestStartMetricsEndPoint_Noop(t *testing.T) {
`

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 := NewCSIMetricsManager(
"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)
}