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

Jump to bottom

Clean up non-functional CloudEvents Metrics in Reconciler for Deprecated CloudEvents #6827

Merged
merged 1 commit into from
Jun 14, 2023
Merged

Clean up non-functional CloudEvents Metrics in Reconciler for Deprecated CloudEvents #6827

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
3 changes: 0 additions & 3 deletions pkg/reconciler/taskrun/taskrun.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,6 @@ func (c *Reconciler) durationAndCountMetrics(ctx context.Context, tr *v1beta1.Ta
if err := c.metrics.DurationAndCount(ctx, tr, beforeCondition); err != nil {
logger.Warnf("Failed to log the duration and count of taskruns : %v", err)
}
if err := c.metrics.CloudEvents(ctx, tr); err != nil {
logger.Warnf("Failed to log the number of cloud events: %v", err)
}
}
}

Expand Down
59 changes: 0 additions & 59 deletions pkg/taskrunmetrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ var (
runningTRsThrottledByQuotaCountView *view.View
runningTRsThrottledByNodeCountView *view.View
podLatencyView *view.View
cloudEventsView *view.View

trDuration = stats.Float64(
"taskrun_duration_seconds",
Expand Down Expand Up @@ -90,10 +89,6 @@ var (
podLatency = stats.Float64("taskruns_pod_latency",
"scheduling latency for the taskruns pods",
stats.UnitMilliseconds)

cloudEvents = stats.Int64("cloudevent_count",
"number of cloud events sent including retries",
stats.UnitDimensionless)
)

// Recorder is used to actually record TaskRun metrics
Expand Down Expand Up @@ -230,12 +225,6 @@ func viewRegister(cfg *config.Metrics) error {
Aggregation: view.LastValue(),
TagKeys: append([]tag.Key{namespaceTag, podTag}, trunTag...),
}
cloudEventsView = &view.View{
Description: cloudEvents.Description(),
Measure: cloudEvents,
Aggregation: view.Sum(),
TagKeys: append([]tag.Key{statusTag, namespaceTag}, append(trunTag, prunTag...)...),
}
return view.Register(
trDurationView,
prTRDurationView,
Expand All @@ -244,7 +233,6 @@ func viewRegister(cfg *config.Metrics) error {
runningTRsThrottledByQuotaCountView,
runningTRsThrottledByNodeCountView,
podLatencyView,
cloudEventsView,
)
}

Expand All @@ -257,7 +245,6 @@ func viewUnregister() {
runningTRsThrottledByQuotaCountView,
runningTRsThrottledByNodeCountView,
podLatencyView,
cloudEventsView,
)
}

Expand Down Expand Up @@ -456,42 +443,6 @@ func (r *Recorder) RecordPodLatency(ctx context.Context, pod *corev1.Pod, tr *v1
return nil
}

// CloudEvents logs the number of cloud events sent for TaskRun
// returns an error if it fails to log the metrics
func (r *Recorder) CloudEvents(ctx context.Context, tr *v1beta1.TaskRun) error {
r.mutex.Lock()
defer r.mutex.Unlock()

if !r.initialized {
return fmt.Errorf("ignoring the metrics recording for %s , failed to initialize the metrics recorder", tr.Name)
}

taskName := anonymous
if tr.Spec.TaskRef != nil {
taskName = tr.Spec.TaskRef.Name
}

status := "success"
if cond := tr.Status.GetCondition(apis.ConditionSucceeded); cond.Status == corev1.ConditionFalse {
status = "failed"
}

tags := []tag.Mutator{tag.Insert(namespaceTag, tr.Namespace), tag.Insert(statusTag, status)}
if ok, pipeline, pipelinerun := IsPartOfPipeline(tr); ok {
tags = append(tags, r.insertPipelineTag(pipeline, pipelinerun)...)
}
tags = append(tags, r.insertTaskTag(taskName, tr.Name)...)

ctx, err := tag.New(ctx, tags...)
if err != nil {
return err
}

metrics.Record(ctx, cloudEvents.M(sentCloudEvents(tr)))

return nil
}

// IsPartOfPipeline return true if TaskRun is a part of a Pipeline.
// It also return the name of Pipeline and PipelineRun
func IsPartOfPipeline(tr *v1beta1.TaskRun) (bool, string, string) {
Expand All @@ -505,16 +456,6 @@ func IsPartOfPipeline(tr *v1beta1.TaskRun) (bool, string, string) {
return false, "", ""
}

func sentCloudEvents(tr *v1beta1.TaskRun) int64 {
var sent int64
for _, event := range tr.Status.CloudEvents {
if event.Status.Condition != v1beta1.CloudEventConditionUnknown {
sent += 1 + int64(event.Status.RetryCount)
}
}
return sent
}

func getScheduledTime(pod *corev1.Pod) metav1.Time {
for _, c := range pod.Status.Conditions {
if c.Type == corev1.PodScheduled {
Expand Down
157 changes: 1 addition & 156 deletions pkg/taskrunmetrics/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ func TestUninitializedMetrics(t *testing.T) {
if err := metrics.RecordPodLatency(ctx, nil, nil); err == nil {
t.Error("Pod Latency recording expected to return error but got nil")
}
if err := metrics.CloudEvents(ctx, &v1beta1.TaskRun{}); err == nil {
t.Error("Cloud Events recording expected to return error but got nil")
}
}

func TestMetricsOnStore(t *testing.T) {
Expand Down Expand Up @@ -612,160 +609,8 @@ func TestTaskRunIsOfPipelinerun(t *testing.T) {
}
}

func TestRecordCloudEvents(t *testing.T) {
for _, c := range []struct {
name string
taskRun *v1beta1.TaskRun
expectedTags map[string]string
expectedCount float64
}{{
name: "for succeeded task",
taskRun: &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "taskrun-1",
Namespace: "ns",
Labels: map[string]string{
pipeline.PipelineLabelKey: "pipeline-1",
pipeline.PipelineRunLabelKey: "pipelinerun-1",
},
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Name: "task-1",
},
},
Status: v1beta1.TaskRunStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionTrue,
}},
},
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
StartTime: &metav1.Time{Time: time.Now()},
CompletionTime: &metav1.Time{Time: time.Now().Add(1 * time.Minute)},
CloudEvents: []v1beta1.CloudEventDelivery{{
Target: "http://event_target",
Status: v1beta1.CloudEventDeliveryState{
Condition: v1beta1.CloudEventConditionSent,
RetryCount: 1,
},
}},
},
},
},
expectedTags: map[string]string{
"pipeline": "pipeline-1",
"pipelinerun": "pipelinerun-1",
"task": "task-1",
"taskrun": "taskrun-1",
"namespace": "ns",
"status": "success",
},
expectedCount: 2,
}, {
name: "for failed task",
taskRun: &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "taskrun-1",
Namespace: "ns",
Labels: map[string]string{
pipeline.PipelineLabelKey: "pipeline-1",
pipeline.PipelineRunLabelKey: "pipelinerun-1",
},
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Name: "task-1",
},
},
Status: v1beta1.TaskRunStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
}},
},
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
StartTime: &metav1.Time{Time: time.Now()},
CompletionTime: &metav1.Time{Time: time.Now().Add(1 * time.Minute)},
CloudEvents: []v1beta1.CloudEventDelivery{{
Target: "http://event_target",
Status: v1beta1.CloudEventDeliveryState{
Condition: v1beta1.CloudEventConditionFailed,
RetryCount: 2,
},
}},
},
},
},
expectedTags: map[string]string{
"pipeline": "pipeline-1",
"pipelinerun": "pipelinerun-1",
"task": "task-1",
"taskrun": "taskrun-1",
"namespace": "ns",
"status": "failed",
},
expectedCount: 3,
}, {
name: "for task not part of pipeline",
taskRun: &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "taskrun-1",
Namespace: "ns",
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Name: "task-1",
},
},
Status: v1beta1.TaskRunStatus{
Status: duckv1.Status{
Conditions: duckv1.Conditions{apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionTrue,
}},
},
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
StartTime: &metav1.Time{Time: time.Now()},
CompletionTime: &metav1.Time{Time: time.Now().Add(1 * time.Minute)},
CloudEvents: []v1beta1.CloudEventDelivery{{
Target: "http://event_target",
Status: v1beta1.CloudEventDeliveryState{
Condition: v1beta1.CloudEventConditionSent,
RetryCount: 1,
},
}},
},
},
},
expectedTags: map[string]string{
"task": "task-1",
"taskrun": "taskrun-1",
"namespace": "ns",
"status": "success",
},
expectedCount: 2,
}} {
t.Run(c.name, func(t *testing.T) {
unregisterMetrics()
ctx := getConfigContext()
metrics, err := NewRecorder(ctx)
if err != nil {
t.Fatalf("NewRecorder: %v", err)
}

if err := metrics.CloudEvents(ctx, c.taskRun); err != nil {
t.Fatalf("CloudEvents: %v", err)
}
metricstest.CheckSumData(t, "cloudevent_count", c.expectedTags, c.expectedCount)
})
}
}

func unregisterMetrics() {
metricstest.Unregister("taskrun_duration_seconds", "pipelinerun_taskrun_duration_seconds", "taskrun_count", "running_taskruns_count", "running_taskruns_throttled_by_quota_count", "running_taskruns_throttled_by_node_count", "taskruns_pod_latency", "cloudevent_count")
metricstest.Unregister("taskrun_duration_seconds", "pipelinerun_taskrun_duration_seconds", "taskrun_count", "running_taskruns_count", "running_taskruns_throttled_by_quota_count", "running_taskruns_throttled_by_node_count", "taskruns_pod_latency")

// Allow the recorder singleton to be recreated.
once = sync.Once{}
Expand Down