feat: deprecations metric

A metric to measure the use of deprecated features in the code base

Signed-off-by: Alan Clucas <[email protected]>

.spelling

@@ -102,6 +102,7 @@ PVCs
 Peixuan
 Ploomber
 Postgres
+PriorityClass
 RCs
 Roadmap
 RoleBinding

cmd/argo/commands/cron/create.go

@@ -90,7 +90,7 @@ func CreateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliCr
 		if err != nil {
 			return fmt.Errorf("Failed to create cron workflow: %v", err)
 		}
-		fmt.Print(getCronWorkflowGet(created))
+		fmt.Print(getCronWorkflowGet(ctx, created))
 	}
 	return nil
 }

cmd/argo/commands/cron/get.go

@@ -33,7 +33,7 @@ func NewGetCommand() *cobra.Command {
 				if err != nil {
 					return err
 				}
-				printCronWorkflow(cronWf, output)
+				printCronWorkflow(ctx, cronWf, output)
 			}
 			return nil
 		},

cmd/argo/commands/cron/list.go

@@ -1,6 +1,7 @@
 package cron
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"text/tabwriter"
@@ -50,7 +51,7 @@ func NewListCommand() *cobra.Command {
 			}
 			switch listArgs.output {
 			case "", "wide":
-				printTable(cronWfList.Items, &listArgs)
+				printTable(ctx, cronWfList.Items, &listArgs)
 			case "name":
 				for _, cronWf := range cronWfList.Items {
 					fmt.Println(cronWf.ObjectMeta.Name)
@@ -67,7 +68,7 @@ func NewListCommand() *cobra.Command {
 	return command
 }
 
-func printTable(wfList []wfv1.CronWorkflow, listArgs *listFlags) {
+func printTable(ctx context.Context, wfList []wfv1.CronWorkflow, listArgs *listFlags) {
 	w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0)
 	if listArgs.allNamespaces {
 		_, _ = fmt.Fprint(w, "NAMESPACE\t")
@@ -85,7 +86,7 @@ func printTable(wfList []wfv1.CronWorkflow, listArgs *listFlags) {
 			cleanLastScheduledTime = "N/A"
 		}
 		var cleanNextScheduledTime string
-		if next, err := GetNextRuntime(&cwf); err == nil {
+		if next, err := GetNextRuntime(ctx, &cwf); err == nil {
 			cleanNextScheduledTime = humanize.RelativeDurationShort(next, time.Now())
 		} else {
 			cleanNextScheduledTime = "N/A"

cmd/argo/commands/cron/update.go

@@ -89,7 +89,7 @@ func updateCronWorkflows(ctx context.Context, filePaths []string, cliOpts *cliUp
 		if err != nil {
 			return fmt.Errorf("Failed to update workflow template: %v", err)
 		}
-		fmt.Print(getCronWorkflowGet(updated))
+		fmt.Print(getCronWorkflowGet(ctx, updated))
 	}
 	return nil
 }

cmd/argo/commands/cron/util.go

@@ -1,6 +1,7 @@
 package cron
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"log"
@@ -22,10 +23,10 @@ import (
 
 // GetNextRuntime returns the next time the workflow should run in local time. It assumes the workflow-controller is in
 // UTC, but nevertheless returns the time in the local timezone.
-func GetNextRuntime(cwf *v1alpha1.CronWorkflow) (time.Time, error) {
+func GetNextRuntime(ctx context.Context, cwf *v1alpha1.CronWorkflow) (time.Time, error) {
 	var nextRunTime time.Time
 	now := time.Now().UTC()
-	for _, schedule := range cwf.Spec.GetSchedulesWithTimezone() {
+	for _, schedule := range cwf.Spec.GetSchedulesWithTimezone(ctx) {
 		cronSchedule, err := cron.ParseStandard(schedule)
 		if err != nil {
 			return time.Time{}, err
@@ -77,7 +78,7 @@ func unmarshalCronWorkflows(wfBytes []byte, strict bool) []wfv1.CronWorkflow {
 	return nil
 }
 
-func printCronWorkflow(wf *wfv1.CronWorkflow, outFmt string) {
+func printCronWorkflow(ctx context.Context, wf *wfv1.CronWorkflow, outFmt string) {
 	switch outFmt {
 	case "name":
 		fmt.Println(wf.ObjectMeta.Name)
@@ -88,13 +89,13 @@ func printCronWorkflow(wf *wfv1.CronWorkflow, outFmt string) {
 		outBytes, _ := yaml.Marshal(wf)
 		fmt.Print(string(outBytes))
 	case "wide", "":
-		fmt.Print(getCronWorkflowGet(wf))
+		fmt.Print(getCronWorkflowGet(ctx, wf))
 	default:
 		log.Fatalf("Unknown output format: %s", outFmt)
 	}
 }
 
-func getCronWorkflowGet(cwf *wfv1.CronWorkflow) string {
+func getCronWorkflowGet(ctx context.Context, cwf *wfv1.CronWorkflow) string {
 	const fmtStr = "%-30s %v\n"
 
 	out := ""
@@ -116,7 +117,7 @@ func getCronWorkflowGet(cwf *wfv1.CronWorkflow) string {
 		out += fmt.Sprintf(fmtStr, "LastScheduledTime:", humanize.Timestamp(cwf.Status.LastScheduledTime.Time))
 	}
 
-	next, err := GetNextRuntime(cwf)
+	next, err := GetNextRuntime(ctx, cwf)
 	if err == nil {
 		out += fmt.Sprintf(fmtStr, "NextScheduledTime:", humanize.Timestamp(next)+" (assumes workflow-controller is in UTC)")
 	}

cmd/argo/commands/cron/util_test.go

@@ -1,6 +1,7 @@
 package cron
 
 import (
+	"context"
 	"testing"
 	"time"
 
@@ -56,13 +57,13 @@ Conditions:
 
 func TestPrintCronWorkflow(t *testing.T) {
 	var cronWf = v1alpha1.MustUnmarshalCronWorkflow(invalidCwf)
-	out := getCronWorkflowGet(cronWf)
+	out := getCronWorkflowGet(context.Background(), cronWf)
 	assert.Contains(t, out, expectedOut)
 }
 
 func TestNextRuntime(t *testing.T) {
 	var cronWf = v1alpha1.MustUnmarshalCronWorkflow(invalidCwf)
-	next, err := GetNextRuntime(cronWf)
+	next, err := GetNextRuntime(context.Background(), cronWf)
 	require.NoError(t, err)
 	assert.LessOrEqual(t, next.Unix(), time.Now().Add(1*time.Minute).Unix())
 	assert.Greater(t, next.Unix(), time.Now().Unix())
@@ -94,7 +95,7 @@ spec:
 
 func TestNextRuntimeWithMultipleSchedules(t *testing.T) {
 	var cronWf = v1alpha1.MustUnmarshalCronWorkflow(cronMultipleSchedules)
-	next, err := GetNextRuntime(cronWf)
+	next, err := GetNextRuntime(context.Background(), cronWf)
 	require.NoError(t, err)
 	assert.LessOrEqual(t, next.Unix(), time.Now().Add(1*time.Minute).Unix())
 	assert.Greater(t, next.Unix(), time.Now().Unix())

config/config.go

@@ -6,6 +6,8 @@ import (
 	"net/url"
 	"time"
 
+	metricsdk "go.opentelemetry.io/otel/sdk/metric"
+	"go.opentelemetry.io/otel/sdk/metric/metricdata"
 	apiv1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -288,13 +290,28 @@ type MetricsConfig struct {
 	Temporality MetricsTemporality `json:"temporality,omitempty"`
 }
 
-func (mc MetricsConfig) GetSecure(defaultValue bool) bool {
+func (mc *MetricsConfig) GetSecure(defaultValue bool) bool {
 	if mc.Secure != nil {
 		return *mc.Secure
 	}
 	return defaultValue
 }
 
+func (mc *MetricsConfig) GetTemporality() metricsdk.TemporalitySelector {
+	switch mc.Temporality {
+	case MetricsTemporalityCumulative:
+		return func(metricsdk.InstrumentKind) metricdata.Temporality {
+			return metricdata.CumulativeTemporality
+		}
+	case MetricsTemporalityDelta:
+		return func(metricsdk.InstrumentKind) metricdata.Temporality {
+			return metricdata.DeltaTemporality
+		}
+	default:
+		return metricsdk.DefaultTemporalitySelector
+	}
+}
+
 type WorkflowRestrictions struct {
 	TemplateReferencing TemplateReferencing `json:"templateReferencing,omitempty"`
 }

docs/deprecations.md

@@ -0,0 +1,75 @@
+# Deprecations
+
+Sometimes a feature of Argo Workflows is deprecated.
+You should stop using a deprecated feature as it may be removed in a future minor or major release of Argo Workflows.
+
+To determine if you are using a deprecated feature the [`deprecated_feature`](metrics.md#deprecated_feature) metric can help.
+This metric will go up for each use of a deprecated feature by the workflow controller.
+This means it may go up once or many times for a single event.
+If the number is going up the feature is still in use by your system.
+If the metric is not present or no longer increasing are no longer using the monitored deprecated features.
+
+## `cronworkflow schedule`
+
+The spec field `schedule` which takes a single value is replaced by `schedules` which takes a list.
+To update this replace the `schedule` with `schedules` as in the following example
+
+```yaml
+spec:
+  schedule: "30 1 * * *"
+```
+
+is replaced with
+
+```yaml
+spec:
+  schedules:
+    - "30 1 * * *"
+```
+
+## `synchronization mutex`
+
+The synchronization field `mutex` which takes a single value is replaced by `mutexes` which takes a list.
+To update this replace `mutex` with `mutexes` as in the following example
+
+```yaml
+synchronization:
+  mutex:
+    name: foobar
+```
+
+is replaced with
+
+```yaml
+synchronization:
+  mutexes:
+    - name: foobar
+```
+
+## `synchronization semaphore`
+
+The synchronization field `semaphore` which takes a single value is replaced by `semaphores` which takes a list.
+To update this replace `semaphore` with `semaphores` as in the following example
+
+```yaml
+synchronization:
+  semaphore:
+    configMapKeyRef:
+      name: my-config
+      key: workflow
+```
+
+is replaced with
+
+```yaml
+synchronization:
+  semaphores:
+    - configMapKeyRef:
+        name: my-config
+        key: workflow
+```
+
+## `workflow podpriority`
+
+The Workflow spec field `podPriority` which takes a numeric value is deprecated and `podPriorityClassName` should be used instead.
+To update this you will need a [PriorityClass](https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/#priorityclass) in your cluster and refer to that using `podPriorityClassName`.

docs/metrics.md

@@ -265,6 +265,23 @@ Suppressed runs due to `concurrencyPolicy: Forbid` will not be counted.
 | `name`     | ⚠️ The name of the CronWorkflow |
 | `namespace` | The namespace of the CronWorkflow |
 
+#### `deprecated_feature`
+
+A counter which goes up when a feature which is [deprecated](deprecations.md) is used.
+🚨 This counter may go up much more than once for a single use of the feature.
+
+| attribute   | explanation                                 |
+|-------------|---------------------------------------------|
+| `feature`   | The name of the feature used                |
+| `namespace` | The namespace of the item using the feature |
+
+`feature` will be one of:
+
+- [`cronworkflow schedule`](deprecations.md#cronworkflow_schedule)
+- [`synchronization mutex`](deprecations.md#synchronization_mutex)
+- [`synchronization semaphore`](deprecations.md#synchronization_semaphore)
+- [`workflow podpriority`](deprecations.md#workflow_podpriority)
+
 #### `gauge`
 
 A gauge of the number of workflows currently in the cluster in each phase. The `Running` count does not mean that a workflows pods are running, just that the controller has scheduled them. A workflow can be stuck in `Running` with pending pods for a long time.

docs/upgrading.md

@@ -31,6 +31,7 @@ The following are new metrics:
 
 * `cronworkflows_concurrencypolicy_triggered`
 * `cronworkflows_triggered_total`
+* `deprecated_feature`
 * `is_leader`
 * `k8s_request_duration`
 * `pod_pending_count`

mkdocs.yml

@@ -248,6 +248,7 @@ nav:
           - offloading-large-workflows.md
           - workflow-archive.md
           - metrics.md
+          - deprecations.md
           - workflow-executors.md
           - workflow-restrictions.md
           - sidecar-injection.md

pkg/apiclient/offline-cron-workflow-service-client.go

@@ -19,7 +19,7 @@ type OfflineCronWorkflowServiceClient struct {
 var _ cronworkflow.CronWorkflowServiceClient = &OfflineCronWorkflowServiceClient{}
 
 func (o OfflineCronWorkflowServiceClient) LintCronWorkflow(ctx context.Context, req *cronworkflow.LintCronWorkflowRequest, _ ...grpc.CallOption) (*v1alpha1.CronWorkflow, error) {
-	err := validate.ValidateCronWorkflow(o.namespacedWorkflowTemplateGetterMap.GetNamespaceGetter(req.Namespace), o.clusterWorkflowTemplateGetter, req.CronWorkflow)
+	err := validate.ValidateCronWorkflow(ctx, o.namespacedWorkflowTemplateGetterMap.GetNamespaceGetter(req.Namespace), o.clusterWorkflowTemplateGetter, req.CronWorkflow)
 	if err != nil {
 		return nil, err
 	}

pkg/apis/workflow/v1alpha1/cron_workflow_types.go

@@ -1,13 +1,15 @@
 package v1alpha1
 
 import (
+	"context"
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/argoproj/argo-workflows/v3/pkg/apis/workflow"
+	"github.com/argoproj/argo-workflows/v3/util/deprecation"
 )
 
 // CronWorkflow is the definition of a scheduled workflow resource
@@ -169,24 +171,25 @@ func (c *CronWorkflowSpec) getScheduleString(withTimezone bool) string {
 
 // GetSchedulesWithTimezone returns all schedules configured for the CronWorkflow with a timezone. It handles
 // both Spec.Schedules and Spec.Schedule for backwards compatibility
-func (c *CronWorkflowSpec) GetSchedulesWithTimezone() []string {
-	return c.getSchedules(true)
+func (c *CronWorkflowSpec) GetSchedulesWithTimezone(ctx context.Context) []string {
+	return c.getSchedules(ctx, true)
 }
 
 // GetSchedules returns all schedules configured for the CronWorkflow. It handles both Spec.Schedules
 // and Spec.Schedule for backwards compatibility
-func (c *CronWorkflowSpec) GetSchedules() []string {
-	return c.getSchedules(false)
+func (c *CronWorkflowSpec) GetSchedules(ctx context.Context) []string {
+	return c.getSchedules(ctx, false)
 }
 
-func (c *CronWorkflowSpec) getSchedules(withTimezone bool) []string {
+func (c *CronWorkflowSpec) getSchedules(ctx context.Context, withTimezone bool) []string {
 	var schedules []string
 	if c.Schedule != "" {
 		schedule := c.Schedule
 		if withTimezone {
 			schedule = c.withTimezone(c.Schedule)
 		}
 		schedules = append(schedules, schedule)
+		deprecation.Record(ctx, deprecation.Schedule)
 	} else {
 		schedules = make([]string, len(c.Schedules))
 		for i, schedule := range c.Schedules {