Merge pull request #493 from wonderflow/metrics

fix metric capability and add tutroial

api/v1alpha1/metricstrait_types.go

@@ -39,12 +39,11 @@ type ScapeServiceEndPoint struct {
 	// The default and only supported format is "prometheus" for now
 	Format string `json:"format,omitempty"`
 	// Number or name of the port to access on the pods targeted by the service.
-	// When this field has value implies that we need to create a service for the workload
-	// Mutually exclusive with port.
+	// The default is discovered automatically from podTemplate, metricTrait will create a service for the workload
 	TargetPort intstr.IntOrString `json:"port,omitempty"`
 	// Route service traffic to pods with label keys and values matching this
-	// The default is the labels in the workload
-	// Mutually exclusive with port.
+	// The default is discovered automatically from podTemplate.
+	// If no podTemplate, use the labels specified here, or use the labels of the workload
 	TargetSelector map[string]string `json:"selector,omitempty"`
 	// HTTP path to scrape for metrics.
 	// default is /metrics
@@ -63,8 +62,13 @@ type ScapeServiceEndPoint struct {
 type MetricsTraitStatus struct {
 	runtimev1alpha1.ConditionedStatus `json:",inline"`
 
-	// ServiceMonitorNames managed by this trait
-	ServiceMonitorNames []string `json:"serviceMonitorName,omitempty"`
+	// ServiceMonitorName managed by this trait
+	ServiceMonitorName string `json:"serviceMonitorName,omitempty"`
+
+	// Port is the real port monitoring
+	Port intstr.IntOrString `json:"port,omitempty"`
+	// SelectorLabels is the real labels selected
+	SelectorLabels map[string]string `json:"selectorLabels,omitempty"`
 }
 
 // +kubebuilder:object:root=true

api/v1alpha1/zz_generated.deepcopy.go

@@ -250,10 +250,13 @@ func (in *MetricsTraitSpec) DeepCopy() *MetricsTraitSpec {
 func (in *MetricsTraitStatus) DeepCopyInto(out *MetricsTraitStatus) {
 	*out = *in
 	in.ConditionedStatus.DeepCopyInto(&out.ConditionedStatus)
-	if in.ServiceMonitorNames != nil {
-		in, out := &in.ServiceMonitorNames, &out.ServiceMonitorNames
-		*out = make([]string, len(*in))
-		copy(*out, *in)
+	out.Port = in.Port
+	if in.SelectorLabels != nil {
+		in, out := &in.SelectorLabels, &out.SelectorLabels
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
 	}
 }
 

charts/vela-core/crds/standard.oam.dev_metricstraits.yaml

@@ -56,9 +56,9 @@ spec:
                     - type: integer
                     - type: string
                     description: Number or name of the port to access on the pods
-                      targeted by the service. When this field has value implies that
-                      we need to create a service for the workload Mutually exclusive
-                      with port.
+                      targeted by the service. The default is discovered automatically
+                      from podTemplate, metricTrait will create a service for the
+                      workload
                     x-kubernetes-int-or-string: true
                   scheme:
                     description: Scheme at which metrics should be scraped The default
@@ -68,8 +68,9 @@ spec:
                     additionalProperties:
                       type: string
                     description: Route service traffic to pods with label keys and
-                      values matching this The default is the labels in the workload
-                      Mutually exclusive with port.
+                      values matching this The default is discovered automatically
+                      from podTemplate. If no podTemplate, use the labels specified
+                      here, or use the labels of the workload
                     type: object
                 type: object
               workloadRef:
@@ -132,11 +133,20 @@ spec:
                   - type
                   type: object
                 type: array
-              serviceMonitorName:
-                description: ServiceMonitorNames managed by this trait
-                items:
+              port:
+                anyOf:
+                - type: integer
+                - type: string
+                description: Port is the real port monitoring
+                x-kubernetes-int-or-string: true
+              selectorLabels:
+                additionalProperties:
                   type: string
-                type: array
+                description: SelectorLabels is the real labels selected
+                type: object
+              serviceMonitorName:
+                description: ServiceMonitorName managed by this trait
+                type: string
             type: object
         required:
         - spec

charts/vela-core/templates/defwithtemplate/metrics.yaml

@@ -21,21 +21,23 @@ spec:
   extension:
     template: |-
       output: {
-        apiVersion: "standard.oam.dev/v1alpha1"
-        kind:       "MetricsTrait"
-        spec: {
-          scrapeService: parameter
-        }
+      	apiVersion: "standard.oam.dev/v1alpha1"
+      	kind:       "MetricsTrait"
+      	spec: {
+      		scrapeService: parameter
+      	}
       }
       parameter: {
-        // +usage=format of the metrics, default as prometheus
-        // +short=f
-        format:  *"prometheus" | string
-        path:    *"/metrics" | string
-        scheme:  *"http" | string
-        enabled: *true | bool
-        port:    *8080 | >=1024 & <=65535 & int
-        // +usage= the label selector for the pods, default is the workload labels
-        selector?: [string]: string
+      	// +usage=format of the metrics, default as prometheus
+      	// +short=f
+      	format: *"prometheus" | string
+      	// +usage= the metric path of the service
+      	path:    *"/metrics" | string
+      	scheme:  *"http" | string
+      	enabled: *true | bool
+      	// +usage= the port for metrics, will discovery automatically by default
+      	port: *0 | >=1024 & <=65535 & int
+      	// +usage= the label selector for the pods, will discovery automatically by default
+      	selector?: [string]: string
       }
       

docs/developers/set-metrics.md

@@ -1,3 +1,45 @@
 # Monitoring Application
 
-> TODO
+If your application has exposed metrics, you can easily setup monitoring system
+with the help of `metric` capability.
+
+Let's run [`christianhxc/gorandom:1.0`](https://github.com/christianhxc/prometheus-tutorial) as an example app.
+The app will emit random latencies as metrics.
+
+```console
+$ vela svc deploy metricapp -t webservice --image christianhxc/gorandom:1.0 --port 8080
+```
+
+Then add metric by:
+
+```console
+$ vela metric metricapp
+Adding metric for app metricapp
+⠋ Deploying ...
+✅ Application Deployed Successfully!
+  - Name: metricapp
+    Type: webservice
+    HEALTHY Ready: 1/1
+    Routes:
+      - ✅ metric: Monitoring port: 8080, path: /metrics, format: prometheus, schema: http.
+    Last Deployment:
+      Created at: 2020-11-02 14:31:56 +0800 CST
+      Updated at: 2020-11-02T14:32:00+08:00
+```
+
+The metrics trait will automatically discover port and label to monitor if no parameters specified.
+If more than one ports found, it will choose the first one by default.
+
+
+## Verify that the metrics are collected on prometheus
+
+TODO we should set up promethus Instance along with service at vela installation
+
+```
+kubectl apply -f e2e/raw-objects/samples/metrics-demo/prometheus/prometheus-oam.yaml
+```
+
+```shell script
+kubectl --namespace monitoring port-forward svc/prometheus-oam  4848
+```
+Then access the prometheus dashboard via http://localhost:4848/targets

e2e/commonContext.go

@@ -160,7 +160,7 @@ var (
 				output, err := LongTimeExec(cli, 180*time.Second)
 				gomega.Expect(err).NotTo(gomega.HaveOccurred())
 				gomega.Expect(output).To(gomega.ContainSubstring("Adding " + traitAlias + " for app"))
-				gomega.Expect(output).To(gomega.ContainSubstring("Deploying"))
+				gomega.Expect(output).To(gomega.ContainSubstring("Checking Status"))
 			})
 		})
 	}
@@ -298,7 +298,7 @@ var (
 					_, _ = c.ExpectEOF()
 				})
 				gomega.Expect(err).NotTo(gomega.HaveOccurred())
-				gomega.Expect(output).To(gomega.ContainSubstring("Deploying"))
+				gomega.Expect(output).To(gomega.ContainSubstring("Checking Status"))
 			})
 		})
 	}

e2e/raw-objects/samples/metrics-demo/README.md

@@ -31,7 +31,11 @@ Then access the prometheus dashboard via http://localhost:4848
 ```shell script
 kubectl --namespace monitoring port-forward service/kube-prometheus-stack-grafana  3000:80
 ```
-Then access the grafana dashboard via http://localhost:3000.  You shall set the data source URL as `http://prometheus-oam:4848`
+Then access the grafana dashboard via http://localhost:3000.
+
+You shall set the data source URL as `http://prometheus-oam:4848` by:
+
+
 
 ## Setup Grafana Panel and Alert
 ```shell script

e2e/raw-objects/samples/metrics-demo/dashboard/grafana-oam-datasource.yaml

@@ -1,7 +1,7 @@
 apiVersion: integreatly.org/v1alpha1
 kind: GrafanaDataSource
 metadata:
-  name: oam-grafanadatasource
+  name: monitoring
 spec:
   name: middleware.yaml
   datasources:

e2e/raw-objects/samples/metrics-demo/dashboard/workload-dashboard.yaml

@@ -2,7 +2,7 @@ apiVersion: v1
 kind: ConfigMap
 metadata:
   name: oam-monitoring-dashboard
-  namespace: oam-monitoring
+  namespace: monitoring
   labels:
     app: oam-runtime
     chart: prometheus-operator-9.3.0

hack/vela-templates/cue/metrics.cue

@@ -1,18 +1,20 @@
 output: {
-  apiVersion: "standard.oam.dev/v1alpha1"
-  kind:       "MetricsTrait"
-  spec: {
-    scrapeService: parameter
-  }
+	apiVersion: "standard.oam.dev/v1alpha1"
+	kind:       "MetricsTrait"
+	spec: {
+		scrapeService: parameter
+	}
 }
 parameter: {
-  // +usage=format of the metrics, default as prometheus
-  // +short=f
-  format:  *"prometheus" | string
-  path:    *"/metrics" | string
-  scheme:  *"http" | string
-  enabled: *true | bool
-  port:    *8080 | >=1024 & <=65535 & int
-  // +usage= the label selector for the pods, default is the workload labels
-  selector?: [string]: string
+	// +usage=format of the metrics, default as prometheus
+	// +short=f
+	format: *"prometheus" | string
+	// +usage= the metric path of the service
+	path:    *"/metrics" | string
+	scheme:  *"http" | string
+	enabled: *true | bool
+	// +usage= the port for metrics, will discovery automatically by default
+	port: *0 | >=1024 & <=65535 & int
+	// +usage= the label selector for the pods, will discovery automatically by default
+	selector?: [string]: string
 }

pkg/commands/status.go

@@ -191,22 +191,21 @@ func printComponentStatus(ctx context.Context, c client.Client, ioStreams cmduti
 	healthInfo = strings.ReplaceAll(healthInfo, "\n", "\n\t") // format healthInfo output
 	ioStreams.Infof("    %s %s\n", healthColor.Sprint(healthStatus), healthColor.Sprint(healthInfo))
 
-	ioStreams.Infof("    Last Deployment:\n")
-	ioStreams.Infof("      Created at: %v\n", appConfig.CreationTimestamp)
-	ioStreams.Infof("      Updated at: %v\n", app.UpdateTime.Format(time.RFC3339))
-
 	// workload Must found
 	ioStreams.Infof("    Routes:\n")
 	workloadStatus, _ := getWorkloadStatusFromAppConfig(appConfig, compName)
 	for _, tr := range workloadStatus.Traits {
 		traitType, traitInfo, err := traitCheckLoop(ctx, c, tr.Reference, compName, appConfig, app, 60*time.Second)
 		if err != nil {
-			ioStreams.Infof("%s status: %s", white.Sprint(traitType), traitInfo)
-			return err
+			ioStreams.Infof("      - %s%s: %s, err: %v", emojiFail, white.Sprint(traitType), traitInfo, err)
+			continue
 		}
-		ioStreams.Infof("      - %s: %s", white.Sprint(traitType), traitInfo)
+		ioStreams.Infof("      - %s%s: %s", emojiSucceed, white.Sprint(traitType), traitInfo)
 	}
 	ioStreams.Info("")
+	ioStreams.Infof("    Last Deployment:\n")
+	ioStreams.Infof("      Created at: %v\n", appConfig.CreationTimestamp)
+	ioStreams.Infof("      Updated at: %v\n", app.UpdateTime.Format(time.RFC3339))
 	return nil
 }
 
@@ -281,7 +280,7 @@ func tryGetWorkloadStatus(ctx context.Context, c client.Client, ns string, wlRef
 }
 
 func printTrackingDeployStatus(ctx context.Context, c client.Client, ioStreams cmdutil.IOStreams, compName, appName string, env *types.EnvMeta) (CompStatus, error) {
-	sDeploy := newTrackingSpinner("Deploying ...")
+	sDeploy := newTrackingSpinner("Checking Status ...")
 	sDeploy.Start()
 	defer sDeploy.Stop()
 TrackDeployLoop:

pkg/controller/common/error.go

@@ -4,4 +4,6 @@ const (
 	ErrLocatingWorkload = "failed to locate the workload"
 	ErrLocatingService  = "failed to locate any the services"
 	ErrCreatingService  = "failed to create the services"
+
+	ErrUpdateStatus = "failed to update status"
 )

pkg/controller/utils/utils.go

@@ -0,0 +1,84 @@
+package utils
+
+import (
+	"encoding/json"
+	"fmt"
+	"strconv"
+
+	"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
+	"github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2"
+	"github.com/oam-dev/kubevela/api/types"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+func GetPodSpecPath(workloadDef *v1alpha2.WorkloadDefinition) (string, bool) {
+	if workloadDef.Spec.PodSpecPath != "" {
+		return workloadDef.Spec.PodSpecPath, true
+	}
+	if workloadDef.Labels == nil {
+		return "", false
+	}
+	podSpecable, ok := workloadDef.Labels[types.LabelPodSpecable]
+	if !ok {
+		return "", false
+	}
+	ok, _ = strconv.ParseBool(podSpecable)
+	return "", ok
+}
+
+func DiscoveryFromPodSpec(w *unstructured.Unstructured, fieldPath string) ([]intstr.IntOrString, error) {
+	paved := fieldpath.Pave(w.Object)
+	obj, err := paved.GetValue(fieldPath)
+	if err != nil {
+		return nil, err
+	}
+	data, err := json.Marshal(obj)
+	if err != nil {
+		return nil, fmt.Errorf("discovery podSpec from %s in workload %v err %v", fieldPath, w.GetName(), err)
+	}
+	var spec v1.PodSpec
+	err = json.Unmarshal(data, &spec)
+	if err != nil {
+		return nil, fmt.Errorf("discovery podSpec from %s in workload %v err %v", fieldPath, w.GetName(), err)
+	}
+	ports := getContainerPorts(spec.Containers)
+	if len(ports) == 0 {
+		return nil, fmt.Errorf("no port found in podSpec %v", w.GetName())
+	}
+	return ports, nil
+}
+
+// DiscoveryFromPodTemplate not only discovery port, will also use labels in podTemplate
+func DiscoveryFromPodTemplate(w *unstructured.Unstructured, fields ...string) ([]intstr.IntOrString, map[string]string, error) {
+	obj, found, _ := unstructured.NestedMap(w.Object, fields...)
+	if !found {
+		return nil, nil, fmt.Errorf("not have spec.template in workload %v", w.GetName())
+	}
+	data, err := json.Marshal(obj)
+	if err != nil {
+		return nil, nil, fmt.Errorf("workload %v convert object err %v", w.GetName(), err)
+	}
+	var spec v1.PodTemplateSpec
+	err = json.Unmarshal(data, &spec)
+	if err != nil {
+		return nil, nil, fmt.Errorf("workload %v convert object to PodTemplate err %v", w.GetName(), err)
+	}
+	ports := getContainerPorts(spec.Spec.Containers)
+	if len(ports) == 0 {
+		return nil, nil, fmt.Errorf("no port found in workload %v", w.GetName())
+	}
+	return ports, spec.Labels, nil
+}
+
+func getContainerPorts(cs []v1.Container) []intstr.IntOrString {
+	var ports []intstr.IntOrString
+	//TODO(wonderflow): exclude some sidecars
+	for _, container := range cs {
+		for _, port := range container.Ports {
+			ports = append(ports, intstr.FromInt(int(port.ContainerPort)))
+		}
+	}
+	return ports
+}

pkg/controller/v1alpha1/metrics/metricstrait_controller.go

@@ -21,13 +21,10 @@ import (
 	"fmt"
 	"reflect"
 
-	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper"
-
-	"github.com/oam-dev/kubevela/pkg/controller/common"
-
 	monitoring "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
 	cpv1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
 	"github.com/crossplane/crossplane-runtime/pkg/event"
+	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper"
 	oamutil "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util"
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
@@ -42,10 +39,13 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/oam-dev/kubevela/api/v1alpha1"
+	"github.com/oam-dev/kubevela/pkg/controller/common"
+	"github.com/oam-dev/kubevela/pkg/controller/utils"
 )
 
 const (
 	errApplyServiceMonitor = "failed to apply the service monitor"
+	errFailDiscoveryLabels = "failed to discover labels from pod template, use workload labels directly"
 	servicePort            = 4848
 )
 
@@ -120,8 +120,9 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 			oamutil.PatchCondition(ctx, r, &metricsTrait,
 				cpv1alpha1.ReconcileError(errors.Wrap(err, common.ErrLocatingWorkload)))
 	}
+	var targetPort = metricsTrait.Spec.ScrapeService.TargetPort
 	// try to see if the workload already has services as child resources
-	serviceLabel, err := r.fetchServicesLabel(ctx, mLog, workload, metricsTrait.Spec.ScrapeService.TargetPort)
+	serviceLabel, err := r.fetchServicesLabel(ctx, mLog, workload, targetPort)
 	if err != nil && !apierrors.IsNotFound(err) {
 		r.record.Event(eventObj, event.Warning(common.ErrLocatingService, err))
 		return oamutil.ReconcileWaitResult,
@@ -130,7 +131,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	} else if serviceLabel == nil {
 		// TODO: use podMonitor instead?
 		// no service with the targetPort found, we will create a service that talks to the targetPort
-		serviceLabel, err = r.createService(ctx, mLog, workload, &metricsTrait)
+		serviceLabel, targetPort, err = r.createService(ctx, mLog, workload, &metricsTrait)
 		if err != nil {
 			r.record.Event(eventObj, event.Warning(common.ErrCreatingService, err))
 			return oamutil.ReconcileWaitResult,
@@ -138,8 +139,12 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 					cpv1alpha1.ReconcileError(errors.Wrap(err, common.ErrCreatingService)))
 		}
 	}
+
+	metricsTrait.Status.Port = targetPort
+	metricsTrait.Status.SelectorLabels = serviceLabel
+
 	// construct the serviceMonitor that hooks the service to the prometheus server
-	serviceMonitor := constructServiceMonitor(&metricsTrait, serviceLabel)
+	serviceMonitor := constructServiceMonitor(&metricsTrait, targetPort)
 	// server side apply the serviceMonitor, only the fields we set are touched
 	applyOpts := []client.PatchOption{client.ForceOwnership, client.FieldOwner(metricsTrait.GetUID())}
 	if err := r.Patch(ctx, serviceMonitor, client.Apply, applyOpts...); err != nil {
@@ -153,8 +158,8 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		fmt.Sprintf("successfully server side patched a serviceMonitor `%s`", serviceMonitor.Name)))
 
 	r.gcOrphanServiceMonitor(ctx, mLog, &metricsTrait)
-
-	return ctrl.Result{}, oamutil.PatchCondition(ctx, r, &metricsTrait, cpv1alpha1.ReconcileSuccess())
+	(&metricsTrait).SetConditions(cpv1alpha1.ReconcileSuccess())
+	return ctrl.Result{}, errors.Wrap(r.Status().Update(ctx, &metricsTrait), common.ErrUpdateStatus)
 }
 
 // fetch the label of the service that is associated with the workload
@@ -186,7 +191,7 @@ func (r *Reconciler) fetchServicesLabel(ctx context.Context, mLog logr.Logger,
 
 // create a service that targets the exposed workload pod
 func (r *Reconciler) createService(ctx context.Context, mLog logr.Logger, workload *unstructured.Unstructured,
-	metricsTrait *v1alpha1.MetricsTrait) (map[string]string, error) {
+	metricsTrait *v1alpha1.MetricsTrait) (map[string]string, intstr.IntOrString, error) {
 	oamService := &corev1.Service{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       common.ServiceKind,
@@ -201,18 +206,31 @@ func (r *Reconciler) createService(ctx context.Context, mLog logr.Logger, worklo
 			Type: corev1.ServiceTypeClusterIP,
 		},
 	}
+	var targetPort = metricsTrait.Spec.ScrapeService.TargetPort
 	// assign selector
-	if len(metricsTrait.Spec.ScrapeService.TargetSelector) == 0 {
-		// default is that we assumed that the pods have the same label as the workload
-		// we might be able to find the podSpec label but it is more complicated
-		oamService.Spec.Selector = workload.GetLabels()
+	ports, labels, err := utils.DiscoveryFromPodTemplate(workload, "spec", "template")
+	if err != nil {
+		mLog.Info(errFailDiscoveryLabels, "err", err)
+		if len(metricsTrait.Spec.ScrapeService.TargetSelector) == 0 {
+			// we assumed that the pods have the same label as the workload if no discoverable
+			oamService.Spec.Selector = workload.GetLabels()
+		} else {
+			oamService.Spec.Selector = metricsTrait.Spec.ScrapeService.TargetSelector
+		}
 	} else {
-		oamService.Spec.Selector = metricsTrait.Spec.ScrapeService.TargetSelector
+		oamService.Spec.Selector = labels
+	}
+	if targetPort.String() == "0" {
+		if len(ports) == 0 {
+			return nil, intstr.IntOrString{}, fmt.Errorf("no ports discovered or specified")
+		}
+		// choose the first one if no port specified
+		targetPort = ports[0]
 	}
 	oamService.Spec.Ports = []corev1.ServicePort{
 		{
 			Port:       servicePort,
-			TargetPort: metricsTrait.Spec.ScrapeService.TargetPort,
+			TargetPort: targetPort,
 			Protocol:   corev1.ProtocolTCP,
 		},
 	}
@@ -220,46 +238,41 @@ func (r *Reconciler) createService(ctx context.Context, mLog logr.Logger, worklo
 	applyOpts := []client.PatchOption{client.ForceOwnership, client.FieldOwner(metricsTrait.GetUID())}
 	if err := r.Patch(ctx, oamService, client.Apply, applyOpts...); err != nil {
 		mLog.Error(err, "Failed to apply to service")
-		return nil, err
+		return nil, intstr.IntOrString{}, err
 	}
-	return oamServiceLabel, nil
+	return oamService.Spec.Selector, targetPort, nil
 }
 
 // remove all service monitors that are no longer used
 func (r *Reconciler) gcOrphanServiceMonitor(ctx context.Context, mLog logr.Logger,
 	metricsTrait *v1alpha1.MetricsTrait) {
-	var gcCandidates []string
-	copy(metricsTrait.Status.ServiceMonitorNames, gcCandidates)
+	var gcCandidate = metricsTrait.Status.ServiceMonitorName
 	if metricsTrait.Spec.ScrapeService.Enabled != nil && !*metricsTrait.Spec.ScrapeService.Enabled {
 		// initialize it to be an empty list, gc everything
-		metricsTrait.Status.ServiceMonitorNames = []string{}
+		metricsTrait.Status.ServiceMonitorName = ""
 	} else {
 		// re-initialize to the current service monitor
-		metricsTrait.Status.ServiceMonitorNames = []string{metricsTrait.Name}
+		metricsTrait.Status.ServiceMonitorName = metricsTrait.Name
 	}
-	for _, smn := range gcCandidates {
-		if smn != metricsTrait.Name {
-			if err := r.Delete(ctx, &monitoring.ServiceMonitor{
-				TypeMeta: metav1.TypeMeta{
-					Kind:       serviceMonitorKind,
-					APIVersion: serviceMonitorAPIVersion,
-				},
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      smn,
-					Namespace: metricsTrait.GetNamespace(),
-				},
-			}, client.GracePeriodSeconds(10)); err != nil {
-				mLog.Error(err, "Failed to delete serviceMonitor", "name", smn, "error", err)
-				// add it back
-				metricsTrait.Status.ServiceMonitorNames = append(metricsTrait.Status.ServiceMonitorNames, smn)
-			}
-		}
+	if gcCandidate == metricsTrait.Name {
+		return
+	}
+	if err := r.Delete(ctx, &monitoring.ServiceMonitor{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       serviceMonitorKind,
+			APIVersion: serviceMonitorAPIVersion,
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      gcCandidate,
+			Namespace: metricsTrait.GetNamespace(),
+		},
+	}, client.GracePeriodSeconds(10)); err != nil {
+		mLog.Error(err, "Failed to delete serviceMonitor", "name", gcCandidate, "error", err)
 	}
 }
 
 // construct a serviceMonitor given a metrics trait along with a label selector pointing to the underlying service
-func constructServiceMonitor(metricsTrait *v1alpha1.MetricsTrait,
-	serviceLabels map[string]string) *monitoring.ServiceMonitor {
+func constructServiceMonitor(metricsTrait *v1alpha1.MetricsTrait, targetPort intstr.IntOrString) *monitoring.ServiceMonitor {
 	return &monitoring.ServiceMonitor{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       serviceMonitorKind,
@@ -282,7 +295,7 @@ func constructServiceMonitor(metricsTrait *v1alpha1.MetricsTrait,
 		},
 		Spec: monitoring.ServiceMonitorSpec{
 			Selector: metav1.LabelSelector{
-				MatchLabels: serviceLabels,
+				MatchLabels: oamServiceLabel,
 			},
 			// we assumed that the service is in the same namespace as the trait
 			NamespaceSelector: monitoring.NamespaceSelector{
@@ -290,7 +303,7 @@ func constructServiceMonitor(metricsTrait *v1alpha1.MetricsTrait,
 			},
 			Endpoints: []monitoring.Endpoint{
 				{
-					TargetPort: &metricsTrait.Spec.ScrapeService.TargetPort,
+					TargetPort: &targetPort,
 					Path:       metricsTrait.Spec.ScrapeService.Path,
 					Scheme:     metricsTrait.Spec.ScrapeService.Scheme,
 				},

pkg/controller/v1alpha1/metrics/metricstrait_controller_integration_test.go

@@ -193,7 +193,7 @@ var _ = Describe("Metrics Trait Integration Test", func() {
 			time.Second*10, time.Millisecond*500).Should(BeNil())
 		logf.Log.Info("[TEST] Get the created service", "service ports", createdService.Spec.Ports)
 		Expect(createdService.Labels).Should(Equal(oamServiceLabel))
-		Expect(createdService.Spec.Selector).Should(Equal(podSelector))
+		Expect(createdService.Spec.Selector).Should(Equal(deployLabel))
 		By("Check that we have created the serviceMonitor in the pre-defined namespaceName")
 		var serviceMonitor monitoringv1.ServiceMonitor
 		Eventually(

pkg/controller/v1alpha1/routes/route_controller.go

@@ -23,6 +23,8 @@ import (
 	"reflect"
 	"time"
 
+	"github.com/oam-dev/kubevela/pkg/controller/utils"
+
 	"github.com/oam-dev/kubevela/api/v1alpha1"
 	standardv1alpha1 "github.com/oam-dev/kubevela/api/v1alpha1"
 	"github.com/oam-dev/kubevela/pkg/controller/common"
@@ -255,16 +257,16 @@ func DiscoverPortsLabel(ctx context.Context, workload *unstructured.Unstructured
 	if err != nil {
 		return nil, nil, err
 	}
-	podSpecPath, ok := GetPodSpecPath(workloadDef)
+	podSpecPath, ok := utils.GetPodSpecPath(workloadDef)
 	if podSpecPath != "" {
-		ports, err := discoveryFromPodSpec(workload, podSpecPath)
+		ports, err := utils.DiscoveryFromPodSpec(workload, podSpecPath)
 		if err != nil {
 			return nil, nil, err
 		}
 		return ports, filterLabels(workload.GetLabels()), nil
 	}
 	if ok {
-		return discoveryFromPodTemplate(workload, "spec", "template")
+		return utils.DiscoveryFromPodTemplate(workload, "spec", "template")
 	}
 
 	// If workload is not podSpecable, try to detect it's child resource
@@ -272,7 +274,7 @@ func DiscoverPortsLabel(ctx context.Context, workload *unstructured.Unstructured
 	resources = append(resources, childResources...)
 	var gatherErrs []error
 	for _, w := range resources {
-		port, labels, err := discoveryFromPodTemplate(w, "spec", "template")
+		port, labels, err := utils.DiscoveryFromPodTemplate(w, "spec", "template")
 		if err == nil {
 			return port, labels, nil
 		}

pkg/controller/v1alpha1/routes/util.go

@@ -1,19 +1,11 @@
 package routes
 
 import (
-	"encoding/json"
-	"fmt"
-	"strconv"
-
 	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam"
 
-	"github.com/oam-dev/kubevela/api/types"
 	"github.com/oam-dev/kubevela/api/v1alpha1"
 
-	"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
-	"github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
@@ -35,76 +27,6 @@ func NeedDiscovery(routeTrait *v1alpha1.Route) bool {
 	return false
 }
 
-func GetPodSpecPath(workloadDef *v1alpha2.WorkloadDefinition) (string, bool) {
-	if workloadDef.Spec.PodSpecPath != "" {
-		return workloadDef.Spec.PodSpecPath, true
-	}
-	if workloadDef.Labels == nil {
-		return "", false
-	}
-	podSpecable, ok := workloadDef.Labels[types.LabelPodSpecable]
-	if !ok {
-		return "", false
-	}
-	ok, _ = strconv.ParseBool(podSpecable)
-	return "", ok
-}
-
-func discoveryFromPodSpec(w *unstructured.Unstructured, fieldPath string) ([]intstr.IntOrString, error) {
-	paved := fieldpath.Pave(w.Object)
-	obj, err := paved.GetValue(fieldPath)
-	if err != nil {
-		return nil, err
-	}
-	data, err := json.Marshal(obj)
-	if err != nil {
-		return nil, fmt.Errorf("discovery podSpec from %s in workload %v err %v", fieldPath, w.GetName(), err)
-	}
-	var spec corev1.PodSpec
-	err = json.Unmarshal(data, &spec)
-	if err != nil {
-		return nil, fmt.Errorf("discovery podSpec from %s in workload %v err %v", fieldPath, w.GetName(), err)
-	}
-	ports := getContainerPorts(spec.Containers)
-	if len(ports) == 0 {
-		return nil, fmt.Errorf("no port found in podSpec %v", w.GetName())
-	}
-	return ports, nil
-}
-
-// discoveryFromPodTemplate not only discovery port, will also use labels in podTemplate
-func discoveryFromPodTemplate(w *unstructured.Unstructured, fields ...string) ([]intstr.IntOrString, map[string]string, error) {
-	obj, found, _ := unstructured.NestedMap(w.Object, fields...)
-	if !found {
-		return nil, nil, fmt.Errorf("not have spec.template in workload %v", w.GetName())
-	}
-	data, err := json.Marshal(obj)
-	if err != nil {
-		return nil, nil, fmt.Errorf("workload %v convert object err %v", w.GetName(), err)
-	}
-	var spec corev1.PodTemplateSpec
-	err = json.Unmarshal(data, &spec)
-	if err != nil {
-		return nil, nil, fmt.Errorf("workload %v convert object to PodTemplate err %v", w.GetName(), err)
-	}
-	ports := getContainerPorts(spec.Spec.Containers)
-	if len(ports) == 0 {
-		return nil, nil, fmt.Errorf("no port found in workload %v", w.GetName())
-	}
-	return ports, spec.Labels, nil
-}
-
-func getContainerPorts(cs []corev1.Container) []intstr.IntOrString {
-	var ports []intstr.IntOrString
-	//TODO(wonderflow): exclude some sidecars
-	for _, container := range cs {
-		for _, port := range container.Ports {
-			ports = append(ports, intstr.FromInt(int(port.ContainerPort)))
-		}
-	}
-	return ports
-}
-
 // MatchService try check if the service matches the rules
 func MatchService(targetPort intstr.IntOrString, rule v1alpha1.Rule) bool {
 	// the rule is nil, continue

pkg/oam/trait_checker.go

@@ -31,6 +31,8 @@ func GetChecker(traitType string, c client.Client) Checker {
 	switch traitType {
 	case "route":
 		return &RouteChecker{c: c}
+	case "metric":
+		return &MetricChecker{c: c}
 	}
 	return &DefaultChecker{c: c}
 }
@@ -59,11 +61,36 @@ func (d *DefaultChecker) Check(ctx context.Context, reference runtimev1alpha1.Ty
 	}
 	var message string
 	for k, v := range traitData {
-		message += fmt.Sprintf("%v=%v\n", k, v)
+		message += fmt.Sprintf("%v=%v\n\t\t", k, v)
 	}
 	return StatusDone, message, err
 }
 
+type MetricChecker struct {
+	c client.Client
+}
+
+func (d *MetricChecker) Check(ctx context.Context, reference runtimev1alpha1.TypedReference, _ string, appConfig *v1alpha2.ApplicationConfiguration, _ *application.Application) (CheckStatus, string, error) {
+	metric := v1alpha1.MetricsTrait{}
+	if err := d.c.Get(ctx, client.ObjectKey{Namespace: appConfig.Namespace, Name: reference.Name}, &metric); err != nil {
+		return StatusChecking, "", err
+	}
+	condition := metric.Status.Conditions
+	if len(condition) < 1 {
+		return StatusChecking, "", nil
+	}
+	if condition[0].Status != v1.ConditionTrue {
+		return StatusChecking, condition[0].Message, nil
+	}
+	if metric.Spec.ScrapeService.Enabled != nil && !*metric.Spec.ScrapeService.Enabled {
+		return StatusDone, "Monitoring disabled", nil
+	}
+	var message = fmt.Sprintf("Monitoring port: %s, path: %s, format: %s, schema: %s.",
+		metric.Status.Port.String(), metric.Spec.ScrapeService.Path,
+		metric.Spec.ScrapeService.Format, metric.Spec.ScrapeService.Scheme)
+	return StatusDone, message, nil
+}
+
 type RouteChecker struct {
 	c client.Client
 }